mirror of
https://github.com/versity/versitygw.git
synced 2026-08-20 14:16:28 +00:00
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.
This commit is contained in:
+24
-5
@@ -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
|
||||
|
||||
@@ -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
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user