From a014c7c1b28fd8017156adc8c00657c21f9acb1b Mon Sep 17 00:00:00 2001 From: LuisFigueroaG Date: Wed, 7 Oct 2026 11:23:45 -0300 Subject: [PATCH 1/2] fix: report the upload, part and ETag in InvalidPart for a malformed part ETag CompleteMultipartUpload with a part ETag that is not a hex MD5 returned a bare InvalidPart error, without the UploadId, PartNumber and ETag elements. S3 includes them, and the gateway already does for a part that is missing or whose ETag doesn't match. Pass the upload ID to ComputeMultipartETagFromPartETags so it can return the detailed InvalidPart error for the offending part, and enable the REST test that covers it. Fixes #1008 --- backend/common.go | 7 +-- backend/common_test.go | 51 ++++++++++++++++++++ backend/posix/posix.go | 2 +- tests/integration/CompleteMultipartUpload.go | 40 +++++++++++++++ tests/integration/group-tests.go | 2 + tests/test_rest_multipart.sh | 3 -- 6 files changed, 98 insertions(+), 7 deletions(-) diff --git a/backend/common.go b/backend/common.go index 3fcf26dc..f7bf15aa 100644 --- a/backend/common.go +++ b/backend/common.go @@ -425,8 +425,9 @@ func isValidTagComponent(str string) bool { } // ComputeMultipartETagFromPartETags computes the S3 multipart ETag -// ("-") from the completed-part ETags. -func ComputeMultipartETagFromPartETags(parts []types.CompletedPart) (string, error) { +// ("-") from the completed-part ETags. uploadID is only +// used to report the upload in an InvalidPart error. +func ComputeMultipartETagFromPartETags(uploadID string, parts []types.CompletedPart) (string, error) { var partsEtagBytes []byte for _, part := range parts { if part.ETag == nil { @@ -437,7 +438,7 @@ func ComputeMultipartETagFromPartETags(parts []types.CompletedPart) (string, err } bts, err := getEtagBytes(*part.ETag) if err != nil { - return "", s3err.GetAPIError(s3err.ErrInvalidPart) + return "", s3err.GetInvalidPartErr(uploadID, *part.PartNumber, *part.ETag) } partsEtagBytes = append(partsEtagBytes, bts...) } diff --git a/backend/common_test.go b/backend/common_test.go index af398c56..eb0213d8 100644 --- a/backend/common_test.go +++ b/backend/common_test.go @@ -16,11 +16,15 @@ package backend import ( "bytes" + "crypto/md5" + "encoding/hex" "encoding/json" "errors" + "fmt" "reflect" "testing" + "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/versity/versitygw/s3err" ) @@ -330,3 +334,50 @@ func TestParseCopySource(t *testing.T) { }) } } + +func TestComputeMultipartETagFromPartETags(t *testing.T) { + etag1 := "5d41402abc4b2a76b9719d911017c592" + etag2 := "7d793037a0760186574b0282f2f435e7" + raw1, _ := hex.DecodeString(etag1) + raw2, _ := hex.DecodeString(etag2) + sum := md5.Sum(append(raw1, raw2...)) + wantETag := fmt.Sprintf("\"%s-2\"", hex.EncodeToString(sum[:])) + + part := func(n int32, etag string) types.CompletedPart { + return types.CompletedPart{PartNumber: &n, ETag: &etag} + } + + tests := []struct { + name string + parts []types.CompletedPart + wantETag string + wantErr error + }{ + { + name: "quoted and unquoted part etags", + parts: []types.CompletedPart{part(1, "\""+etag1+"\""), part(2, etag2)}, + wantETag: wantETag, + }, + { + name: "missing part etag", + parts: []types.CompletedPart{{PartNumber: new(int32)}}, + wantErr: s3err.GetAPIError(s3err.ErrMalformedXML), + }, + { + name: "invalid part etag reports the upload, part and etag", + parts: []types.CompletedPart{part(1, etag1), part(2, "\"invalidETag\"")}, + wantErr: s3err.GetInvalidPartErr("upload-id", 2, "\"invalidETag\""), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + etag, err := ComputeMultipartETagFromPartETags("upload-id", tt.parts) + if !reflect.DeepEqual(err, tt.wantErr) { + t.Fatalf("expected error %#v, got %#v", tt.wantErr, err) + } + if etag != tt.wantETag { + t.Fatalf("expected etag %q, got %q", tt.wantETag, etag) + } + }) + } +} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index a2a39b9e..ace6a132 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -2356,7 +2356,7 @@ func (p *Posix) CompleteMultipartUploadWithCopy(ctx context.Context, input *s3.C // Compute the default multipart ETag token used for claim naming. // In standard mode this is the S3-compatible multipart MD5 ETag; in // dataIntegrityEtag mode it may fall back to a deterministic claim token. - multipartClaimToken, err := backend.ComputeMultipartETagFromPartETags(parts) + multipartClaimToken, err := backend.ComputeMultipartETagFromPartETags(uploadID, parts) if err != nil { if !p.dataIntegrityEtag { return res, "", err diff --git a/tests/integration/CompleteMultipartUpload.go b/tests/integration/CompleteMultipartUpload.go index caabbff7..59a075f8 100644 --- a/tests/integration/CompleteMultipartUpload.go +++ b/tests/integration/CompleteMultipartUpload.go @@ -190,6 +190,46 @@ func CompleteMultipartUpload_invalid_ETag(s *S3Conf) error { return nil }) } + +func CompleteMultipartUpload_invalid_ETag_error_fields(s *S3Conf) error { + testName := "CompleteMultipartUpload_invalid_ETag_error_fields" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + out, err := createMp(s3client, bucket, obj) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.UploadPart(ctx, &s3.UploadPartInput{ + Bucket: &bucket, + Key: &obj, + UploadId: out.UploadId, + PartNumber: getPtr(int32(1)), + }) + cancel() + if err != nil { + return err + } + + // the sdk doesn't expose the InvalidPart error fields, so the + // request is signed and sent by hand + body := []byte(`"invalidETag"1`) + req, err := createSignedReq(http.MethodPost, s.endpoint, + fmt.Sprintf("%v/%v?uploadId=%v", bucket, obj, *out.UploadId), + s.awsID, s.awsSecret, "s3", s.awsRegion, "", body, time.Now(), nil) + if err != nil { + return err + } + + resp, err := s.httpClient.Do(req) + if err != nil { + return err + } + + return checkHTTPResponseApiErr(resp, s3err.GetInvalidPartErr(*out.UploadId, 1, `"invalidETag"`)) + }) +} func CompleteMultipartUpload_invalid_checksum_type(s *S3Conf) error { testName := "CompleteMultipartUpload_invalid_checksum_type" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 328683b1..6ff339c5 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -585,6 +585,7 @@ func TestCompleteMultipartUpload(ts *TestState) { ts.Run(CompleteMultipartUpload_default_content_type) ts.Run(CompleteMultipartUpload_overwrite_resets_attributes) ts.Run(CompleteMultipartUpload_invalid_ETag) + ts.Run(CompleteMultipartUpload_invalid_ETag_error_fields) ts.Run(CompleteMultipartUpload_small_upload_size) ts.Run(CompleteMultipartUpload_empty_parts) ts.Run(CompleteMultipartUpload_missing_part_fields) @@ -3322,6 +3323,7 @@ func GetIntTests() IntTests { "CompleteMultipartUpload_default_content_type": CompleteMultipartUpload_default_content_type, "CompleteMultipartUpload_overwrite_resets_attributes": CompleteMultipartUpload_overwrite_resets_attributes, "CompleteMultipartUpload_invalid_ETag": CompleteMultipartUpload_invalid_ETag, + "CompleteMultipartUpload_invalid_ETag_error_fields": CompleteMultipartUpload_invalid_ETag_error_fields, "CompleteMultipartUpload_small_upload_size": CompleteMultipartUpload_small_upload_size, "CompleteMultipartUpload_empty_parts": CompleteMultipartUpload_empty_parts, "CompleteMultipartUpload_missing_part_fields": CompleteMultipartUpload_missing_part_fields, diff --git a/tests/test_rest_multipart.sh b/tests/test_rest_multipart.sh index 27bb5ec1..bcbfccc5 100755 --- a/tests/test_rest_multipart.sh +++ b/tests/test_rest_multipart.sh @@ -65,9 +65,6 @@ source ./tests/drivers/upload_part/upload_part_rest.sh # tags: curl,multipart,CompleteMultipartUpload,invalid-header,ETag @test "REST - complete upload - invalid part" { - if [ "$DIRECT" != "true" ]; then - skip "https://github.com/versity/versitygw/issues/1008" - fi run get_bucket_name "$BUCKET_ONE_NAME" assert_success bucket_name="$output" From 6f4cf9b4af80bc10c874dab06816faff4b552aa9 Mon Sep 17 00:00:00 2001 From: LuisFigueroaG Date: Thu, 8 Oct 2026 21:29:06 -0300 Subject: [PATCH 2/2] test: upload a non-empty part in the InvalidPart error fields test The azure backend tracks zero-byte parts separately and reports its own block id as the ETag when the client ETag doesn't match, so the test failed against azurite. Upload some data so the part is a regular block, and send the ETag unquoted so both backends echo it back as sent. --- tests/integration/CompleteMultipartUpload.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/CompleteMultipartUpload.go b/tests/integration/CompleteMultipartUpload.go index 59a075f8..4ffa5be4 100644 --- a/tests/integration/CompleteMultipartUpload.go +++ b/tests/integration/CompleteMultipartUpload.go @@ -206,6 +206,7 @@ func CompleteMultipartUpload_invalid_ETag_error_fields(s *S3Conf) error { Key: &obj, UploadId: out.UploadId, PartNumber: getPtr(int32(1)), + Body: bytes.NewReader([]byte("dummy")), }) cancel() if err != nil { @@ -214,7 +215,7 @@ func CompleteMultipartUpload_invalid_ETag_error_fields(s *S3Conf) error { // the sdk doesn't expose the InvalidPart error fields, so the // request is signed and sent by hand - body := []byte(`"invalidETag"1`) + body := []byte(`invalidETag1`) req, err := createSignedReq(http.MethodPost, s.endpoint, fmt.Sprintf("%v/%v?uploadId=%v", bucket, obj, *out.UploadId), s.awsID, s.awsSecret, "s3", s.awsRegion, "", body, time.Now(), nil) @@ -227,7 +228,7 @@ func CompleteMultipartUpload_invalid_ETag_error_fields(s *S3Conf) error { return err } - return checkHTTPResponseApiErr(resp, s3err.GetInvalidPartErr(*out.UploadId, 1, `"invalidETag"`)) + return checkHTTPResponseApiErr(resp, s3err.GetInvalidPartErr(*out.UploadId, 1, "invalidETag")) }) } func CompleteMultipartUpload_invalid_checksum_type(s *S3Conf) error {