fix: end GetObjectAttributes part pagination correctly

GetObjectAttributes decided truncation by comparing the last returned
part number with the part count (cmd/object-handlers.go:720), which is
only a coincidence of contiguous numbering. Sparse parts 1/3 reported a
complete page as truncated with a marker that loops, and parts 1/3/5
with max-parts=1 stopped after part 3 and silently dropped part 5. Set
IsTruncated in the break that proves an eligible part was left
unreturned, and zero NextPartNumberMarker when the listing is complete,
as ListObjectParts already does. Also reject negative x-amz-max-parts
and x-amz-part-number-marker in getAndValidateAttributesOpts with the
same API errors ListObjectParts uses, instead of answering an invalid
request with an empty parts listing; an absent or zero max-parts still
means the default page size.

Tests: TestAPIGetObjectAttributesPartsPagination (sparse 1/3/5 and
contiguous 1/2 walks on ErasureSD and Erasure),
TestGetAndValidateAttributesOptsPartsRange, and the sparse variants of
TestAPIGetObjectAttributesMultipartLogicalPartSize. Compatibility: no
field is added or removed; IsTruncated and NextPartNumberMarker change
only where they were wrong, and negative pagination values that no SDK
sends now fail fast.

Fixes pgsty/silo#115

Signed-off-by: Feng Ruohang <rh@vonng.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe

cmd/object-api-options.go

@@ -184,6 +184,16 @@ func getAndValidateAttributesOpts(ctx context.Context, w http.ResponseWriter, r
 		return opts, valid
 	}
 
+	// Reject out-of-range page sizes as ListObjectParts does, instead of
+	// answering an invalid request with an empty parts listing.
+	if opts.MaxParts < 0 {
+		apiErr = errorCodes.ToAPIErr(ErrInvalidMaxParts)
+		argumentName = strings.ToLower(xhttp.AmzMaxParts)
+		argumentValue = r.Header.Get(xhttp.AmzMaxParts)
+		valid = false
+		return opts, valid
+	}
+
 	if opts.MaxParts == 0 {
 		opts.MaxParts = maxPartsList
 	}
@@ -196,6 +206,14 @@ func getAndValidateAttributesOpts(ctx context.Context, w http.ResponseWriter, r
 		return opts, valid
 	}
 
+	if opts.PartNumberMarker < 0 {
+		apiErr = errorCodes.ToAPIErr(ErrInvalidPartNumberMarker)
+		argumentName = strings.ToLower(xhttp.AmzPartNumberMarker)
+		argumentValue = r.Header.Get(xhttp.AmzPartNumberMarker)
+		valid = false
+		return opts, valid
+	}
+
 	opts.ObjectAttributes = parseObjectAttributes(r.Header)
 	if len(opts.ObjectAttributes) < 1 {
 		apiErr = errorCodes.ToAPIErr(ErrInvalidAttributeName)

cmd/object-api-options_test.go

@@ -18,9 +18,11 @@
 package cmd
 
 import (
+	"encoding/xml"
 	"net/http"
 	"net/http/httptest"
 	"reflect"
+	"strings"
 	"testing"
 
 	xhttp "github.com/minio/minio/internal/http"
@@ -76,3 +78,109 @@ func TestGetAndValidateAttributesOpts(t *testing.T) {
 		})
 	}
 }
+
+// TestGetAndValidateAttributesOptsPartsRange asserts that GetObjectAttributes
+// range checks the ObjectParts pagination headers the way ListObjectParts
+// does: a negative value is rejected with the same API error, an absent or
+// zero x-amz-max-parts means the default page size, and in-range values are
+// passed through unchanged.
+func TestGetAndValidateAttributesOptsPartsRange(t *testing.T) {
+	globalBucketVersioningSys = &BucketVersioningSys{}
+	bucket := minioMetaBucket
+	ctx := t.Context()
+
+	testCases := []struct {
+		name         string
+		maxParts     string
+		marker       string
+		wantValid    bool
+		wantErr      APIErrorCode
+		wantArgument string
+		wantValue    string
+		wantMaxParts int
+		wantMarker   int
+	}{
+		{
+			name:         "defaults",
+			wantValid:    true,
+			wantMaxParts: maxPartsList,
+		},
+		{
+			name:         "zero max-parts means the default page size",
+			maxParts:     "0",
+			wantValid:    true,
+			wantMaxParts: maxPartsList,
+		},
+		{
+			name:         "in range values pass through",
+			maxParts:     "10",
+			marker:       "3",
+			wantValid:    true,
+			wantMaxParts: 10,
+			wantMarker:   3,
+		},
+		{
+			name:         "negative max-parts is rejected",
+			maxParts:     "-1",
+			wantValid:    false,
+			wantErr:      ErrInvalidMaxParts,
+			wantArgument: strings.ToLower(xhttp.AmzMaxParts),
+			wantValue:    "-1",
+		},
+		{
+			name:         "negative part-number-marker is rejected",
+			marker:       "-1",
+			wantValid:    false,
+			wantErr:      ErrInvalidPartNumberMarker,
+			wantArgument: strings.ToLower(xhttp.AmzPartNumberMarker),
+			wantValue:    "-1",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			rec := httptest.NewRecorder()
+			req := httptest.NewRequest(http.MethodGet, "/testbucket/testobject?attributes", nil)
+			req.Header.Set(xhttp.AmzObjectAttributes, "ObjectParts")
+			if testCase.maxParts != "" {
+				req.Header.Set(xhttp.AmzMaxParts, testCase.maxParts)
+			}
+			if testCase.marker != "" {
+				req.Header.Set(xhttp.AmzPartNumberMarker, testCase.marker)
+			}
+
+			opts, valid := getAndValidateAttributesOpts(ctx, rec, req, bucket, "testobject")
+			if valid != testCase.wantValid {
+				t.Fatalf("want valid %v, got %v (%s)", testCase.wantValid, valid, rec.Body.String())
+			}
+
+			if testCase.wantValid {
+				if opts.MaxParts != testCase.wantMaxParts {
+					t.Errorf("want MaxParts %d, got %d", testCase.wantMaxParts, opts.MaxParts)
+				}
+				if opts.PartNumberMarker != testCase.wantMarker {
+					t.Errorf("want PartNumberMarker %d, got %d", testCase.wantMarker, opts.PartNumberMarker)
+				}
+				return
+			}
+
+			wantErr := errorCodes.ToAPIErr(testCase.wantErr)
+			if rec.Code != wantErr.HTTPStatusCode {
+				t.Errorf("want HTTP status %d, got %d", wantErr.HTTPStatusCode, rec.Code)
+			}
+			var errResp objectAttributesErrorResponse
+			if err := xml.Unmarshal(rec.Body.Bytes(), &errResp); err != nil {
+				t.Fatalf("decode error response: %v (%s)", err, rec.Body.String())
+			}
+			if errResp.Code != wantErr.Code || errResp.Message != wantErr.Description {
+				t.Errorf("want error %s/%q, got %s/%q", wantErr.Code, wantErr.Description, errResp.Code, errResp.Message)
+			}
+			if errResp.ArgumentName == nil || *errResp.ArgumentName != testCase.wantArgument {
+				t.Errorf("want ArgumentName %q, got %v", testCase.wantArgument, errResp.ArgumentName)
+			}
+			if errResp.ArgumentValue == nil || *errResp.ArgumentValue != testCase.wantValue {
+				t.Errorf("want ArgumentValue %q, got %v", testCase.wantValue, errResp.ArgumentValue)
+			}
+		})
+	}
+}

cmd/object-attributes-parts-pagination_test.go

@@ -0,0 +1,185 @@
+// Copyright (c) 2015-2026 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmd
+
+import (
+	"bytes"
+	"encoding/xml"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"testing"
+
+	"github.com/minio/minio/internal/auth"
+	xhttp "github.com/minio/minio/internal/http"
+)
+
+type attributesPartsPage struct {
+	ObjectParts struct {
+		IsTruncated          bool
+		MaxParts             int
+		NextPartNumberMarker int
+		PartNumberMarker     int
+		PartsCount           int
+		Parts                []struct {
+			PartNumber int
+			Size       int64
+		} `xml:"Part"`
+	}
+}
+
+// TestAPIGetObjectAttributesPartsPagination asserts that ObjectParts pagination
+// terminates for sparse part numbers: truncation is decided by whether parts
+// remain, not by comparing the last part number with the part count.
+func TestAPIGetObjectAttributesPartsPagination(t *testing.T) {
+	defer DetectTestLeak(t)()
+	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+		t:          t,
+		objAPITest: testAPIGetObjectAttributesPartsPagination,
+	})
+}
+
+func testAPIGetObjectAttributesPartsPagination(_ ObjectLayer, instanceType, bucketName string,
+	apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
+) {
+	signedRequest := func(method, target string, body []byte, headers map[string]string) *httptest.ResponseRecorder {
+		t.Helper()
+		req, err := newTestSignedRequestV4(method, target, int64(len(body)), bytes.NewReader(body),
+			credentials.AccessKey, credentials.SecretKey, headers)
+		if err != nil {
+			t.Fatal(err)
+		}
+		rec := httptest.NewRecorder()
+		apiRouter.ServeHTTP(rec, req)
+		return rec
+	}
+
+	const partSize = 5 * 1024 * 1024
+
+	// upload completes a multipart object with the given part numbers. Part
+	// numbers must be strictly increasing, but gaps are allowed, so the last
+	// part number need not equal the number of parts.
+	upload := func(object string, partNumbers []int) {
+		t.Helper()
+		initRec := signedRequest(http.MethodPost, getNewMultipartURL("", bucketName, object), nil, nil)
+		if initRec.Code != http.StatusOK {
+			t.Fatalf("%s INIT: %d %s", instanceType, initRec.Code, initRec.Body.String())
+		}
+		var initiated struct {
+			UploadID string `xml:"UploadId"`
+		}
+		if err := xml.Unmarshal(initRec.Body.Bytes(), &initiated); err != nil {
+			t.Fatal(err)
+		}
+		var complete bytes.Buffer
+		complete.WriteString("<CompleteMultipartUpload>")
+		for i, number := range partNumbers {
+			body := bytes.Repeat([]byte("abcd"), partSize/4)
+			if i == len(partNumbers)-1 {
+				body = bytes.Repeat([]byte("12345"), 103)
+			}
+			put := signedRequest(http.MethodPut,
+				getPutObjectPartURL("", bucketName, object, initiated.UploadID, strconv.Itoa(number)), body, nil)
+			if put.Code != http.StatusOK {
+				t.Fatalf("%s PART %d: %d %s", instanceType, number, put.Code, put.Body.String())
+			}
+			fmt.Fprintf(&complete, "<Part><PartNumber>%d</PartNumber><ETag>%s</ETag></Part>",
+				number, put.Header()[xhttp.ETag][0])
+		}
+		complete.WriteString("</CompleteMultipartUpload>")
+		finish := signedRequest(http.MethodPost,
+			getCompleteMultipartUploadURL("", bucketName, object, initiated.UploadID), complete.Bytes(), nil)
+		if finish.Code != http.StatusOK {
+			t.Fatalf("%s COMPLETE: %d %s", instanceType, finish.Code, finish.Body.String())
+		}
+	}
+
+	attributes := func(object string, marker, maxParts int) attributesPartsPage {
+		t.Helper()
+		headers := map[string]string{xhttp.AmzObjectAttributes: "ObjectParts"}
+		if marker > 0 {
+			headers[xhttp.AmzPartNumberMarker] = strconv.Itoa(marker)
+		}
+		if maxParts > 0 {
+			headers[xhttp.AmzMaxParts] = strconv.Itoa(maxParts)
+		}
+		rec := signedRequest(http.MethodGet, getGetObjectURL("", bucketName, object)+"?attributes", nil, headers)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("%s ATTRIBUTES marker=%d max=%d: %d %s", instanceType, marker, maxParts, rec.Code, rec.Body.String())
+		}
+		var page attributesPartsPage
+		if err := xml.Unmarshal(rec.Body.Bytes(), &page); err != nil {
+			t.Fatal(err)
+		}
+		return page
+	}
+
+	check := func(name string, page attributesPartsPage, wantParts []int, wantTruncated bool, wantNext, wantCount int) {
+		t.Helper()
+		got := make([]int, 0, len(page.ObjectParts.Parts))
+		for _, p := range page.ObjectParts.Parts {
+			got = append(got, p.PartNumber)
+		}
+		if fmt.Sprint(got) != fmt.Sprint(wantParts) {
+			t.Errorf("%s %s: parts=%v want=%v", instanceType, name, got, wantParts)
+		}
+		if page.ObjectParts.IsTruncated != wantTruncated {
+			t.Errorf("%s %s: IsTruncated=%v want=%v", instanceType, name, page.ObjectParts.IsTruncated, wantTruncated)
+		}
+		if page.ObjectParts.NextPartNumberMarker != wantNext {
+			t.Errorf("%s %s: NextPartNumberMarker=%d want=%d", instanceType, name,
+				page.ObjectParts.NextPartNumberMarker, wantNext)
+		}
+		if page.ObjectParts.PartsCount != wantCount {
+			t.Errorf("%s %s: PartsCount=%d want=%d", instanceType, name, page.ObjectParts.PartsCount, wantCount)
+		}
+	}
+
+	sparse := "review/attributes-pagination-sparse"
+	upload(sparse, []int{1, 3, 5})
+
+	// A single page holding every sparse part is complete.
+	check("sparse full page", attributes(sparse, 0, 0), []int{1, 3, 5}, false, 0, 3)
+
+	// One part per page walks 1, 3, 5 and stops. Page 2 returns part 3,
+	// whose number equals the part count, and must still be truncated:
+	// the old comparison declared that page final and silently dropped
+	// part 5.
+	check("sparse max-parts=1 page 1", attributes(sparse, 0, 1), []int{1}, true, 1, 3)
+	check("sparse max-parts=1 page 2", attributes(sparse, 1, 1), []int{3}, true, 3, 3)
+	check("sparse max-parts=1 page 3", attributes(sparse, 3, 1), []int{5}, false, 0, 3)
+
+	// A page that exactly holds the remainder is not truncated, so no
+	// empty trailing page is requested.
+	check("sparse max-parts=2 page 2", attributes(sparse, 1, 2), []int{3, 5}, false, 0, 3)
+
+	// A marker at or past the last part ends the listing instead of
+	// looping back through NextPartNumberMarker=0.
+	check("sparse marker at last part", attributes(sparse, 5, 0), nil, false, 0, 3)
+	check("sparse marker past last part", attributes(sparse, 9, 0), nil, false, 0, 3)
+
+	// Contiguous numbering is affected too: the empty page past the end
+	// used to report IsTruncated=true with NextPartNumberMarker=0.
+	contiguous := "review/attributes-pagination-contiguous"
+	upload(contiguous, []int{1, 2})
+	check("contiguous full page", attributes(contiguous, 0, 0), []int{1, 2}, false, 0, 2)
+	check("contiguous max-parts=1 page 1", attributes(contiguous, 0, 1), []int{1}, true, 1, 2)
+	check("contiguous max-parts=1 page 2", attributes(contiguous, 1, 1), []int{2}, false, 0, 2)
+	check("contiguous marker at last part", attributes(contiguous, 2, 0), nil, false, 0, 2)
+}

cmd/object-handlers.go

@@ -716,6 +716,9 @@ func (api objectAPIHandlers) getObjectAttributesHandler(ctx context.Context, obj
 				}
 
 				if len(OA.ObjectParts.Parts) == opts.MaxParts {
+					// This page is full and at least one more part
+					// is still pending, so the listing is truncated.
+					OA.ObjectParts.IsTruncated = true
 					break
 				}
 
@@ -758,8 +761,11 @@ func (api objectAPIHandlers) getObjectAttributesHandler(ctx context.Context, obj
 			}
 		}
 
-		if OA.ObjectParts.NextPartNumberMarker != partsLength {
-			OA.ObjectParts.IsTruncated = true
+		// Part numbers may be sparse, so they cannot be compared against
+		// the part count. NextPartNumberMarker only carries a continuation
+		// token for a truncated listing, as in ListObjectParts.
+		if !OA.ObjectParts.IsTruncated {
+			OA.ObjectParts.NextPartNumberMarker = 0
 		}
 	}