diff --git a/s3api/middlewares/url-decoder.go b/s3api/middlewares/url-decoder.go index ac61852..a981f56 100644 --- a/s3api/middlewares/url-decoder.go +++ b/s3api/middlewares/url-decoder.go @@ -18,14 +18,18 @@ import ( "net/url" "github.com/gofiber/fiber/v3" + "github.com/versity/versitygw/debuglogger" + "github.com/versity/versitygw/s3err" ) // DecodeURL url path unescapes the request url for the gateway -// to handle some special characters +// to handle some special characters. A malformed escape sequence +// is rejected with InvalidURI. func DecodeURL(ctx fiber.Ctx) error { unescp, err := url.PathUnescape(string(ctx.Request().URI().PathOriginal())) if err != nil { - return err + debuglogger.Logf("failed to unescape the request path: %v", err) + return s3err.GetAPIError(s3err.ErrInvalidURI) } ctx.Path(unescp) return nil diff --git a/s3api/middlewares/url-decoder_test.go b/s3api/middlewares/url-decoder_test.go new file mode 100644 index 0000000..73761ae --- /dev/null +++ b/s3api/middlewares/url-decoder_test.go @@ -0,0 +1,75 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package middlewares + +import ( + "testing" + + "github.com/gofiber/fiber/v3" + "github.com/stretchr/testify/assert" + "github.com/valyala/fasthttp" + "github.com/versity/versitygw/s3err" +) + +func TestDecodeURL(t *testing.T) { + tests := []struct { + name string + uri string + wantPath string + wantErr error + }{ + { + name: "plain path", + uri: "/bucket/object", + wantPath: "/bucket/object", + }, + { + name: "escaped characters", + uri: "/bucket/my%20object%2Bkey", + wantPath: "/bucket/my object+key", + }, + { + name: "invalid escape", + uri: "/bucket/(*&^%&())", + wantErr: s3err.GetAPIError(s3err.ErrInvalidURI), + }, + { + name: "truncated escape", + uri: "/bucket/object%2", + wantErr: s3err.GetAPIError(s3err.ErrInvalidURI), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // net/http refuses to build a request with a malformed escape, + // so the raw uri is set on the fasthttp request directly + fctx := &fasthttp.RequestCtx{} + fctx.Request.SetRequestURI(tt.uri) + app := fiber.New() + ctx := app.AcquireCtx(fctx) + defer app.ReleaseCtx(ctx) + + err := DecodeURL(ctx) + if tt.wantErr != nil { + assert.Equal(t, tt.wantErr, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.wantPath, ctx.Path()) + }) + } +} diff --git a/tests/integration/general.go b/tests/integration/general.go index 50ff019..ecffa9c 100644 --- a/tests/integration/general.go +++ b/tests/integration/general.go @@ -342,6 +342,27 @@ func RouterObjectTorrentMethodNotAllowed(s *S3Conf) error { }) } +func RouterInvalidURLEscape(s *S3Conf) error { + testName := "RouterInvalidURLEscape" + return actionHandlerNoSetup(s, testName, func(s3client *s3.Client, bucket string) error { + req, err := http.NewRequest(http.MethodGet, s.endpoint+"/bucket/object", nil) + if err != nil { + return err + } + + // http.NewRequest refuses a malformed escape, so the raw path + // is set on the url directly + req.URL.Opaque = "/bucket/(*&^%&())" + + resp, err := s.httpClient.Do(req) + if err != nil { + return err + } + + return checkHTTPResponseApiErr(resp, s3err.GetAPIError(s3err.ErrInvalidURI)) + }) +} + // CORS middleware tests func CORSMiddleware_invalid_method(s *S3Conf) error { testName := "CORSMiddleware_invalid_method" diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 0b5eb90..fe51f04 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -2162,6 +2162,7 @@ func TestRouter(ts *TestState) { ts.Run(RouterObjectAnnotationNotImplemented) ts.Run(RouterRenameObjectNotImplemented) ts.Run(RouterObjectTorrentMethodNotAllowed) + ts.Run(RouterInvalidURLEscape) } func TestPostObject(ts *TestState) { @@ -3749,6 +3750,7 @@ func GetIntTests() IntTests { "RouterObjectAnnotationNotImplemented": RouterObjectAnnotationNotImplemented, "RouterRenameObjectNotImplemented": RouterRenameObjectNotImplemented, "RouterObjectTorrentMethodNotAllowed": RouterObjectTorrentMethodNotAllowed, + "RouterInvalidURLEscape": RouterInvalidURLEscape, "UnsignedStreaminPayloadTrailer_malformed_trailer": UnsignedStreaminPayloadTrailer_malformed_trailer, "UnsignedStreamingPayloadTrailer_aborted_connection": UnsignedStreamingPayloadTrailer_aborted_connection, "UnsignedStreamingPayloadTrailer_missing_invalid_dec_content_length": UnsignedStreamingPayloadTrailer_missing_invalid_dec_content_length, diff --git a/website/handler.go b/website/handler.go index b7e48a0..d43bdae 100644 --- a/website/handler.go +++ b/website/handler.go @@ -141,8 +141,7 @@ func registerWebsiteRoutes(app *fiber.App, be backend.Backend, domain string) { // report malformed percent-encoding as an html error page. func decodeURL(ctx fiber.Ctx) error { if err := middlewares.DecodeURL(ctx); err != nil { - debuglogger.Logf("failed to unescape the request path: %v", err) - return sendError(ctx, s3err.GetAPIError(s3err.ErrInvalidURI)) + return sendError(ctx, err) } return ctx.Next()