s3api: validate ListObjectVersions max-keys and honor max-keys=0 (#11703)

* fix(s3api): accept max-keys up to 2147483647 and cap list pages at 1000

* add tests for ListObjects max-keys range and 1000 cap

* refactor(s3api): return uint16 from parseMaxKeys to avoid narrowing casts

* fix(s3api): validate ListObjectVersions max-keys and honor max-keys=0

* add test for ListObjectVersions max-keys=0

* fix(s3api): widen uint16 max-keys for ListObjectVersions

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.

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")

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)
+}

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)