From e0fef79b7b00a76729ab5fb8c35f60eadc8c9455 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 2 Oct 2026 15:59:21 +0200 Subject: [PATCH] fix: return InvalidURI for a malformed escape in the request path A request path with an invalid percent escape, such as "/bucket/%zz", failed url.PathUnescape in the DecodeURL middleware and the raw error was answered with 500 InternalError. S3 rejects such a request with 400 InvalidURI. Return InvalidURI from DecodeURL, so the s3 and admin routers answer with it as well, and let the website handler pass the error through instead of mapping it on its own. Fixes #2070 --- s3api/middlewares/url-decoder.go | 8 ++- s3api/middlewares/url-decoder_test.go | 75 +++++++++++++++++++++++++++ tests/integration/general.go | 21 ++++++++ tests/integration/group-tests.go | 2 + website/handler.go | 3 +- 5 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 s3api/middlewares/url-decoder_test.go 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()