Skip to content

Commit d404777

Browse files
fix(storage): Omit auto checksum in final request when MD5 is given (#14024)
1 parent 3e66d57 commit d404777

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

storage/grpc_writer.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,11 @@ func (w *gRPCWriter) Write(p []byte) (n int, err error) {
5252
case <-w.donec:
5353
return 0, w.streamResult
5454
case w.writesChan <- cmd:
55-
// update fullObjectChecksum on every write and send it on finalWrite
56-
if !w.disableAutoChecksum {
55+
// Update fullObjectChecksum on every write and send it on finalWrite if not disabled.
56+
// Skip checksum calculation if user configures MD5 or CRC32C themselves.
57+
if !w.disableAutoChecksum &&
58+
!w.sendCRC32C &&
59+
w.attrs.MD5 == nil {
5760
w.fullObjectChecksum = crc32.Update(w.fullObjectChecksum, crc32cTable, p)
5861
}
5962
// write command successfully delivered to sender. We no longer own cmd.
@@ -848,7 +851,7 @@ func getObjectChecksums(params *getObjectChecksumsParams) *storagepb.ObjectCheck
848851
}
849852

850853
// send user's checksum on last write op if available
851-
if params.sendCRC32C {
854+
if params.sendCRC32C || params.objectAttrs.MD5 != nil {
852855
return toProtoChecksums(params.sendCRC32C, params.objectAttrs)
853856
}
854857
// TODO(b/461982277): Enable checksum validation for appendable takeover writer gRPC

storage/integration_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2160,6 +2160,8 @@ func TestIntegration_MultiChunkWrite(t *testing.T) {
21602160

21612161
func TestIntegration_WriterCRC32CValidation(t *testing.T) {
21622162
ctx := skipZonalBucket(context.Background(), "Test for resumable and oneshot writers")
2163+
ctx = skipExtraReadAPIs(ctx, "Test for uploads")
2164+
21632165
h := testHelper{t}
21642166
multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, bucket string, _ string, client *Client) {
21652167
testCases := []struct {
@@ -2170,6 +2172,7 @@ func TestIntegration_WriterCRC32CValidation(t *testing.T) {
21702172
disableAutoChecksum bool
21712173
incorrectChecksum bool
21722174
wantErr bool
2175+
sendMD5 bool
21732176
}{
21742177
{
21752178
name: "oneshot with user-sent CRC32C",
@@ -2194,6 +2197,19 @@ func TestIntegration_WriterCRC32CValidation(t *testing.T) {
21942197
chunkSize: 256 * 1024,
21952198
sendCRC32C: true,
21962199
},
2200+
{
2201+
name: "resumable with user-sent MD5",
2202+
content: bytes.Repeat([]byte("a"), 1*MiB),
2203+
chunkSize: 256 * 1024,
2204+
sendMD5: true,
2205+
},
2206+
{
2207+
name: "resumable with user-sent MD5 & CRC32C",
2208+
content: bytes.Repeat([]byte("a"), 1*MiB),
2209+
chunkSize: 256 * 1024,
2210+
sendMD5: true,
2211+
sendCRC32C: true,
2212+
},
21972213
{
21982214
name: "small uploads - data size < chunk size with default CRC32C",
21992215
content: bytes.Repeat([]byte("a"), 200*1024),
@@ -2236,6 +2252,10 @@ func TestIntegration_WriterCRC32CValidation(t *testing.T) {
22362252
w.CRC32C = correctCRC32C + 1
22372253
}
22382254
w.DisableAutoChecksum = tc.disableAutoChecksum
2255+
if tc.sendMD5 {
2256+
md5Sum := md5.Sum(tc.content)
2257+
w.MD5 = md5Sum[:]
2258+
}
22392259

22402260
if _, err := w.Write(tc.content); err != nil {
22412261
t.Fatalf("Writer.Write: %v", err)

0 commit comments

Comments
 (0)