feat(storage): object-level checksum in gRPC bidi read by xlai20 · Pull Request #6131 · googleapis/google-cloud-rust

xlai20 · 2026-07-22T10:09:26Z

Description

This PR introduces object-level checksum validation (CRC32C and MD5) for gRPC bidirectional streaming reads (open_object).

go/rust-sdk-grpc-bidi-object-checksum, Issue #6056

Key Changes:

Checksum Validation Options: Added compute_crc32c(bool) and compute_md5() configuration toggles to open_object. CRC32C is enabled by default.
Resilient Error Handling: Validating checksums on the foreground reader prevents a mismatch from tearing down the underlying gRPC connection for other multiplexed streams. Additionally, streams now gracefully exhaust themselves on error to prevent infinite polling loops.
Testing: Added comprehensive unit and integration tests, including verification that content-encoding: gzip objects successfully validate their CRC32C hashes over gRPC without false mismatches.

gemini-code-assist

Code Review

This pull request implements checksum verification (CRC32C and MD5) for the gRPC bidirectional read stream in Google Cloud Storage. It updates RangeReader to track offsets, update checksums, and validate them upon stream completion, while bypassing validation for ranged reads. It also adds builder methods compute_md5 and compute_crc32c to OpenObject to control checksum behavior, along with comprehensive unit and integration tests. Feedback from the review suggests marking the reader as exhausted when an error is encountered to prevent misleading checksum mismatches on subsequent calls, and refactoring compute_crc32c to return early and avoid an unnecessary else block in accordance with the repository style guide.

codecov · 2026-07-22T16:12:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.58%. Comparing base (e50b169) to head (e01bbb2).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6131      +/-   ##
==========================================
+ Coverage   96.57%   96.58%   +0.01%     
==========================================
  Files         264      264              
  Lines       66496    66689     +193     
==========================================
+ Hits        64216    64410     +194     
+ Misses       2280     2279       -1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Mark reader as exhausted on error to prevent misleading checksum mismatches - Refactor compute_crc32c for early return to follow style guide

…ct-checksum-branch

joshuatants

Can this PR be split s.t. that are at most 200 new LOC per PR?

xlai20 · 2026-07-27T05:10:34Z

Can this PR be split s.t. that are at most 200 new LOC per PR?

It is not a good idea to to split this PR because the PR is doing 1 single thing (i.e., adding object-level checksum for gRPC read), and also, most of the changes are tests and comments.

Below is the stats:

Production Code (~130 lines total)
- Executable Code: 70 lines
- Comments & RustDocs: 54 lines
- Blank Lines: 6 lines
Test Code (~361 lines total)

As you can see, the actual logic added to open_object.rs, range_reader.rs, etc. is merely 70 lines of code.

joshuatants · 2026-07-28T03:34:35Z

Can this PR be split s.t. that are at most 200 new LOC per PR?

It is not a good idea to to split this PR because the PR is doing 1 single thing (i.e., adding object-level checksum for gRPC read), and also, most of the changes are tests and comments.

Below is the stats:

Production Code (~130 lines total)

Executable Code: 70 lines

Comments & RustDocs: 54 lines

Blank Lines: 6 lines

Test Code (~361 lines total)

As you can see, the actual logic added to open_object.rs, range_reader.rs, etc. is merely 70 lines of code.

I review tests too. Perhaps for PRs like these we can have the logic mainly in 1 PR and tests in another.

Let's do that for future PRs. I reviewed the ones in this one already.

joshuatants · 2026-07-28T03:23:51Z

+        // Note: We do not skip checksums for gzip-encoded objects (decompressive transcoding)
+        // because gRPC automatic object decompression is not supported by the server.
+        // The client receives the raw compressed bytes which match the stored checksum.
+        let checksum = if matches!(range, RequestedRange::Offset(0)) {


If the user makes a range call that results in RequestedRange::Segment { 0, 0 }, this will still return the whole object but the checksum won't be calculated.

A more robust detection method is to either convert the RequestadRange into the actual offset and length used in the RPC call (I think this function already exists somewhere in the code), or handle all the possible RequestedRange cases.

joshuatants · 2026-07-28T03:25:04Z


+    /// Enables computation of MD5 checksums.
+    ///
+    /// By default, MD5 checksums are disabled, as they are not supported by the GCS gRPC API.


If they're not supported by gRPC, and open-object is gRPC, then why offer this setter?

joshuatants · 2026-07-28T03:32:05Z

    async fn next(&mut self) -> Option<crate::Result<bytes::Bytes>> {
-        let msg = self.inner.recv().await?;
-        Some(msg.map_err(Error::io))
+        if self.exhausted {


Let's take this opportunity to improve the readability of the code. Let's add:

a docstring to the function

comments at suitable locations, e.g. logical blocks, to explain what the code does. For example, what would be helpful here is a comment explaining what each match case handles

joshuatants · 2026-07-28T03:37:30Z

+    }
+
+    #[tokio::test]
+    async fn object_with_checksum_bypassed_due_to_ranged_read() -> anyhow::Result<()> {


Is it possible to have the test enable a checksum and then perform a ranged read to verify that no checksum is calculated?

Right now, what the test verifies is that if Checksum::default is passed in, then no validation is performed. Which is a valuable test, but not what the name suggests.

joshuatants · 2026-07-28T03:41:42Z

        Ok(())
    }

+    #[tokio::test]


Doesn't need to be async

joshuatants · 2026-07-28T03:47:33Z

+            }
+            None => {
+                self.exhausted = true;
+                if let Some(expected) = &self.object.checksums {


To double check: if it's possible for object.checksums to be None, then I think that in that case we should skip calling self.checksum.update in order to save computation

joshuatants · 2026-07-28T03:49:37Z

+                    got_err = true;
+                    let fmt = format!("{e:?}");
+                    assert!(fmt.contains("ChecksumMismatch"), "{fmt}");


Instead of checking the error string, can the error itself be checked like what is done in object_with_checksum_mismatch?

xlai20 added 6 commits July 22, 2026 10:01

first commit

2ddf3f2

Add test

474546b

Add integration tests

35da34a

Enforce checksum validation on gRPC for gzipped objects

f87f857

More doctest

5182c80

format

604bbb0

product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jul 22, 2026

gemini-code-assist Bot reviewed Jul 22, 2026

View reviewed changes

Comment thread src/storage/src/storage/bidi/range_reader.rs Outdated

Comment thread src/storage/src/storage/open_object.rs

xlai20 added 4 commits July 24, 2026 04:32

Apply gemini-code-assist review feedback

2d137f1

- Mark reader as exhausted on error to prevent misleading checksum mismatches - Refactor compute_crc32c for early return to follow style guide

format

0b9224b

Add unit test coverage for compute_md5 and compute_crc32c toggles

50cd90e

Merge remote-tracking branch 'upstream/main' into gRPC-bidi-read-obje…

da363b9

…ct-checksum-branch

xlai20 marked this pull request as ready for review July 24, 2026 09:15

xlai20 requested review from a team as code owners July 24, 2026 09:15

xlai20 requested review from joshuatants and vsharonlynn July 24, 2026 09:16

joshuatants requested changes Jul 27, 2026

View reviewed changes

Comment thread src/storage/src/storage/open_object.rs Outdated

Comment thread src/storage/src/storage/open_object.rs

Comment thread src/storage/src/storage/bidi/transport.rs Outdated

Comment thread src/storage/src/storage/bidi/transport.rs Outdated

xlai20 added 3 commits July 27, 2026 05:51

Refactor compute_md5 to use mut self

3d3ee7e

avoid double negation

7bd213f

Use shadowing for checksum in map_range

e01bbb2

xlai20 requested a review from joshuatants July 27, 2026 07:22

joshuatants requested changes Jul 28, 2026

View reviewed changes

Uh oh!

Conversation

xlai20 commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Key Changes:

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joshuatants left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xlai20 commented Jul 27, 2026

Uh oh!

joshuatants commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

joshuatants Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xlai20 commented Jul 22, 2026 •

edited

Loading

codecov Bot commented Jul 22, 2026 •

edited

Loading

joshuatants commented Jul 28, 2026 •

edited

Loading