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
This commit is contained in:
1 parent
59493b527e
commit
e0fef79b7b
5 files changed
+105
-4
No files matched your search
@@ -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
|
||||
|
||||
@@ -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())
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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"
|
||||
|
||||
@@ -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,
|
||||
|
||||
+1
-2
@@ -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()
|
||||
|
||||
Reference in new issue
Block a user