[#224] Add conditional escaping for object name

Chi gives inconsistent results in terms of whether
the strings returned are URL coded or not
See:
* https://github.com/go-chi/chi/issues/641
* https://github.com/go-chi/chi/issues/642

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
This commit is contained in:
Denis Kirillov 2023-10-27 17:56:51 +03:00 committed by Alexey Vanin
parent b169c5e6c3
commit 0bed25816c
5 changed files with 52 additions and 5 deletions

View file

@ -13,7 +13,7 @@ This document outlines major changes between releases.
- Replace part on re-upload when use multipart upload (#176) - Replace part on re-upload when use multipart upload (#176)
- Fix goroutine leak on put object error (#178) - Fix goroutine leak on put object error (#178)
- Fix parsing signed headers in presigned urls (#182) - Fix parsing signed headers in presigned urls (#182)
- Fix url escaping (#188) - Fix url escaping (#188, #224)
- Use correct keys in `list-multipart-uploads` response (#185) - Use correct keys in `list-multipart-uploads` response (#185)
- Fix parsing `key-marker` for object list versions (#243) - Fix parsing `key-marker` for object list versions (#243)
- Fix marshaling errors in `DeleteObjects` method (#222) - Fix marshaling errors in `DeleteObjects` method (#222)

View file

@ -112,6 +112,36 @@ func TestS3CompatibilityBucketListV2BothContinuationTokenStartAfter(t *testing.T
require.Equal(t, "quxx", listV2Response2.Contents[1].Key) require.Equal(t, "quxx", listV2Response2.Contents[1].Key)
} }
func TestS3BucketListV2EncodingBasic(t *testing.T) {
hc := prepareHandlerContext(t)
bktName := "bucket-for-listing-v1-encoding"
bktInfo := createTestBucket(hc, bktName)
objects := []string{"foo+1/bar", "foo/bar/xyzzy", "quux ab/thud", "asdf+b"}
for _, objName := range objects {
createTestObject(hc, bktInfo, objName)
}
query := make(url.Values)
query.Add("delimiter", "/")
query.Add("encoding-type", "url")
w, r := prepareTestFullRequest(hc, bktName, "", query, nil)
hc.Handler().ListObjectsV2Handler(w, r)
assertStatus(hc.t, w, http.StatusOK)
listV2Response := &ListObjectsV2Response{}
parseTestResponse(hc.t, w, listV2Response)
require.Equal(t, "/", listV2Response.Delimiter)
require.Len(t, listV2Response.Contents, 1)
require.Equal(t, "asdf%2Bb", listV2Response.Contents[0].Key)
require.Len(t, listV2Response.CommonPrefixes, 3)
require.Equal(t, "foo%2B1/", listV2Response.CommonPrefixes[0].Prefix)
require.Equal(t, "foo/", listV2Response.CommonPrefixes[1].Prefix)
require.Equal(t, "quux%20ab/", listV2Response.CommonPrefixes[2].Prefix)
}
func TestS3BucketListDelimiterBasic(t *testing.T) { func TestS3BucketListDelimiterBasic(t *testing.T) {
tc := prepareHandlerContext(t) tc := prepareHandlerContext(t)

View file

@ -241,12 +241,23 @@ func AddObjectName(l *zap.Logger) Func {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context() ctx := r.Context()
reqInfo := GetReqInfo(ctx) reqInfo := GetReqInfo(ctx)
reqLogger := reqLogOrDefault(ctx, l)
rctx := chi.RouteContext(ctx) rctx := chi.RouteContext(ctx)
// trim leading slash (always present) // trim leading slash (always present)
reqInfo.ObjectName = rctx.RoutePath[1:] reqInfo.ObjectName = rctx.RoutePath[1:]
reqLogger := reqLogOrDefault(ctx, l) if r.URL.RawPath != "" {
// we have to do this because of
// https://github.com/go-chi/chi/issues/641
// https://github.com/go-chi/chi/issues/642
if obj, err := url.PathUnescape(reqInfo.ObjectName); err != nil {
reqLogger.Warn(logs.FailedToUnescapeObjectName, zap.Error(err))
} else {
reqInfo.ObjectName = obj
}
}
r = r.WithContext(SetReqLogger(ctx, reqLogger.With(zap.String("object", reqInfo.ObjectName)))) r = r.WithContext(SetReqLogger(ctx, reqLogger.With(zap.String("object", reqInfo.ObjectName))))
h.ServeHTTP(w, r) h.ServeHTTP(w, r)

View file

@ -82,15 +82,20 @@ func TestRouterObjectEscaping(t *testing.T) {
objName: "fix/object", objName: "fix/object",
}, },
{ {
name: "with percentage", name: "with slash escaped",
expectedObjName: "fix/object%ac", expectedObjName: "/foo/bar",
objName: "fix/object%ac", objName: "/foo%2fbar",
}, },
{ {
name: "with percentage escaped", name: "with percentage escaped",
expectedObjName: "fix/object%ac", expectedObjName: "fix/object%ac",
objName: "fix/object%25ac", objName: "fix/object%25ac",
}, },
{
name: "with awful mint name",
expectedObjName: "äöüex ®©µÄÆÐÕæŒƕƩDž 01000000 0x40 \u0040 amȡȹɆple&0a!-_.*'()&$@=;:+,?<>.pdf",
objName: "%C3%A4%C3%B6%C3%BCex%20%C2%AE%C2%A9%C2%B5%C3%84%C3%86%C3%90%C3%95%C3%A6%C5%92%C6%95%C6%A9%C7%85%2001000000%200x40%20%40%20am%C8%A1%C8%B9%C9%86ple%260a%21-_.%2A%27%28%29%26%24%40%3D%3B%3A%2B%2C%3F%3C%3E.pdf",
},
} { } {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
target := fmt.Sprintf("/%s/%s", bktName, tc.objName) target := fmt.Sprintf("/%s/%s", bktName, tc.objName)

View file

@ -95,6 +95,7 @@ const (
FailedToPassAuthentication = "failed to pass authentication" // Error in ../../api/middleware/auth.go FailedToPassAuthentication = "failed to pass authentication" // Error in ../../api/middleware/auth.go
FailedToResolveCID = "failed to resolve CID" // Debug in ../../api/middleware/metrics.go FailedToResolveCID = "failed to resolve CID" // Debug in ../../api/middleware/metrics.go
RequestStart = "request start" // Info in ../../api/middleware/reqinfo.go RequestStart = "request start" // Info in ../../api/middleware/reqinfo.go
FailedToUnescapeObjectName = "failed to unescape object name" // Warn in ../../api/middleware/reqinfo.go
CouldNotHandleMessage = "could not handle message" // Error in ../../api/notifications/controller.go CouldNotHandleMessage = "could not handle message" // Error in ../../api/notifications/controller.go
CouldNotACKMessage = "could not ACK message" // Error in ../../api/notifications/controller.go CouldNotACKMessage = "could not ACK message" // Error in ../../api/notifications/controller.go
CouldntMarshalAnEvent = "couldn't marshal an event" // Error in ../../api/notifications/controller.go CouldntMarshalAnEvent = "couldn't marshal an event" // Error in ../../api/notifications/controller.go