diff --git a/weed/s3api/filer_multipart.go b/weed/s3api/filer_multipart.go index 9d9f1f8cc8..299c9899cd 100644 --- a/weed/s3api/filer_multipart.go +++ b/weed/s3api/filer_multipart.go @@ -1420,9 +1420,9 @@ func (p *multipartUploadPage) result() *ListMultipartUploadsResult { output.Upload = append(output.Upload, upload) } if p.more { - last := output.Upload[len(output.Upload)-1] + last := candidates[len(candidates)-1] output.IsTruncated = aws.Bool(true) - output.NextKeyMarker, output.NextUploadIdMarker = last.Key, last.UploadId + output.NextKeyMarker, output.NextUploadIdMarker = aws.String(last.key), aws.String(last.entry.Name) } if aws.StringValue(input.EncodingType) == s3.EncodingTypeUrl { // AWS url-encodes these key-named elements when encoding-type=url is @@ -1436,22 +1436,29 @@ func (p *multipartUploadPage) result() *ListMultipartUploadsResult { for _, upload := range output.Upload { upload.Key = encodeMultipartUploadURLField(upload.Key) } + } else { + output.KeyMarker = encodeListContinuationTokenField(output.KeyMarker) + output.NextKeyMarker = encodeListContinuationTokenField(output.NextKeyMarker) } return output } -// encodeMultipartUploadURLField url-encodes key text the way ListObjects does: -// every "/"-separated segment is query-escaped and the separators are kept, so a -// client recovers the exact key with url.QueryUnescape. +// urlQueryEscape url-encodes key text per segment: every "/"-separated part is +// query-escaped and the separators are kept, so a client recovers the exact key +// with url.QueryUnescape. +func urlQueryEscape(key string) string { + segments := strings.Split(key, "/") + for i, segment := range segments { + segments[i] = url.QueryEscape(segment) + } + return strings.Join(segments, "/") +} + func encodeMultipartUploadURLField(value *string) *string { if value == nil { return nil } - segments := strings.Split(*value, "/") - for i, segment := range segments { - segments[i] = url.QueryEscape(segment) - } - return aws.String(strings.Join(segments, "/")) + return aws.String(urlQueryEscape(*value)) } type ListPartsResult struct { diff --git a/weed/s3api/filer_multipart_list_test.go b/weed/s3api/filer_multipart_list_test.go index c00f38acaf..3d2a73789c 100644 --- a/weed/s3api/filer_multipart_list_test.go +++ b/weed/s3api/filer_multipart_list_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "math" + "net/url" "slices" "testing" @@ -143,6 +144,68 @@ func TestSelectMultipartUploadsHoldsOnlyThePage(t *testing.T) { } } +// With encoding-type=url the markers and upload keys take the wire form the +// object listings already use: a client that echoes the escaped marker back +// still resumes at the decoded key. +func TestSelectMultipartUploadsUrlEncodesMarkersAndKeys(t *testing.T) { + entries := []*filer_pb.Entry{ + uploadEntry("1", "a\x01b", 10), + uploadEntry("2", "a b", 20), + uploadEntry("3", "b", 30), + } + input := listUploadsInput("", "", "", 1) + input.EncodingType = aws.String(s3.EncodingTypeUrl) + + out := selectMultipartUploads(input, entries) + if *out.Upload[0].Key != "a%01b" || *out.NextUploadIdMarker != "1" { + t.Fatalf("page got key %q upload-id-marker %q", *out.Upload[0].Key, *out.NextUploadIdMarker) + } + marker, err := url.QueryUnescape(*out.NextKeyMarker) + if err != nil || decodeListContinuationToken(marker) != "a\x01b" { + t.Fatalf("next key marker %q does not resume at %q", *out.NextKeyMarker, "a\x01b") + } + + input.KeyMarker = aws.String(decodeListContinuationToken(marker)) + input.UploadIdMarker = out.NextUploadIdMarker + out = selectMultipartUploads(input, entries) + if *out.KeyMarker != "a%01b" || *out.Upload[0].Key != "a+b" || *out.NextKeyMarker != "a+b" { + t.Fatalf("page got marker %q key %q next %q", *out.KeyMarker, *out.Upload[0].Key, *out.NextKeyMarker) + } + + input.KeyMarker = aws.String("a b") + input.UploadIdMarker = out.NextUploadIdMarker + out = selectMultipartUploads(input, entries) + if *out.IsTruncated || *out.Upload[0].Key != "b" { + t.Fatalf("last page got truncated=%v key %q", *out.IsTruncated, *out.Upload[0].Key) + } +} + +// A raw key that already carries the token prefix must survive an +// encoding-type=url round trip: under url mode the wire form is the url escape +// alone, so the request decode only unescapes — running the token decode on +// "base64:..." would misread the raw key as an encoded token. +func TestSelectMultipartUploadsUrlTokenPrefixedKeyRoundTrip(t *testing.T) { + entries := []*filer_pb.Entry{ + uploadEntry("1", "base64:AQ", 10), + uploadEntry("2", "base64:z", 20), + } + input := listUploadsInput("", "", "", 1) + input.EncodingType = aws.String(s3.EncodingTypeUrl) + + out := selectMultipartUploads(input, entries) + resume := decodeEscapedEcho(*out.NextKeyMarker, true, urlQueryEscape) + if resume != "base64:AQ" { + t.Fatalf("next key marker %q resumes at %q, want %q", *out.NextKeyMarker, resume, "base64:AQ") + } + + input.KeyMarker = aws.String(resume) + input.UploadIdMarker = out.NextUploadIdMarker + out = selectMultipartUploads(input, entries) + if *out.IsTruncated || len(out.Upload) != 1 || *out.Upload[0].Key != "base64%3Az" { + t.Fatalf("second page got truncated=%v uploads %v", *out.IsTruncated, out.Upload) + } +} + func TestSelectMultipartUploadsCapsRequestedPageSize(t *testing.T) { // max-uploads is caller-controlled; the page stays bounded at the server cap page := newMultipartUploadPage(listUploadsInput("", "", "", 10*maxUploadsList)) diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 6b049e0e70..76614932ce 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -2,6 +2,7 @@ package s3api import ( "context" + "encoding/base64" "encoding/xml" "errors" "fmt" @@ -11,7 +12,9 @@ import ( "sort" "strconv" "strings" + "unicode/utf8" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" @@ -70,8 +73,8 @@ func toListBucketResultV1(in ListBucketResult) listBucketResultV1 { Metadata: in.Metadata, Name: in.Name, Prefix: in.Prefix, - Marker: in.Marker, - NextMarker: in.NextMarker, + Marker: encodeMarkerEcho(in.Marker, in.EncodingType == s3.EncodingTypeUrl), + NextMarker: encodeMarkerEcho(in.NextMarker, in.EncodingType == s3.EncodingTypeUrl), MaxKeys: in.MaxKeys, Delimiter: in.Delimiter, IsTruncated: in.IsTruncated, @@ -104,9 +107,10 @@ func (s3a *S3ApiServer) ListObjectsV2Handler(w http.ResponseWriter, r *http.Requ return } - marker := continuationToken.string + startAfterKey := decodeMarkerEcho(startAfter, encodingTypeUrl) + marker := decodeListContinuationToken(continuationToken.string) if !continuationToken.set { - marker = startAfter + marker = startAfterKey } // Adjust marker if it ends with delimiter to skip all entries with that prefix @@ -145,27 +149,139 @@ func (s3a *S3ApiServer) ListObjectsV2Handler(w http.ResponseWriter, r *http.Requ IsTruncated: response.IsTruncated, KeyCount: len(response.Contents) + len(response.CommonPrefixes), MaxKeys: uint16(response.MaxKeys), - NextContinuationToken: response.NextMarker, + NextContinuationToken: encodeListContinuationToken(response.NextMarker), Prefix: response.Prefix, StartAfter: startAfter, } if encodingTypeUrl { responseV2.EncodingType = s3.EncodingTypeUrl + responseV2.StartAfter = urlPathEscape(startAfterKey) } glog.V(3).Infof("ListObjectsV2Handler response: %+v", responseV2) writeSuccessResponseXML(w, r, responseV2) } +// continuationTokenEncodingPrefix tags a NextContinuationToken whose payload is the +// base64url encoding of the raw key. XML 1.0 cannot carry control characters, and the +// XML encoder replaces them with U+FFFD, so a token holding such a key would resume +// past a mangled name and skip every key sorting between the two. Keys that are valid +// XML text still travel as the bare key, so the token is unchanged for them. +const continuationTokenEncodingPrefix = "base64:" + +func encodeListContinuationToken(key string) string { + if key == "" { + return "" + } + if isXMLText(key) && !strings.HasPrefix(key, continuationTokenEncodingPrefix) { + return key + } + return continuationTokenEncodingPrefix + base64.RawURLEncoding.EncodeToString([]byte(key)) +} + +func encodeListContinuationTokenField(value *string) *string { + if value == nil { + return nil + } + return aws.String(encodeListContinuationToken(*value)) +} + +// encodeMarkerEcho renders a decoded request marker for the response echo: +// the token encoding keeps an XML-unsafe key legible, and urlPathEscape applies +// the encoding-type=url form the keys in the same response already carry. +func encodeMarkerEcho(marker string, encodingTypeUrl bool) string { + marker = encodeListContinuationToken(marker) + if encodingTypeUrl { + marker = urlPathEscape(marker) + } + return marker +} + +// decodeMarkerEcho reverses the wire encoding on a marker the client echoes +// back: under encoding-type=url the emitted echo was urlPathEscape'd. Only a +// value that is the canonical escaped form of its unescape could have come +// from that encoder — "a%2541" round-trips to itself, "a%41" does not — so +// anything else stays the raw key the client sent. +func decodeMarkerEcho(marker string, encodingTypeUrl bool) string { + return decodeEscapedEcho(marker, encodingTypeUrl, urlPathEscape) +} + +func decodeEscapedEcho(marker string, encodingTypeUrl bool, escape func(string) string) string { + if !encodingTypeUrl { + return marker + } + if unescaped, err := url.QueryUnescape(marker); err == nil && escape(unescaped) == marker { + return unescaped + } + return marker +} + +func decodeListContinuationToken(token string) string { + key, ok := strings.CutPrefix(token, continuationTokenEncodingPrefix) + if !ok { + return token + } + decoded, err := base64.RawURLEncoding.DecodeString(key) + if err != nil { + // Not a token this server wrote; resume at the token as the raw key. + return token + } + decodedKey := string(decoded) + // encodeListContinuationToken only writes a token for a key that cannot + // travel bare, so a decoded key that could have stayed bare — valid XML + // text without the prefix — means the input was a legacy raw-key token, + // not something to decode. + if isXMLText(decodedKey) && !strings.HasPrefix(decodedKey, continuationTokenEncodingPrefix) { + return token + } + // A decoded key that could not travel bare makes the token ambiguous + // ("base64:AQ" reads as both the encoded form of "\x01" and a literal + // legacy key). The encoded reading wins: preferring the raw one would + // resume past real entries for every token this server writes, while a + // legacy token aliasing the encoded form only re-lists keys. + return decodedKey +} + +// isXMLText reports whether s survives an XML 1.0 round trip unchanged, i.e. the +// encoder does not replace any of its characters with U+FFFD. +func isXMLText(s string) bool { + for i := 0; i < len(s); { + r, size := utf8.DecodeRuneInString(s[i:]) + if r == utf8.RuneError && size == 1 { + return false + } + if !isXMLChar(r) { + return false + } + i += size + } + return true +} + +// isXMLChar follows the XML 1.0 Char production the encoding/xml encoder enforces. +func isXMLChar(r rune) bool { + switch { + case r == 0x9 || r == 0xA || r == 0xD: + return true + case r >= 0x20 && r <= 0xD7FF: + return true + case r >= 0xE000 && r <= 0xFFFD: + return true + case r >= 0x10000 && r <= 0x10FFFF: + return true + } + return false +} + func (s3a *S3ApiServer) ListObjectsV1Handler(w http.ResponseWriter, r *http.Request) { // https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjects.html // collect parameters bucket, _ := s3_constants.GetBucketAndObject(r) - originalPrefix, marker, delimiter, encodingTypeUrl, maxKeys, allowUnordered, errCode := getListObjectsV1Args(r.URL.Query()) + originalPrefix, listMarker, delimiter, encodingTypeUrl, maxKeys, allowUnordered, errCode := getListObjectsV1Args(r.URL.Query()) - glog.V(2).Infof("ListObjectsV1Handler bucket=%s prefix=%s marker=%s delimiter=%s maxKeys=%d", bucket, originalPrefix, marker, delimiter, maxKeys) + glog.V(2).Infof("ListObjectsV1Handler bucket=%s prefix=%s marker=%s delimiter=%s maxKeys=%d", bucket, originalPrefix, listMarker, delimiter, maxKeys) if errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) @@ -181,8 +297,8 @@ func (s3a *S3ApiServer) ListObjectsV1Handler(w http.ResponseWriter, r *http.Requ } // Adjust marker if it ends with delimiter to skip all entries with that prefix - requestMarker := marker - marker = adjustMarkerForDelimiter(marker, originalPrefix, delimiter) + requestMarker := decodeListContinuationToken(decodeMarkerEcho(listMarker, encodingTypeUrl)) + marker := adjustMarkerForDelimiter(requestMarker, originalPrefix, delimiter) response, err := s3a.listFilerEntries(r.Context(), listObjectsRequest{ bucket: bucket, diff --git a/weed/s3api/s3api_object_handlers_list_token_test.go b/weed/s3api/s3api_object_handlers_list_token_test.go new file mode 100644 index 0000000000..75ffbdcf9f --- /dev/null +++ b/weed/s3api/s3api_object_handlers_list_token_test.go @@ -0,0 +1,328 @@ +package s3api + +import ( + "encoding/xml" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "testing" + "time" + + "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type listV2TestPage struct { + XMLName xml.Name `xml:"ListBucketResult"` + IsTruncated bool `xml:"IsTruncated"` + NextContinuationToken string `xml:"NextContinuationToken"` + StartAfter string `xml:"StartAfter"` + Contents []struct { + Key string `xml:"Key"` + } `xml:"Contents"` +} + +func listTokenTestFile(name string) *filer_pb.Entry { + return &filer_pb.Entry{Name: name, Attributes: &filer_pb.FuseAttributes{}} +} + +func newListTokenTestServer(t *testing.T, keys []string) *S3ApiServer { + t.Helper() + entries := make([]*filer_pb.Entry, 0, len(keys)) + for _, key := range keys { + entries = append(entries, listTokenTestFile(key)) + } + filer := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/test": entries, + }} + s3a := newFailoverTestServer(t, startFakeFiler(t, filer)) + s3a.option.BucketsPath = "/buckets" + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set("test", &BucketConfig{Name: "test"}) + return s3a +} + +func listV2Page(t *testing.T, s3a *S3ApiServer, query url.Values) listV2TestPage { + t.Helper() + r := httptest.NewRequest(http.MethodGet, "/test?"+query.Encode(), nil) + r = mux.SetURLVars(r, map[string]string{"bucket": "test"}) + w := httptest.NewRecorder() + s3a.ListObjectsV2Handler(w, r) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var page listV2TestPage + require.NoError(t, xml.Unmarshal(w.Body.Bytes(), &page), w.Body.String()) + return page +} + +// A page that ends on a key XML 1.0 cannot carry must still resume right after +// that key. The token used to be the raw key, which the XML encoder turned into +// U+FFFD, so the next page resumed past the mangled name and skipped every key +// sorting between it and the real one. +func TestListObjectsV2ContinuationTokenWithXMLUnsafeKeys(t *testing.T) { + keys := []string{"a\x01b", "a\x02b", "a\x1fb", "a b", "b", "c"} + + for _, encodingTypeURL := range []bool{false, true} { + t.Run("encoding-type=url:"+strconv.FormatBool(encodingTypeURL), func(t *testing.T) { + s3a := newListTokenTestServer(t, keys) + + var got []string + token := "" + for page := 0; ; page++ { + require.Less(t, page, len(keys), "listing did not terminate") + query := url.Values{"list-type": {"2"}, "max-keys": {"2"}} + if encodingTypeURL { + query.Set("encoding-type", "url") + } + if token != "" { + query.Set("continuation-token", token) + } + result := listV2Page(t, s3a, query) + for _, content := range result.Contents { + key := content.Key + if encodingTypeURL { + decoded, err := url.QueryUnescape(key) + require.NoError(t, err) + key = decoded + } + got = append(got, key) + } + if !result.IsTruncated { + break + } + require.NotEmpty(t, result.NextContinuationToken) + token = result.NextContinuationToken + } + + if encodingTypeURL { + assert.Equal(t, keys, got) + return + } + // Without encoding-type=url the keys themselves still lose control + // characters to the XML encoder, but no key may be skipped. + require.Len(t, got, len(keys)) + assert.Equal(t, "c", got[len(got)-1]) + }) + } +} + +// The StartAfter echo is the wire encoding of the decoded key, so a client +// that feeds the echo back lands on the same resume point instead of +// drifting to the literal escape text. +func TestListObjectsV2StartAfterEchoStable(t *testing.T) { + s3a := newListTokenTestServer(t, []string{"a b", "c"}) + for _, sent := range []string{"a%20b", "a b"} { + result := listV2Page(t, s3a, url.Values{ + "list-type": {"2"}, "encoding-type": {"url"}, "start-after": {sent}, + }) + assert.Equal(t, "a%20b", result.StartAfter, "start-after %q", sent) + require.Len(t, result.Contents, 1) + assert.Equal(t, "c", result.Contents[0].Key) + } +} + +// A token for a key that XML carries unchanged stays the bare key. +func TestListObjectsV2ContinuationTokenUnchangedForNormalKeys(t *testing.T) { + keys := []string{"a\x01b", "a\x02b", "a\x1fb", "a b", "b", "c"} + s3a := newListTokenTestServer(t, keys) + + result := listV2Page(t, s3a, url.Values{"list-type": {"2"}, "max-keys": {"5"}}) + require.True(t, result.IsTruncated) + assert.Equal(t, "b", result.NextContinuationToken) +} + +func TestListObjectsV2ContinuationTokenRoundTrip(t *testing.T) { + for _, key := range []string{"a\x01b", "a\x1fb", "a\xffb", "base64:literal", "dir/file", "a b", "b"} { + token := encodeListContinuationToken(key) + assert.Equal(t, key, decodeListContinuationToken(token), "token %q", token) + } + for _, key := range []string{"b", "dir/file", "a b"} { + assert.Equal(t, key, encodeListContinuationToken(key)) + } + // Tokens written before the encoding existed are still bare keys. + assert.Equal(t, "dir/file", decodeListContinuationToken("dir/file")) +} + +func TestDecodeListContinuationTokenKeepsForeignTokensRaw(t *testing.T) { + // A prefix-looking token that does not decode is a key, not a token. + assert.Equal(t, "base64:%%%", decodeListContinuationToken("base64:%%%")) + // An empty payload is never something encodeListContinuationToken wrote. + assert.Equal(t, "base64:", decodeListContinuationToken("base64:")) + // A legacy bare token is a raw key even when its suffix is valid base64: + // the decoded key could have traveled bare, so the input never was an + // encoded token. + for _, legacy := range []string{"base64:Yw", "base64:YQ", "base64:cGFy", "base64:a2V5LXdpdGg"} { + assert.Equal(t, legacy, decodeListContinuationToken(legacy), "token %q", legacy) + } +} + +// Some tokens are ambiguous: "base64:AQ" is both a possible legacy bare key +// and the encoded form of "\x01". The encoded reading wins because a decoded +// key that cannot travel bare is a shape only encodeListContinuationToken +// writes — and a wrong raw reading skips entries, while a wrong encoded +// reading only re-lists them. +func TestDecodeListContinuationTokenResolvesCollisionEncoded(t *testing.T) { + assert.Equal(t, "\x01", decodeListContinuationToken("base64:AQ")) + assert.Equal(t, "\xff", decodeListContinuationToken("base64:_w")) + assert.Equal(t, "base64:AQ", decodeListContinuationToken("base64:YmFzZTY0OkFR")) +} + +type listV1TestPage struct { + XMLName xml.Name `xml:"ListBucketResult"` + IsTruncated bool `xml:"IsTruncated"` + Marker string `xml:"Marker"` + NextMarker string `xml:"NextMarker"` + Contents []struct { + Key string `xml:"Key"` + } `xml:"Contents"` +} + +func listV1Page(t *testing.T, s3a *S3ApiServer, query url.Values) listV1TestPage { + t.Helper() + r := httptest.NewRequest(http.MethodGet, "/test?"+query.Encode(), nil) + r = mux.SetURLVars(r, map[string]string{"bucket": "test"}) + w := httptest.NewRecorder() + s3a.ListObjectsV1Handler(w, r) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var page listV1TestPage + require.NoError(t, xml.Unmarshal(w.Body.Bytes(), &page), w.Body.String()) + return page +} + +// The V1 marker carries the same resume position as the V2 token, so it gets +// the same treatment: a page ending on an XML-unsafe key must still resume +// right after it. +func TestListObjectsV1MarkerWithXMLUnsafeKeys(t *testing.T) { + keys := []string{"a\x01b", "a\x02b", "a\x1fb", "a b", "b", "c"} + s3a := newListTokenTestServer(t, keys) + + var got []string + marker := "" + for page := 0; ; page++ { + require.Less(t, page, len(keys), "listing did not terminate") + query := url.Values{"max-keys": {"2"}, "encoding-type": {"url"}} + if marker != "" { + query.Set("marker", marker) + } + result := listV1Page(t, s3a, query) + for _, content := range result.Contents { + key, err := url.QueryUnescape(content.Key) + require.NoError(t, err) + got = append(got, key) + } + if !result.IsTruncated { + break + } + require.NotEmpty(t, result.NextMarker) + marker = result.NextMarker + } + require.Equal(t, keys, got) +} + +// A url-encoded marker is only a wire echo when re-escaping its unescaped +// form reproduces it: a real echo decodes, a raw key merely containing '%' +// or a malformed escape passes through untouched. +func TestDecodeMarkerEchoKeepsNonCanonicalRawKey(t *testing.T) { + assert.Equal(t, "a b", decodeMarkerEcho("a%20b", true)) + assert.Equal(t, "a%41", decodeMarkerEcho("a%2541", true)) + assert.Equal(t, "a%41", decodeMarkerEcho("a%41", true)) + assert.Equal(t, "a%2G", decodeMarkerEcho("a%2G", true)) + assert.Equal(t, "a%20b", decodeMarkerEcho("a%20b", false)) +} + +// Under encoding-type=url AWS encodes the StartAfter echo the same as the +// listed keys; a client resuming from the emitted form lands on the raw key. +func TestListObjectsV2UrlStartAfterEchoAndResume(t *testing.T) { + s3a := newListTokenTestServer(t, []string{"a b", "a b0", "a c", "b"}) + + result := listV2Page(t, s3a, url.Values{"list-type": {"2"}, "encoding-type": {"url"}, "start-after": {"a b"}}) + assert.Equal(t, urlPathEscape("a b"), result.StartAfter) + + var got []string + for _, content := range result.Contents { + key, err := url.QueryUnescape(content.Key) + require.NoError(t, err) + got = append(got, key) + } + assert.Equal(t, []string{"a b0", "a c", "b"}, got) + + // Echoing the emitted form back resumes at the same raw key. + result = listV2Page(t, s3a, url.Values{"list-type": {"2"}, "encoding-type": {"url"}, "start-after": {result.StartAfter}}) + got = nil + for _, content := range result.Contents { + key, err := url.QueryUnescape(content.Key) + require.NoError(t, err) + got = append(got, key) + } + assert.Equal(t, []string{"a b0", "a c", "b"}, got) +} + +// A url-encoded marker that is not the canonical urlPathEscape form of any key +// is the client's raw key, not an encoded echo: "a%41" must resume after that +// literal key instead of decoding to "aA" and skipping the keys between them. +func TestListObjectsV1UrlMarkerKeepsNonCanonicalRawKey(t *testing.T) { + s3a := newListTokenTestServer(t, []string{"a%41", "a%42", "aA", "b"}) + + result := listV1Page(t, s3a, url.Values{"encoding-type": {"url"}, "marker": {"a%41"}}) + var got []string + for _, content := range result.Contents { + key, err := url.QueryUnescape(content.Key) + require.NoError(t, err) + got = append(got, key) + } + assert.Equal(t, []string{"a%42", "aA", "b"}, got) +} + +// The echoed request marker carries the client's token, not the decoded key: +// echoing the raw key would mangle an XML-unsafe name to U+FFFD, and under +// encoding-type=url the echo takes the same escaped form as the listed keys. +func TestListObjectsV1MarkerEchoStaysXMLSafe(t *testing.T) { + s3a := newListTokenTestServer(t, []string{"a\x01b", "b", "c"}) + + token := encodeListContinuationToken("a\x01b") + result := listV1Page(t, s3a, url.Values{"max-keys": {"2"}, "marker": {token}}) + assert.Equal(t, token, result.Marker) + + result = listV1Page(t, s3a, url.Values{"max-keys": {"2"}, "marker": {"a b"}, "encoding-type": {"url"}}) + assert.Equal(t, urlPathEscape("a b"), result.Marker) +} + +// A raw-key token resuming at an XML-unsafe prefix must not lose keys when a +// sibling directory shares the key's leading bytes: the start window begins at +// the marker's first path segment and descends only into blocks that can still +// hold keys past it. +func TestListObjectsV2ResumeXMLUnsafePrefixWithSiblingDir(t *testing.T) { + filer := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/test/x": { + {Name: "a\x01", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}}, + listTokenTestFile("a\x01b"), + {Name: "a\x01b", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}}, + listTokenTestFile("a\x01b0"), + }, + "/buckets/test/x/a\x01": { + listTokenTestFile("b"), + }, + "/buckets/test/x/a\x01b": { + listTokenTestFile("0"), + }, + }} + s3a := newFailoverTestServer(t, startFakeFiler(t, filer)) + s3a.option.BucketsPath = "/buckets" + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set("test", &BucketConfig{Name: "test"}) + + result := listV2Page(t, s3a, url.Values{ + "list-type": {"2"}, + "encoding-type": {"url"}, + "prefix": {"x/a\x01b"}, + "max-keys": {"1"}, + "continuation-token": {"x/a\x01b"}, + }) + require.Len(t, result.Contents, 1) + got, err := url.QueryUnescape(result.Contents[0].Key) + require.NoError(t, err) + assert.Equal(t, "x/a\x01b/0", got) + assert.True(t, result.IsTruncated) +} diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index 945d7862d6..ddb349bdeb 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/weed/s3api/s3api_object_handlers_multipart.go @@ -260,11 +260,17 @@ func (s3a *S3ApiServer) ListMultipartUploadsHandler(w http.ResponseWriter, r *ht return } - prefix, keyMarker, uploadIDMarker, delimiter, maxUploads, encodingType := getBucketMultipartResources(r.URL.Query()) + prefix, multipartKeyMarker, uploadIDMarker, delimiter, maxUploads, encodingType := getBucketMultipartResources(r.URL.Query()) if maxUploads < 0 { s3err.WriteErrorResponse(w, r, s3err.ErrInvalidMaxUploads) return } + // The decode mirrors the response encoding: under encoding-type=url the + // markers go out url-escaped, otherwise they carry the token form. + keyMarker := decodeListContinuationToken(multipartKeyMarker) + if encodingType == s3.EncodingTypeUrl { + keyMarker = decodeEscapedEcho(multipartKeyMarker, true, urlQueryEscape) + } if keyMarker != "" { // Marker not common with prefix is not implemented. if !strings.HasPrefix(keyMarker, prefix) { diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index b4a7a4bed9..0e4cc950f3 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -448,12 +448,12 @@ func (s3a *S3ApiServer) splitIntoResult(combinedList []versionListItem, bucket, result := &S3ListObjectVersionsResult{ Name: bucket, Prefix: prefix, - KeyMarker: keyMarker, + KeyMarker: encodeListContinuationToken(keyMarker), VersionIdMarker: versionIdMarker, MaxKeys: maxKeys, Delimiter: delimiter, IsTruncated: isTruncated, - NextKeyMarker: nextKeyMarker, + NextKeyMarker: encodeListContinuationToken(nextKeyMarker), NextVersionIdMarker: nextVersionIdMarker, Entries: make([]VersionListEntry, 0, len(combinedList)), } @@ -1906,7 +1906,7 @@ func (s3a *S3ApiServer) ListObjectVersionsHandler(w http.ResponseWriter, r *http prefix := strings.TrimPrefix(originalPrefix, "/") // Note: prefix is used for filtering relative to bucket root, so no leading slash needed - keyMarker := query.Get("key-marker") + keyMarker := decodeListContinuationToken(query.Get("key-marker")) versionIdMarker := query.Get("version-id-marker") delimiter := query.Get("delimiter")