Skip to content

Commit 17d8d81

Browse files
authored
Make it possible to disable integrity checks for multipart uploads (#3148)
As announced in #2960, AWS SDK for Go v2 service/s3 v1.73.0 shipped a change (#2808) that adopted the new default integrity protections. While it is possible to revert to the previous behavior by setting `AWS_REQUEST_CHECKSUM_CALCULATION=when_required`, this config setting did not apply to the S3 Manager multipart uploader, which always enabled CRC32 checksums by default. This commit checks the S3 options and only attaches the CRC32 checksums if `when_required` is not set. Relates to #3007 Signed-off-by: Stan Hu <[email protected]>
1 parent 0475b82 commit 17d8d81

File tree

2 files changed

+106
-1
lines changed

2 files changed

+106
-1
lines changed

feature/s3/manager/upload.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,18 @@ type uploader struct {
341341
totalSize int64 // set to -1 if the size is not known
342342
}
343343

344+
// getRequestChecksumCalculation extracts the RequestChecksumCalculation setting
345+
// from the uploader's ClientOptions configuration.
346+
func (u *uploader) getRequestChecksumCalculation() aws.RequestChecksumCalculation {
347+
opts := &s3.Options{}
348+
349+
for _, opt := range u.cfg.ClientOptions {
350+
opt(opts)
351+
}
352+
353+
return opts.RequestChecksumCalculation
354+
}
355+
344356
// internal logic for deciding whether to upload a single part or use a
345357
// multipart upload.
346358
func (u *uploader) upload() (*UploadOutput, error) {
@@ -776,7 +788,11 @@ func (u *multiuploader) initChecksumAlgorithm() {
776788
case u.in.ChecksumSHA256 != nil:
777789
u.in.ChecksumAlgorithm = types.ChecksumAlgorithmSha256
778790
default:
779-
u.in.ChecksumAlgorithm = types.ChecksumAlgorithmCrc32
791+
checksumCalc := u.getRequestChecksumCalculation()
792+
793+
if checksumCalc != aws.RequestChecksumCalculationWhenRequired {
794+
u.in.ChecksumAlgorithm = types.ChecksumAlgorithmCrc32
795+
}
780796
}
781797
}
782798

feature/s3/manager/upload_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,95 @@ func (h *failPartHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
11601160
h.failsRemaining--
11611161
}
11621162

1163+
// TestUploadRequestChecksumCalculation tests that checksum behavior respects
1164+
// the RequestChecksumCalculation setting for backwards compatibility.
1165+
func TestUploadRequestChecksumCalculation(t *testing.T) {
1166+
testCases := []struct {
1167+
name string
1168+
requestChecksumCalculation aws.RequestChecksumCalculation
1169+
inputChecksumAlgorithm types.ChecksumAlgorithm
1170+
expectedChecksumAlgorithm types.ChecksumAlgorithm
1171+
description string
1172+
}{
1173+
{
1174+
name: "WhenRequired_NoDefault",
1175+
requestChecksumCalculation: aws.RequestChecksumCalculationWhenRequired,
1176+
inputChecksumAlgorithm: "",
1177+
expectedChecksumAlgorithm: "",
1178+
description: "When RequestChecksumCalculationWhenRequired is set, no default checksum should be applied (backwards compatibility)",
1179+
},
1180+
{
1181+
name: "WhenSupported_HasDefault",
1182+
requestChecksumCalculation: aws.RequestChecksumCalculationWhenSupported,
1183+
inputChecksumAlgorithm: "",
1184+
expectedChecksumAlgorithm: types.ChecksumAlgorithmCrc32,
1185+
description: "When RequestChecksumCalculationWhenSupported is set, default CRC32 checksum should be applied",
1186+
},
1187+
{
1188+
name: "WhenRequired_ExplicitPreserved",
1189+
requestChecksumCalculation: aws.RequestChecksumCalculationWhenRequired,
1190+
inputChecksumAlgorithm: types.ChecksumAlgorithmSha256,
1191+
expectedChecksumAlgorithm: types.ChecksumAlgorithmSha256,
1192+
description: "Explicit checksums should always be preserved regardless of RequestChecksumCalculation setting",
1193+
},
1194+
{
1195+
name: "WhenSupported_ExplicitPreserved",
1196+
requestChecksumCalculation: aws.RequestChecksumCalculationWhenSupported,
1197+
inputChecksumAlgorithm: types.ChecksumAlgorithmSha1,
1198+
expectedChecksumAlgorithm: types.ChecksumAlgorithmSha1,
1199+
description: "Explicit checksums should override defaults even with WhenSupported",
1200+
},
1201+
}
1202+
1203+
for _, tc := range testCases {
1204+
t.Run(tc.name, func(t *testing.T) {
1205+
c, invocations, args := s3testing.NewUploadLoggingClient(nil)
1206+
1207+
mgr := manager.NewUploader(c,
1208+
manager.WithUploaderRequestOptions(func(o *s3.Options) {
1209+
o.RequestChecksumCalculation = tc.requestChecksumCalculation
1210+
}))
1211+
1212+
input := &s3.PutObjectInput{
1213+
Bucket: aws.String("Bucket"),
1214+
Key: aws.String("Key"),
1215+
Body: bytes.NewReader(buf12MB), // Large enough to trigger multipart upload
1216+
}
1217+
if tc.inputChecksumAlgorithm != "" {
1218+
input.ChecksumAlgorithm = tc.inputChecksumAlgorithm
1219+
}
1220+
1221+
resp, err := mgr.Upload(context.Background(), input)
1222+
if err != nil {
1223+
t.Errorf("Expected no error but received %v", err)
1224+
}
1225+
1226+
expectedOps := []string{"CreateMultipartUpload", "UploadPart", "UploadPart", "UploadPart", "CompleteMultipartUpload"}
1227+
if diff := cmpDiff(expectedOps, *invocations); len(diff) > 0 {
1228+
t.Error(diff)
1229+
}
1230+
1231+
if resp.UploadID != "UPLOAD-ID" {
1232+
t.Errorf("expect %q, got %q", "UPLOAD-ID", resp.UploadID)
1233+
}
1234+
1235+
cmu := (*args)[0].(*s3.CreateMultipartUploadInput)
1236+
if cmu.ChecksumAlgorithm != tc.expectedChecksumAlgorithm {
1237+
t.Errorf("%s: Expected checksum algorithm %v in CreateMultipartUpload, but got %v",
1238+
tc.description, tc.expectedChecksumAlgorithm, cmu.ChecksumAlgorithm)
1239+
}
1240+
1241+
for i := 1; i <= 3; i++ {
1242+
uploadPart := (*args)[i].(*s3.UploadPartInput)
1243+
if uploadPart.ChecksumAlgorithm != tc.expectedChecksumAlgorithm {
1244+
t.Errorf("%s: Expected checksum algorithm %v in UploadPart %d, but got %v",
1245+
tc.description, tc.expectedChecksumAlgorithm, i, uploadPart.ChecksumAlgorithm)
1246+
}
1247+
}
1248+
})
1249+
}
1250+
}
1251+
11631252
type recordedBufferProvider struct {
11641253
content []byte
11651254
size int

0 commit comments

Comments
 (0)