Skip to content

Commit 9b3ba35

Browse files
frristclaude
andcommitted
feat(s3): CopyObject checksum propagation and replacement
A copy without a checksum request carries the source's checksum triple (algorithm/value/type) to the destination; a request naming a different x-amz-checksum-algorithm replaces it, streaming the shared body through the new algorithm once for a full-object value. The CopyObjectResult now reports the destination checksum fields. Promotes the four checksum rows of the CopyObject XFail table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2ca1cce commit 9b3ba35

3 files changed

Lines changed: 124 additions & 16 deletions

File tree

itest/versity_object_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,14 @@ var copyObjectPass = []forgeCase{
173173
{name: "invalid_legal_hold", fn: integration.CopyObject_invalid_legal_hold},
174174
{name: "invalid_object_lock_mode", fn: integration.CopyObject_invalid_object_lock_mode},
175175
{name: "invalid_website_redirect_location", fn: integration.CopyObject_invalid_website_redirect_location},
176+
{name: "create_checksum_on_copy", fn: integration.CopyObject_create_checksum_on_copy},
177+
{name: "should_copy_the_existing_checksum", fn: integration.CopyObject_should_copy_the_existing_checksum},
178+
{name: "should_replace_the_existing_checksum", fn: integration.CopyObject_should_replace_the_existing_checksum},
179+
{name: "to_itself_by_replacing_the_checksum", fn: integration.CopyObject_to_itself_by_replacing_the_checksum},
176180
}
177181

178182
// Observed failing against the forge stack: multi-account semantics, tagging,
179-
// object-lock, and checksum-on-copy are unimplemented surface.
183+
// and object-lock are unimplemented surface.
180184
var copyObjectXFail = []forgeCase{
181185
{name: "not_owned_source_bucket", fn: integration.CopyObject_not_owned_source_bucket},
182186
{name: "should_replace_tagging", fn: integration.CopyObject_should_replace_tagging},
@@ -185,10 +189,6 @@ var copyObjectXFail = []forgeCase{
185189
{name: "with_legal_hold", fn: integration.CopyObject_with_legal_hold},
186190
{name: "with_retention_lock", fn: integration.CopyObject_with_retention_lock},
187191
{name: "object_acl_not_supported", fn: integration.CopyObject_object_acl_not_supported},
188-
{name: "create_checksum_on_copy", fn: integration.CopyObject_create_checksum_on_copy},
189-
{name: "should_copy_the_existing_checksum", fn: integration.CopyObject_should_copy_the_existing_checksum},
190-
{name: "should_replace_the_existing_checksum", fn: integration.CopyObject_should_replace_the_existing_checksum},
191-
{name: "to_itself_by_replacing_the_checksum", fn: integration.CopyObject_to_itself_by_replacing_the_checksum},
192192
{name: "incorrect_source_bucket_expected_owner", fn: integration.CopyObject_incorrect_source_bucket_expected_owner},
193193
}
194194

s3frontend/copy.go

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"io"
78
"time"
89

910
"github.com/aws/aws-sdk-go-v2/service/s3/types"
1011
"github.com/fil-forge/versitygw/backend"
12+
"github.com/fil-forge/versitygw/s3api/utils"
1113
"github.com/fil-forge/versitygw/s3err"
1214
"github.com/fil-forge/versitygw/s3response"
1315

@@ -83,16 +85,42 @@ func (b *Backend) CopyObject(ctx context.Context, input s3response.CopyObjectInp
8385
return s3response.CopyObjectOutput{}, err
8486
}
8587

88+
// Destination checksum: same bytes → the source's checksum (and type)
89+
// carries over. A request naming a DIFFERENT x-amz-checksum-algorithm
90+
// replaces it: the shared body streams through the new algorithm once and
91+
// the result is a full-object value — the sole per-object checksum, never
92+
// accumulated alongside the source's.
93+
ckAlgo, ckVal, ckType := srcMf.ChecksumAlgorithm, srcMf.Checksum, srcMf.ChecksumType
94+
if ckVal != "" && ckType == "" {
95+
ckType = string(types.ChecksumTypeFullObject)
96+
}
97+
if reqAlgo := input.ChecksumAlgorithm; reqAlgo != "" && string(reqAlgo) != srcMf.ChecksumAlgorithm {
98+
ht, err := hashTypeForAlgo(reqAlgo)
99+
if err != nil {
100+
return s3response.CopyObjectOutput{}, err
101+
}
102+
rc := msbucket.OpenBody(ctx, b.read, srcRv.st.Space, srcMf.Body)
103+
defer rc.Close()
104+
hr, err := utils.NewHashReader(rc, "", ht)
105+
if err != nil {
106+
return s3response.CopyObjectOutput{}, fmt.Errorf("s3frontend: copy checksum reader: %w", err)
107+
}
108+
if _, err := io.Copy(io.Discard, hr); err != nil {
109+
return s3response.CopyObjectOutput{}, fmt.Errorf("s3frontend: copy checksum: %w", err)
110+
}
111+
ckAlgo, ckVal, ckType = string(reqAlgo), hr.Sum(), string(types.ChecksumTypeFullObject)
112+
}
113+
86114
// Destination manifest: the SAME body (size/sha/md5/blobs) and ETag, since
87115
// the content is identical. Metadata per the directive.
88116
dstMf := &msbucket.ObjectManifest{
89-
Key: dstKey,
90-
Created: time.Now().Unix(),
91-
Body: srcMf.Body,
92-
ETag: srcMf.ETag,
93-
// Same content → same additional checksum, regardless of directive.
94-
ChecksumAlgorithm: srcMf.ChecksumAlgorithm,
95-
Checksum: srcMf.Checksum,
117+
Key: dstKey,
118+
Created: time.Now().Unix(),
119+
Body: srcMf.Body,
120+
ETag: srcMf.ETag,
121+
ChecksumAlgorithm: ckAlgo,
122+
Checksum: ckVal,
123+
ChecksumType: ckType,
96124
}
97125
if replace {
98126
ct := backend.GetStringFromPtr(input.ContentType)
@@ -130,11 +158,13 @@ func (b *Backend) CopyObject(ctx context.Context, input s3response.CopyObjectInp
130158

131159
lastMod := time.Unix(dstMf.Created, 0)
132160
etag := etagOf(dstMf)
161+
result := &s3response.CopyObjectResult{
162+
ETag: &etag,
163+
LastModified: &lastMod,
164+
}
165+
result.ChecksumCRC32, result.ChecksumCRC32C, result.ChecksumSHA1, result.ChecksumSHA256, result.ChecksumCRC64NVME, result.ChecksumSHA512, result.ChecksumMD5, result.ChecksumXXHASH64, result.ChecksumXXHASH3, result.ChecksumXXHASH128, result.ChecksumType = checksumFields(dstMf.ChecksumAlgorithm, dstMf.Checksum, dstMf.ChecksumType)
133166
out := s3response.CopyObjectOutput{
134-
CopyObjectResult: &s3response.CopyObjectResult{
135-
ETag: &etag,
136-
LastModified: &lastMod,
137-
},
167+
CopyObjectResult: result,
138168
}
139169
// Version ids in the response, per each side's bucket state (§4.3).
140170
if srcRv.versioned() {

s3frontend/multipart_checksum_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,3 +430,81 @@ func TestListParts_Checksums(t *testing.T) {
430430
t.Fatalf("internal CRC64NVME leaked into ListParts: %s", *res.Parts[0].ChecksumCRC64NVME)
431431
}
432432
}
433+
434+
// TestCopyObject_Checksums: a copy without a checksum request propagates the
435+
// source's checksum verbatim; a copy naming a different algorithm replaces it
436+
// with a recomputed full-object value (the old algorithm's field goes nil),
437+
// and the result agrees with a subsequent HEAD.
438+
func TestCopyObject_Checksums(t *testing.T) {
439+
b, _, _ := newRefTestBackend(t)
440+
bucket := "bk"
441+
data := []byte("copy-source-bytes")
442+
crcSum := crc32cB64(data)
443+
444+
srcKey := "cp-src"
445+
if _, err := b.PutObject(context.Background(), s3response.PutObjectInput{
446+
Bucket: &bucket,
447+
Key: &srcKey,
448+
Body: bytes.NewReader(data),
449+
ChecksumCRC32C: &crcSum,
450+
}); err != nil {
451+
t.Fatalf("PutObject: %v", err)
452+
}
453+
454+
// No checksum request → the source's CRC32C carries over.
455+
dstKey, src := "cp-propagate", bucket+"/"+srcKey
456+
out, err := b.CopyObject(context.Background(), s3response.CopyObjectInput{
457+
Bucket: &bucket, Key: &dstKey, CopySource: &src,
458+
})
459+
if err != nil {
460+
t.Fatalf("CopyObject: %v", err)
461+
}
462+
if out.CopyObjectResult.ChecksumCRC32C == nil || *out.CopyObjectResult.ChecksumCRC32C != crcSum {
463+
t.Fatalf("propagate: result CRC32C = %v, want %s", out.CopyObjectResult.ChecksumCRC32C, crcSum)
464+
}
465+
if out.CopyObjectResult.ChecksumType != types.ChecksumTypeFullObject {
466+
t.Fatalf("propagate: type = %v, want FULL_OBJECT", out.CopyObjectResult.ChecksumType)
467+
}
468+
hCrc32c, _, _, _ := headChecksum(t, b, dstKey)
469+
if hCrc32c == nil || *hCrc32c != crcSum {
470+
t.Fatalf("propagate: HEAD CRC32C = %v, want %s", hCrc32c, crcSum)
471+
}
472+
473+
// A different requested algorithm replaces the checksum: SHA256 set to the
474+
// recomputed body digest, the source's CRC32C dropped.
475+
dstKey = "cp-replace"
476+
out, err = b.CopyObject(context.Background(), s3response.CopyObjectInput{
477+
Bucket: &bucket, Key: &dstKey, CopySource: &src,
478+
ChecksumAlgorithm: types.ChecksumAlgorithmSha256,
479+
})
480+
if err != nil {
481+
t.Fatalf("CopyObject: %v", err)
482+
}
483+
if out.CopyObjectResult.ChecksumSHA256 == nil || *out.CopyObjectResult.ChecksumSHA256 != sha256B64(data) {
484+
t.Fatalf("replace: result SHA256 = %v, want %s", out.CopyObjectResult.ChecksumSHA256, sha256B64(data))
485+
}
486+
if out.CopyObjectResult.ChecksumCRC32C != nil {
487+
t.Fatalf("replace: source CRC32C survived: %s", *out.CopyObjectResult.ChecksumCRC32C)
488+
}
489+
_, _, hSha, hType := headChecksum(t, b, dstKey)
490+
if hSha == nil || *hSha != sha256B64(data) || hType != types.ChecksumTypeFullObject {
491+
t.Fatalf("replace: HEAD = %v/%v, want %s/FULL_OBJECT", hSha, hType, sha256B64(data))
492+
}
493+
494+
// Self-copy with REPLACE swapping the algorithm again.
495+
selfSrc := bucket + "/" + dstKey
496+
out, err = b.CopyObject(context.Background(), s3response.CopyObjectInput{
497+
Bucket: &bucket, Key: &dstKey, CopySource: &selfSrc,
498+
MetadataDirective: types.MetadataDirectiveReplace,
499+
ChecksumAlgorithm: types.ChecksumAlgorithmCrc32c,
500+
})
501+
if err != nil {
502+
t.Fatalf("self-copy: %v", err)
503+
}
504+
if out.CopyObjectResult.ChecksumCRC32C == nil || *out.CopyObjectResult.ChecksumCRC32C != crcSum {
505+
t.Fatalf("self-copy: result CRC32C = %v, want %s", out.CopyObjectResult.ChecksumCRC32C, crcSum)
506+
}
507+
if out.CopyObjectResult.ChecksumSHA256 != nil {
508+
t.Fatalf("self-copy: SHA256 survived: %s", *out.CopyObjectResult.ChecksumSHA256)
509+
}
510+
}

0 commit comments

Comments
 (0)