From 12f22838e25e88a2dd3760aa0f5be884ed34536e Mon Sep 17 00:00:00 2001 From: Ed Espino Date: Tue, 30 Jun 2026 11:51:38 -0700 Subject: [PATCH] make azure multipart part etags quoted and consistent quote part etags across all azure multipart surfaces (uploadpart, listparts, uploadpartcopy) via a shared quoteETag helper, matching the s3 contract and the posix backend's GenerateEtag convention. populate the previously-empty UploadPartCopy CopyPartResult with the quoted etag and last-modified. normalize the client-supplied etag in CompleteMultipartUpload with backend.TrimEtag so both quoted and raw etags are accepted. add an azure-full-flow integration test asserting part etags are quoted and consistent across uploadpart, listparts and completemultipartupload. --- backend/azure/azure.go | 29 +++++++++-- tests/integration/UploadPart.go | 84 ++++++++++++++++++++++++++++++++ tests/integration/group-tests.go | 7 +++ 3 files changed, 115 insertions(+), 5 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 4d0d22cf..b18708f7 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -1492,7 +1492,7 @@ func (az *Azure) UploadPart(ctx context.Context, input *s3.UploadPartInput) (*s3 // block id serves as etag here etag := blockIDInt32ToBase64(*input.PartNumber) - quotedETag := fmt.Sprintf("%q", etag) + quotedETag := quoteETag(etag) // Azure StageBlock rejects Content-Length: 0 as an invalid header value. // Track zero-byte parts in the sgwtmp metadata instead of staging them. @@ -1545,7 +1545,13 @@ func (az *Azure) UploadPartCopy(ctx context.Context, input *s3.UploadPartCopyInp return s3response.CopyPartResult{}, parseMpError(err) } - return s3response.CopyPartResult{}, nil + // The staged block id serves as the part ETag, returned quoted to match + // the S3 contract and the form UploadPart/ListParts emit. + quotedETag := quoteETag(eTag) + return s3response.CopyPartResult{ + ETag: "edETag, + LastModified: time.Now(), + }, nil } // Lists all uncommitted parts from the blob @@ -1596,7 +1602,7 @@ func (az *Azure) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3res } parts = append(parts, s3response.Part{ Size: *el.Size, - ETag: *el.Name, + ETag: quoteETag(*el.Name), PartNumber: partNumber, LastModified: time.Now(), }) @@ -1611,7 +1617,7 @@ func (az *Azure) ListParts(ctx context.Context, input *s3.ListPartsInput) (s3res } parts = append(parts, s3response.Part{ Size: 0, - ETag: blockIDInt32ToBase64(zbPartNum), + ETag: quoteETag(blockIDInt32ToBase64(zbPartNum)), PartNumber: int(zbPartNum), LastModified: time.Now(), }) @@ -1874,7 +1880,10 @@ func (az *Azure) CompleteMultipartUpload(ctx context.Context, input *s3.Complete if part.ETag == nil { return res, "", s3err.GetAPIError(s3err.ErrMalformedXML) } - clientETag := strings.Trim(getString(part.ETag), "\"") + // Clients may submit the part ETag in quoted form (as returned by + // UploadPart/ListParts) or raw; normalize before comparing to the + // raw Azure block id. + clientETag := getString(backend.TrimEtag(part.ETag)) if *part.PartNumber < 1 { return res, "", s3err.GetInvalidArgumentErr(s3err.InvalidArgCompleteMpPartNumber, fmt.Sprint(*part.PartNumber)) } @@ -2363,6 +2372,16 @@ func getReadSeekCloser(input io.Reader) (io.ReadSeekCloser, error) { return streaming.NopCloser(bytes.NewReader(buffer.Bytes())), nil } +// quoteETag wraps a raw Azure block id (used as a multipart part ETag) in +// double quotes. S3 ETags are quoted strings, and the posix backend follows +// the same convention (see backend.GenerateEtag). Keeping all Azure part-ETag +// surfaces (UploadPart, ListParts, UploadPartCopy) quoted ensures the values +// returned to clients are consistent, while the raw (unquoted) block id is +// still used for the Azure StageBlock/GetBlockList APIs. +func quoteETag(etag string) string { + return fmt.Sprintf("%q", etag) +} + // Creates a new Base64 encoded block id from a 32 bit integer func blockIDInt32ToBase64(blockID int32) string { binaryBlockID := &[4]byte{} // All block IDs are 4 bytes long diff --git a/tests/integration/UploadPart.go b/tests/integration/UploadPart.go index 8576f6e8..2a1a8e04 100644 --- a/tests/integration/UploadPart.go +++ b/tests/integration/UploadPart.go @@ -501,3 +501,87 @@ func UploadPart_success(s *S3Conf) error { return nil }) } + +// isQuotedEtag reports whether an ETag is a non-empty double-quoted string, +// as required by the S3 contract (e.g. "\"abc\""). +func isQuotedEtag(etag string) bool { + return len(etag) >= 2 && + strings.HasPrefix(etag, "\"") && + strings.HasSuffix(etag, "\"") +} + +// UploadPart_etag_quoting_consistency verifies that multipart part ETags are +// returned as quoted strings and stay consistent across the UploadPart +// response, ListParts, and CompleteMultipartUpload. This is the S3 contract +// (ETags are quoted) and matches the posix backend's GenerateEtag convention; +// it is the regression guard for the Azure backend, which previously returned +// raw, unquoted block ids. +func UploadPart_etag_quoting_consistency(s *S3Conf) error { + testName := "UploadPart_etag_quoting_consistency" + 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 + } + + parts, _, err := uploadParts(s3client, 15*1024*1024, 3, bucket, obj, *out.UploadId) + if err != nil { + return err + } + + // Every ETag returned by UploadPart must be a quoted string. + for _, p := range parts { + etag := getString(p.ETag) + if !isQuotedEtag(etag) { + return fmt.Errorf("expected UploadPart etag to be quoted, instead got %q", etag) + } + } + + // ListParts must report the same quoted ETags as UploadPart. + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.ListParts(ctx, &s3.ListPartsInput{ + Bucket: &bucket, + Key: &obj, + UploadId: out.UploadId, + }) + cancel() + if err != nil { + return err + } + for _, p := range res.Parts { + etag := getString(p.ETag) + if !isQuotedEtag(etag) { + return fmt.Errorf("expected ListParts etag to be quoted, instead got %q", etag) + } + } + if ok := compareParts(parts, res.Parts); !ok { + return fmt.Errorf("expected ListParts parts %+v to match UploadPart parts %+v", + res.Parts, parts) + } + + // CompleteMultipartUpload must accept the quoted ETags returned above. + compParts := []types.CompletedPart{} + for _, p := range parts { + compParts = append(compParts, types.CompletedPart{ + ETag: p.ETag, + PartNumber: p.PartNumber, + }) + } + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{ + Bucket: &bucket, + Key: &obj, + UploadId: out.UploadId, + MultipartUpload: &types.CompletedMultipartUpload{ + Parts: compParts, + }, + }) + cancel() + if err != nil { + return fmt.Errorf("complete multipart upload with quoted etags: %w", err) + } + + return nil + }) +} diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 89503a05..5e627c4d 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -460,6 +460,7 @@ func TestUploadPart(ts *TestState) { ts.Run(UploadPart_with_checksums_success) } ts.Run(UploadPart_success) + ts.Run(UploadPart_etag_quoting_consistency) } func TestUploadPartCopy(ts *TestState) { @@ -898,6 +899,11 @@ func TestFullFlow(ts *TestState) { TestDeleteObjectTagging(ts) TestCreateMultipartUpload(ts) TestUploadPart(ts) + // UploadPartCopy maps to Azure StageBlockFromURL, which Azurite does not + // implement (returns HTTP 500 InternalError), so the group is skipped in + // Azure full-flow. The backend now returns a populated, quoted CopyPartResult + // ETag (consistent with UploadPart/ListParts); that path is exercised against + // real Azure rather than Azurite. if !ts.conf.azureTests { TestUploadPartCopy(ts) } @@ -1685,6 +1691,7 @@ func GetIntTests() IntTests { "UploadPart_no_checksum_with_composite_checksum_type": UploadPart_no_checksum_with_composite_checksum_type, "UploadPart_with_checksums_success": UploadPart_with_checksums_success, "UploadPart_success": UploadPart_success, + "UploadPart_etag_quoting_consistency": UploadPart_etag_quoting_consistency, "UploadPartCopy_non_existing_bucket": UploadPartCopy_non_existing_bucket, "UploadPartCopy_incorrect_uploadId": UploadPartCopy_incorrect_uploadId, "UploadPartCopy_incorrect_object_key": UploadPartCopy_incorrect_object_key,