diff --git a/test/s3/checksum/s3_checksum_multipart_test.go b/test/s3/checksum/s3_checksum_multipart_test.go index 1482fe753..a09af198a 100644 --- a/test/s3/checksum/s3_checksum_multipart_test.go +++ b/test/s3/checksum/s3_checksum_multipart_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "net/http/httputil" "net/url" + "strings" "testing" "github.com/aws/aws-sdk-go-v2/aws" @@ -17,15 +18,18 @@ import ( "github.com/stretchr/testify/require" ) -func TestMultipartCopyPreservesChecksum(t *testing.T) { - // aws-sdk-go-v2 sends flexible checksums as unsigned streaming trailers, which - // it refuses over plain HTTP, so front the HTTP endpoint with a TLS proxy. +// newTrailerChecksumClient returns a client that can send flexible checksums. +// aws-sdk-go-v2 sends them as unsigned streaming trailers, which it refuses over +// plain HTTP, so front the HTTP endpoint with a TLS proxy. +func newTrailerChecksumClient(t *testing.T) *s3.Client { + t.Helper() + target, err := url.Parse(defaultConfig.Endpoint) require.NoError(t, err) proxy := httputil.NewSingleHostReverseProxy(target) server := httptest.NewTLSServer(proxy) - defer server.Close() + t.Cleanup(server.Close) cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(defaultConfig.Region), @@ -34,10 +38,14 @@ func TestMultipartCopyPreservesChecksum(t *testing.T) { config.WithHTTPClient(server.Client()), ) require.NoError(t, err) - client := s3.NewFromConfig(cfg, func(o *s3.Options) { + return s3.NewFromConfig(cfg, func(o *s3.Options) { o.BaseEndpoint = aws.String(server.URL) o.UsePathStyle = true }) +} + +func TestMultipartCopyPreservesChecksum(t *testing.T) { + client := newTrailerChecksumClient(t) bucket := uniqueBucket() createBucket(t, client, bucket) @@ -122,3 +130,120 @@ func TestMultipartCopyPreservesChecksum(t *testing.T) { }) } } + +// A multipart upload that asked for a checksum must report it in the +// CompleteMultipartUpload response, alongside the algorithm and type that +// CreateMultipartUpload echoed back. +func TestMultipartUploadReturnsObjectChecksum(t *testing.T) { + client := newTrailerChecksumClient(t) + + bucket := uniqueBucket() + createBucket(t, client, bucket) + defer cleanupBucket(t, client, bucket) + + cases := []struct { + algorithm types.ChecksumAlgorithm + expectedType types.ChecksumType + completeSum func(*s3.CompleteMultipartUploadOutput) *string + headSum func(*s3.HeadObjectOutput) *string + partSum func(*s3.UploadPartOutput) *string + setPart func(*types.CompletedPart, *string) + }{ + { + algorithm: types.ChecksumAlgorithmCrc32, + expectedType: types.ChecksumTypeComposite, + completeSum: func(o *s3.CompleteMultipartUploadOutput) *string { return o.ChecksumCRC32 }, + headSum: func(o *s3.HeadObjectOutput) *string { return o.ChecksumCRC32 }, + partSum: func(o *s3.UploadPartOutput) *string { return o.ChecksumCRC32 }, + setPart: func(p *types.CompletedPart, v *string) { p.ChecksumCRC32 = v }, + }, + { + algorithm: types.ChecksumAlgorithmCrc32c, + expectedType: types.ChecksumTypeComposite, + completeSum: func(o *s3.CompleteMultipartUploadOutput) *string { return o.ChecksumCRC32C }, + headSum: func(o *s3.HeadObjectOutput) *string { return o.ChecksumCRC32C }, + partSum: func(o *s3.UploadPartOutput) *string { return o.ChecksumCRC32C }, + setPart: func(p *types.CompletedPart, v *string) { p.ChecksumCRC32C = v }, + }, + { + algorithm: types.ChecksumAlgorithmCrc64nvme, + expectedType: types.ChecksumTypeFullObject, + completeSum: func(o *s3.CompleteMultipartUploadOutput) *string { return o.ChecksumCRC64NVME }, + headSum: func(o *s3.HeadObjectOutput) *string { return o.ChecksumCRC64NVME }, + partSum: func(o *s3.UploadPartOutput) *string { return o.ChecksumCRC64NVME }, + setPart: func(p *types.CompletedPart, v *string) { p.ChecksumCRC64NVME = v }, + }, + { + algorithm: types.ChecksumAlgorithmSha1, + expectedType: types.ChecksumTypeComposite, + completeSum: func(o *s3.CompleteMultipartUploadOutput) *string { return o.ChecksumSHA1 }, + headSum: func(o *s3.HeadObjectOutput) *string { return o.ChecksumSHA1 }, + partSum: func(o *s3.UploadPartOutput) *string { return o.ChecksumSHA1 }, + setPart: func(p *types.CompletedPart, v *string) { p.ChecksumSHA1 = v }, + }, + { + algorithm: types.ChecksumAlgorithmSha256, + expectedType: types.ChecksumTypeComposite, + completeSum: func(o *s3.CompleteMultipartUploadOutput) *string { return o.ChecksumSHA256 }, + headSum: func(o *s3.HeadObjectOutput) *string { return o.ChecksumSHA256 }, + partSum: func(o *s3.UploadPartOutput) *string { return o.ChecksumSHA256 }, + setPart: func(p *types.CompletedPart, v *string) { p.ChecksumSHA256 = v }, + }, + } + + // Every part but the last has to reach the 5MB multipart minimum. + parts := [][]byte{bytes.Repeat([]byte("a"), 5*1024*1024), []byte("tail")} + + for _, tc := range cases { + t.Run(string(tc.algorithm), func(t *testing.T) { + key := "multipart-" + string(tc.algorithm) + create, err := client.CreateMultipartUpload(context.Background(), &s3.CreateMultipartUploadInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + ChecksumAlgorithm: tc.algorithm, + }) + require.NoError(t, err) + require.Equal(t, tc.algorithm, create.ChecksumAlgorithm) + require.Equal(t, tc.expectedType, create.ChecksumType) + + var completed []types.CompletedPart + for i, data := range parts { + part, err := client.UploadPart(context.Background(), &s3.UploadPartInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + UploadId: create.UploadId, + PartNumber: aws.Int32(int32(i + 1)), + Body: bytes.NewReader(data), + ChecksumAlgorithm: tc.algorithm, + }) + require.NoError(t, err) + require.NotEmpty(t, aws.ToString(tc.partSum(part))) + + entry := types.CompletedPart{ETag: part.ETag, PartNumber: aws.Int32(int32(i + 1))} + tc.setPart(&entry, tc.partSum(part)) + completed = append(completed, entry) + } + + done, err := client.CompleteMultipartUpload(context.Background(), &s3.CompleteMultipartUploadInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + UploadId: create.UploadId, + MultipartUpload: &types.CompletedMultipartUpload{Parts: completed}, + }) + require.NoError(t, err) + require.NotEmpty(t, aws.ToString(tc.completeSum(done))) + require.Equal(t, tc.expectedType, done.ChecksumType) + if tc.expectedType == types.ChecksumTypeComposite { + require.True(t, strings.HasSuffix(aws.ToString(tc.completeSum(done)), fmt.Sprintf("-%d", len(parts)))) + } + + head, err := client.HeadObject(context.Background(), &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + ChecksumMode: types.ChecksumModeEnabled, + }) + require.NoError(t, err) + require.Equal(t, aws.ToString(tc.completeSum(done)), aws.ToString(tc.headSum(head))) + }) + } +} diff --git a/weed/s3api/filer_multipart.go b/weed/s3api/filer_multipart.go index fd537260e..3bde26441 100644 --- a/weed/s3api/filer_multipart.go +++ b/weed/s3api/filer_multipart.go @@ -43,6 +43,10 @@ const ( type InitiateMultipartUploadResult struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ InitiateMultipartUploadResult"` s3.CreateMultipartUploadOutput + + // Checksum fields — returned as HTTP response headers, not in the XML body + ChecksumAlgorithm string `xml:"-"` + ChecksumType string `xml:"-"` } // getRequestScheme determines the URL scheme (http or https) from the request @@ -148,6 +152,8 @@ func (s3a *S3ApiServer) createMultipartUpload(r *http.Request, input *s3.CreateM Key: objectKey(input.Key), UploadId: aws.String(uploadIdString), }, + ChecksumAlgorithm: checksumAlgorithmNameFromHeaderName(checksumHeaderName), + ChecksumType: checksumType, } return @@ -160,10 +166,8 @@ type CompleteMultipartUploadResult struct { Key *string `xml:"Key,omitempty"` ETag *string `xml:"ETag,omitempty"` - // Checksum fields — returned as HTTP response headers, not in the XML body - ChecksumHeaderName string `xml:"-"` - ChecksumValue string `xml:"-"` - ChecksumType string `xml:"-"` + ChecksumResult + ChecksumType string `xml:"ChecksumType,omitempty"` // VersionId is NOT included in XML body - it should only be in x-amz-version-id HTTP header @@ -253,6 +257,8 @@ func completeMultipartResult(r *http.Request, input *s3.CompleteMultipartUploadI result.VersionId = aws.String(versionId) } } + result.SetChecksum(string(entry.Extended[s3_constants.ExtChecksumAlgorithm]), string(entry.Extended[s3_constants.ExtChecksumValue])) + result.ChecksumType = string(entry.Extended[s3_constants.ExtChecksumType]) } return result } @@ -763,15 +769,14 @@ func (s3a *S3ApiServer) completeMultipartUpload(r *http.Request, input *s3.Compl // For versioned buckets, all content is stored in .versions directory // The latest version information is tracked in the .versions directory metadata output = &CompleteMultipartUploadResult{ - Location: aws.String(fmt.Sprintf("%s://%s/%s/%s", getRequestScheme(r), r.Host, url.PathEscape(*input.Bucket), urlPathEscape(*input.Key))), - Bucket: input.Bucket, - ETag: aws.String(etagQuote), - Key: objectKey(input.Key), - VersionId: aws.String(versionId), - ChecksumHeaderName: completionState.checksumHeaderName, - ChecksumValue: completionState.checksumValue, - ChecksumType: completionState.checksumType, + Location: aws.String(fmt.Sprintf("%s://%s/%s/%s", getRequestScheme(r), r.Host, url.PathEscape(*input.Bucket), urlPathEscape(*input.Key))), + Bucket: input.Bucket, + ETag: aws.String(etagQuote), + Key: objectKey(input.Key), + VersionId: aws.String(versionId), + ChecksumType: completionState.checksumType, } + output.SetChecksum(completionState.checksumHeaderName, completionState.checksumValue) return s3err.ErrNone } @@ -840,15 +845,14 @@ func (s3a *S3ApiServer) completeMultipartUpload(r *http.Request, input *s3.Compl // Note: Suspended versioning should NOT return VersionId field according to AWS S3 spec output = &CompleteMultipartUploadResult{ - Location: aws.String(fmt.Sprintf("%s://%s/%s/%s", getRequestScheme(r), r.Host, url.PathEscape(*input.Bucket), urlPathEscape(*input.Key))), - Bucket: input.Bucket, - ETag: aws.String(etagQuote), - Key: objectKey(input.Key), - ChecksumHeaderName: completionState.checksumHeaderName, - ChecksumValue: completionState.checksumValue, - ChecksumType: completionState.checksumType, + Location: aws.String(fmt.Sprintf("%s://%s/%s/%s", getRequestScheme(r), r.Host, url.PathEscape(*input.Bucket), urlPathEscape(*input.Key))), + Bucket: input.Bucket, + ETag: aws.String(etagQuote), + Key: objectKey(input.Key), + ChecksumType: completionState.checksumType, // VersionId field intentionally omitted for suspended versioning } + output.SetChecksum(completionState.checksumHeaderName, completionState.checksumValue) return s3err.ErrNone } @@ -911,14 +915,13 @@ func (s3a *S3ApiServer) completeMultipartUpload(r *http.Request, input *s3.Compl // For non-versioned buckets, return response without VersionId output = &CompleteMultipartUploadResult{ - Location: aws.String(fmt.Sprintf("%s://%s/%s/%s", getRequestScheme(r), r.Host, url.PathEscape(*input.Bucket), urlPathEscape(*input.Key))), - Bucket: input.Bucket, - ETag: aws.String(etagQuote), - Key: objectKey(input.Key), - ChecksumHeaderName: completionState.checksumHeaderName, - ChecksumValue: completionState.checksumValue, - ChecksumType: completionState.checksumType, + Location: aws.String(fmt.Sprintf("%s://%s/%s/%s", getRequestScheme(r), r.Host, url.PathEscape(*input.Bucket), urlPathEscape(*input.Key))), + Bucket: input.Bucket, + ETag: aws.String(etagQuote), + Key: objectKey(input.Key), + ChecksumType: completionState.checksumType, } + output.SetChecksum(completionState.checksumHeaderName, completionState.checksumValue) return s3err.ErrNone } var finalizeCode s3err.ErrorCode diff --git a/weed/s3api/filer_multipart_checksum_test.go b/weed/s3api/filer_multipart_checksum_test.go index f0558e8f1..02a0f738b 100644 --- a/weed/s3api/filer_multipart_checksum_test.go +++ b/weed/s3api/filer_multipart_checksum_test.go @@ -4,11 +4,17 @@ import ( "bytes" "encoding/base64" "hash/crc32" + "net/http" + "net/http/httptest" + "strings" "testing" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/s3" "github.com/minio/crc64nvme" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) func makeCRC64NVMEPartEntry(data []byte) *filer_pb.Entry { @@ -144,3 +150,57 @@ func TestResolveMultipartChecksumType(t *testing.T) { }) } } + +// CompleteMultipartUpload returns the object checksum in the XML body, where the +// AWS SDKs read it from — not as an HTTP response header. +func TestCompleteMultipartUploadResultChecksumXML(t *testing.T) { + result := &CompleteMultipartUploadResult{ + ETag: aws.String("\"etag-1\""), + ChecksumType: s3_constants.ChecksumTypeComposite, + } + result.SetChecksum(s3_constants.AmzChecksumCRC32C, "fx4FQw==-1") + + encoded := string(s3err.EncodeXMLResponse(result)) + for _, want := range []string{ + "fx4FQw==-1", + "COMPOSITE", + } { + if !strings.Contains(encoded, want) { + t.Fatalf("response %q does not contain %q", encoded, want) + } + } + if strings.Contains(encoded, "") { + t.Fatalf("response %q carries an unrequested algorithm", encoded) + } +} + +// A retried CompleteMultipartUpload rebuilds the response from the committed +// entry, so it must repeat the checksum the first attempt returned. +func TestCompleteMultipartResultChecksumFromEntry(t *testing.T) { + input := &s3.CompleteMultipartUploadInput{Bucket: aws.String("bucket"), Key: aws.String("key")} + entry := &filer_pb.Entry{Extended: map[string][]byte{ + s3_constants.ExtChecksumAlgorithm: []byte(s3_constants.AmzChecksumCRC32C), + s3_constants.ExtChecksumValue: []byte("fx4FQw==-1"), + s3_constants.ExtChecksumType: []byte(s3_constants.ChecksumTypeComposite), + }} + + result := completeMultipartResult(httptest.NewRequest(http.MethodPost, "/bucket/key", nil), input, "\"etag-1\"", entry) + if result.ChecksumCRC32C != "fx4FQw==-1" { + t.Fatalf("ChecksumCRC32C = %q, want %q", result.ChecksumCRC32C, "fx4FQw==-1") + } + if result.ChecksumType != s3_constants.ChecksumTypeComposite { + t.Fatalf("ChecksumType = %q, want %q", result.ChecksumType, s3_constants.ChecksumTypeComposite) + } +} + +// CreateMultipartUpload echoes the algorithm and type it recorded, so a client +// can tell which checksum the upload will be completed with. +func TestCreateMultipartUploadResultChecksumHeaders(t *testing.T) { + result := &InitiateMultipartUploadResult{ + ChecksumAlgorithm: "CRC32C", + ChecksumType: s3_constants.ChecksumTypeComposite, + } + if encoded := string(s3err.EncodeXMLResponse(result)); strings.Contains(encoded, "Checksum") { + t.Fatalf("response %q carries checksum members in the XML body", encoded) + } +} diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index 5885188e1..84951348e 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -787,13 +787,9 @@ func pathToBucketObjectAndVersion(rawPath, decodedPath string) (bucket, object, } type CopyPartResult struct { - LastModified time.Time `xml:"LastModified"` - ETag string `xml:"ETag"` - ChecksumCRC32 string `xml:"ChecksumCRC32,omitempty"` - ChecksumCRC32C string `xml:"ChecksumCRC32C,omitempty"` - ChecksumCRC64NVME string `xml:"ChecksumCRC64NVME,omitempty"` - ChecksumSHA1 string `xml:"ChecksumSHA1,omitempty"` - ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` + LastModified time.Time `xml:"LastModified"` + ETag string `xml:"ETag"` + ChecksumResult } func buildCopyPartResult(etag string, lastModified time.Time, metadata SSEResponseMetadata) CopyPartResult { @@ -801,18 +797,7 @@ func buildCopyPartResult(etag string, lastModified time.Time, metadata SSERespon ETag: etag, LastModified: lastModified, } - switch metadata.ChecksumHeaderName { - case s3_constants.AmzChecksumCRC32: - result.ChecksumCRC32 = metadata.ChecksumValue - case s3_constants.AmzChecksumCRC32C: - result.ChecksumCRC32C = metadata.ChecksumValue - case s3_constants.AmzChecksumCRC64NVME: - result.ChecksumCRC64NVME = metadata.ChecksumValue - case s3_constants.AmzChecksumSHA1: - result.ChecksumSHA1 = metadata.ChecksumValue - case s3_constants.AmzChecksumSHA256: - result.ChecksumSHA256 = metadata.ChecksumValue - } + result.SetChecksum(metadata.ChecksumHeaderName, metadata.ChecksumValue) return result } diff --git a/weed/s3api/s3api_object_handlers_copy_checksum_test.go b/weed/s3api/s3api_object_handlers_copy_checksum_test.go index b6ed9247a..705a6ada3 100644 --- a/weed/s3api/s3api_object_handlers_copy_checksum_test.go +++ b/weed/s3api/s3api_object_handlers_copy_checksum_test.go @@ -75,7 +75,7 @@ func TestBuildCopyPartResult(t *testing.T) { header: s3_constants.AmzChecksumCRC32, element: "value", expected: CopyPartResult{ - ETag: "etag", LastModified: modified, ChecksumCRC32: "value", + ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC32: "value"}, }, }, { @@ -83,7 +83,7 @@ func TestBuildCopyPartResult(t *testing.T) { header: s3_constants.AmzChecksumCRC32C, element: "value", expected: CopyPartResult{ - ETag: "etag", LastModified: modified, ChecksumCRC32C: "value", + ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC32C: "value"}, }, }, { @@ -91,7 +91,7 @@ func TestBuildCopyPartResult(t *testing.T) { header: s3_constants.AmzChecksumCRC64NVME, element: "value", expected: CopyPartResult{ - ETag: "etag", LastModified: modified, ChecksumCRC64NVME: "value", + ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumCRC64NVME: "value"}, }, }, { @@ -99,7 +99,7 @@ func TestBuildCopyPartResult(t *testing.T) { header: s3_constants.AmzChecksumSHA1, element: "value", expected: CopyPartResult{ - ETag: "etag", LastModified: modified, ChecksumSHA1: "value", + ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumSHA1: "value"}, }, }, { @@ -107,7 +107,7 @@ func TestBuildCopyPartResult(t *testing.T) { header: s3_constants.AmzChecksumSHA256, element: "value", expected: CopyPartResult{ - ETag: "etag", LastModified: modified, ChecksumSHA256: "value", + ETag: "etag", LastModified: modified, ChecksumResult: ChecksumResult{ChecksumSHA256: "value"}, }, }, { diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index f5c593c08..6926e3da0 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/weed/s3api/s3api_object_handlers_multipart.go @@ -120,6 +120,13 @@ func (s3a *S3ApiServer) NewMultipartUploadHandler(w http.ResponseWriter, r *http return } + if response.ChecksumAlgorithm != "" { + w.Header().Set(s3_constants.AmzChecksumAlgorithm, response.ChecksumAlgorithm) + if response.ChecksumType != "" { + w.Header().Set(s3_constants.AmzChecksumType, response.ChecksumType) + } + } + writeSuccessResponseXML(w, r, response) } @@ -180,14 +187,6 @@ func (s3a *S3ApiServer) CompleteMultipartUploadHandler(w http.ResponseWriter, r w.Header().Set("x-amz-version-id", *response.VersionId) } - // Set checksum header if present - if response.ChecksumHeaderName != "" && response.ChecksumValue != "" { - w.Header().Set(response.ChecksumHeaderName, response.ChecksumValue) - if response.ChecksumType != "" { - w.Header().Set(s3_constants.AmzChecksumType, response.ChecksumType) - } - } - stats_collect.RecordBucketActiveTime(bucket) stats_collect.S3UploadedObjectsCounter.WithLabelValues(bucket).Inc() diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index f8990de43..472e13895 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -1067,6 +1067,31 @@ var checksumHeaders = []struct { {s3_constants.AmzChecksumSHA256, ChecksumAlgorithmSHA256, s3_constants.AmzChecksumSHA256}, } +// ChecksumResult carries the flexible-checksum members S3 returns inside an XML +// response body, keyed by the canonical x-amz-checksum-* header name. +type ChecksumResult struct { + ChecksumCRC32 string `xml:"ChecksumCRC32,omitempty"` + ChecksumCRC32C string `xml:"ChecksumCRC32C,omitempty"` + ChecksumCRC64NVME string `xml:"ChecksumCRC64NVME,omitempty"` + ChecksumSHA1 string `xml:"ChecksumSHA1,omitempty"` + ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` +} + +func (c *ChecksumResult) SetChecksum(headerName, value string) { + switch headerName { + case s3_constants.AmzChecksumCRC32: + c.ChecksumCRC32 = value + case s3_constants.AmzChecksumCRC32C: + c.ChecksumCRC32C = value + case s3_constants.AmzChecksumCRC64NVME: + c.ChecksumCRC64NVME = value + case s3_constants.AmzChecksumSHA1: + c.ChecksumSHA1 = value + case s3_constants.AmzChecksumSHA256: + c.ChecksumSHA256 = value + } +} + // lookupHeaderOrQuery returns the value of an x-amz-* parameter, checking the // request headers first and falling back to the pre-parsed query values. AWS // SDK presigners hoist headers such as x-amz-sdk-checksum-algorithm into the