diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 0af321bc5..3cc7b1e49 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/weed/s3api/s3api_object_handlers_list.go @@ -171,10 +171,7 @@ func (s3a *S3ApiServer) ListObjectsV1Handler(w http.ResponseWriter, r *http.Requ return } - if maxKeys < 0 { - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidMaxKeys) - return - } + // maxKeys is uint16 here; negative values are rejected during parsing. // AWS S3 compatibility: allow-unordered cannot be used with delimiter if allowUnordered && delimiter != "" { @@ -192,7 +189,7 @@ func (s3a *S3ApiServer) ListObjectsV1Handler(w http.ResponseWriter, r *http.Requ marker: marker, requestMarker: requestMarker, delimiter: delimiter, - maxKeys: uint16(maxKeys), + maxKeys: maxKeys, encodingTypeUrl: encodingTypeUrl, fetchOwner: true, }) @@ -1313,16 +1310,9 @@ func getListObjectsV2Args(values url.Values) (prefix, startAfter, delimiter stri startAfter = values.Get("start-after") delimiter = values.Get("delimiter") encodingTypeUrl = values.Get("encoding-type") == s3.EncodingTypeUrl - if values.Get("max-keys") != "" { - if maxKeys, err := strconv.ParseUint(values.Get("max-keys"), 10, 16); err == nil { - maxkeys = uint16(maxKeys) - } else { - // Invalid max-keys value (non-numeric) - errCode = s3err.ErrInvalidMaxKeys - return - } - } else { - maxkeys = maxObjectListSizeLimit + maxkeys, errCode = parseMaxKeys(values.Get("max-keys")) + if errCode != s3err.ErrNone { + return } fetchOwner = values.Get("fetch-owner") == "true" allowUnordered = values.Get("allow-unordered") == "true" @@ -1330,27 +1320,37 @@ func getListObjectsV2Args(values url.Values) (prefix, startAfter, delimiter stri return } -func getListObjectsV1Args(values url.Values) (prefix, marker, delimiter string, encodingTypeUrl bool, maxkeys int16, allowUnordered bool, errCode s3err.ErrorCode) { +func getListObjectsV1Args(values url.Values) (prefix, marker, delimiter string, encodingTypeUrl bool, maxkeys uint16, allowUnordered bool, errCode s3err.ErrorCode) { prefix = values.Get("prefix") marker = values.Get("marker") delimiter = values.Get("delimiter") encodingTypeUrl = values.Get("encoding-type") == "url" - if values.Get("max-keys") != "" { - if maxKeys, err := strconv.ParseInt(values.Get("max-keys"), 10, 16); err == nil { - maxkeys = int16(maxKeys) - } else { - // Invalid max-keys value (non-numeric) - errCode = s3err.ErrInvalidMaxKeys - return - } - } else { - maxkeys = maxObjectListSizeLimit + maxkeys, errCode = parseMaxKeys(values.Get("max-keys")) + if errCode != s3err.ErrNone { + return } allowUnordered = values.Get("allow-unordered") == "true" errCode = s3err.ErrNone return } +// parseMaxKeys parses the max-keys query parameter. Like AWS S3, any integer in +// [0, 2147483647] is accepted and the page size is capped at maxObjectListSizeLimit; +// an empty value means the default limit, anything else is ErrInvalidMaxKeys. +func parseMaxKeys(value string) (uint16, s3err.ErrorCode) { + if value == "" { + return maxObjectListSizeLimit, s3err.ErrNone + } + maxKeys, err := strconv.ParseInt(value, 10, 32) + if err != nil || maxKeys < 0 { + return 0, s3err.ErrInvalidMaxKeys + } + if maxKeys > maxObjectListSizeLimit { + return maxObjectListSizeLimit, s3err.ErrNone + } + return uint16(maxKeys), s3err.ErrNone +} + // compareWithDelimiter compares two strings for sorting, treating the delimiter character // as having lower precedence than other characters to match AWS S3 behavior. // For example, with delimiter '/', 'foo/' should come before 'foo+1/' even though '+' < '/' in ASCII. diff --git a/weed/s3api/s3api_object_handlers_list_test.go b/weed/s3api/s3api_object_handlers_list_test.go index ecbde4671..998971f6d 100644 --- a/weed/s3api/s3api_object_handlers_list_test.go +++ b/weed/s3api/s3api_object_handlers_list_test.go @@ -667,12 +667,48 @@ func TestMaxKeysParameterValidation(t *testing.T) { assert.Equal(t, s3err.ErrInvalidMaxKeys, errCode, "non-numeric max-keys should return ErrInvalidMaxKeys") }) + t.Run("out of range max-keys values should return error", func(t *testing.T) { + for _, v := range []string{"-1", "2147483648", "1.5", " 10"} { + values := map[string][]string{ + "max-keys": {v}, + } + _, _, _, _, _, _, errCode := getListObjectsV1Args(values) + assert.Equal(t, s3err.ErrInvalidMaxKeys, errCode, "V1 max-keys=%q should return ErrInvalidMaxKeys", v) + + _, _, _, _, _, _, _, _, errCode = getListObjectsV2Args(values) + assert.Equal(t, s3err.ErrInvalidMaxKeys, errCode, "V2 max-keys=%q should return ErrInvalidMaxKeys", v) + } + }) + + t.Run("max-keys up to 2147483647 is accepted and capped at 1000", func(t *testing.T) { + for v, want := range map[string]int{ + "0": 0, + "999": 999, + "1000": 1000, + "1001": 1000, + "32768": 1000, + "65536": 1000, + "2147483647": 1000, + } { + values := map[string][]string{ + "max-keys": {v}, + } + _, _, _, _, maxkeys, _, errCode := getListObjectsV1Args(values) + assert.Equal(t, s3err.ErrNone, errCode, "V1 max-keys=%q should not return error", v) + assert.Equal(t, uint16(want), maxkeys, "V1 max-keys=%q", v) + + _, _, _, _, _, _, maxkeys2, _, errCode := getListObjectsV2Args(values) + assert.Equal(t, s3err.ErrNone, errCode, "V2 max-keys=%q should not return error", v) + assert.Equal(t, uint16(want), maxkeys2, "V2 max-keys=%q", v) + } + }) + t.Run("empty max-keys should use default", func(t *testing.T) { // Test empty max-keys values := map[string][]string{} _, _, _, _, maxkeys, _, errCode := getListObjectsV1Args(values) assert.Equal(t, s3err.ErrNone, errCode, "empty max-keys should not return error") - assert.Equal(t, int16(1000), maxkeys, "empty max-keys should use default value") + assert.Equal(t, uint16(1000), maxkeys, "empty max-keys should use default value") _, _, _, _, _, _, maxkeys2, _, errCode := getListObjectsV2Args(values) assert.Equal(t, s3err.ErrNone, errCode, "empty max-keys should not return error") diff --git a/weed/s3api/s3api_object_handlers_list_versioned_test.go b/weed/s3api/s3api_object_handlers_list_versioned_test.go index 3bb97a342..51a01ce89 100644 --- a/weed/s3api/s3api_object_handlers_list_versioned_test.go +++ b/weed/s3api/s3api_object_handlers_list_versioned_test.go @@ -739,3 +739,19 @@ func TestVersionedListSkipsEchoedVersionsMarker(t *testing.T) { assert.NoError(t, err) assert.Equal(t, []string{"file-2"}, seen, "the echoed marker entry should not be returned") } + +// TestListObjectVersionsMaxKeysZero verifies that max-keys=0 returns an empty, +// non-truncated page without listing the bucket, matching ListObjects. +func TestListObjectVersionsMaxKeysZero(t *testing.T) { + s3a := &S3ApiServer{} + result, err := s3a.listObjectVersions("bucket", "prefix/", "key-marker", "version-marker", "/", 0) + assert.NoError(t, err) + assert.Empty(t, result.Entries) + assert.False(t, result.IsTruncated) + assert.Equal(t, 0, result.MaxKeys) + assert.Equal(t, "", result.NextKeyMarker) + assert.Equal(t, "", result.NextVersionIdMarker) + assert.Equal(t, "key-marker", result.KeyMarker) + assert.Equal(t, "version-marker", result.VersionIdMarker) + assert.Equal(t, "/", result.Delimiter) +} diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index 8ef46b7cc..b4a7a4bed 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -328,6 +328,10 @@ func (s3a *S3ApiServer) listObjectVersions(bucket, prefix, keyMarker, versionIdM if maxKeys > 1000 { maxKeys = 1000 } + // max-keys=0 asks for an empty, non-truncated page; no need to scan the bucket + if maxKeys <= 0 { + return s3a.splitIntoResult(nil, bucket, prefix, keyMarker, versionIdMarker, delimiter, 0, false, "", ""), nil + } // Pre-allocate with capacity for maxKeys+1 to reduce reallocations // The extra 1 is for truncation detection allVersions := make([]interface{}, 0, maxKeys+1) @@ -1906,16 +1910,14 @@ func (s3a *S3ApiServer) ListObjectVersionsHandler(w http.ResponseWriter, r *http versionIdMarker := query.Get("version-id-marker") delimiter := query.Get("delimiter") - maxKeysStr := query.Get("max-keys") - maxKeys := 1000 - if maxKeysStr != "" { - if mk, err := strconv.Atoi(maxKeysStr); err == nil && mk > 0 { - maxKeys = mk - } + maxKeys, errCode := parseMaxKeys(query.Get("max-keys")) + if errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return } // List versions - result, err := s3a.listObjectVersions(bucket, prefix, keyMarker, versionIdMarker, delimiter, maxKeys) + result, err := s3a.listObjectVersions(bucket, prefix, keyMarker, versionIdMarker, delimiter, int(maxKeys)) if err != nil { glog.Errorf("ListObjectVersionsHandler: %v", err) s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)