feat(Storage): Enable full object checksum validation for resumable uploads by mahendra-google · Pull Request #15395 · googleapis/google-cloud-dotnet

mahendra-google · 2026-01-21T10:57:43Z

This PR enables full object checksum validation (specifically CRC32C) for JSON-based resumable uploads, ensuring end-to-end data integrity for multi-chunk transfers. Please see b/461996128

gemini-code-assist · 2026-01-21T10:58:01Z

Summary of Changes

Hello @mahendra-google, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces robust data integrity checks for resumable uploads by integrating CRC32C checksum validation. This enhancement ensures that multi-chunk transfers to Google Cloud Storage are verified end-to-end, preventing data corruption. The changes also update the client's error handling to align with the server's response to checksum mismatches.

Highlights

Enabled CRC32C Checksum Validation: Implemented end-to-end CRC32C checksum validation for JSON-based resumable uploads, ensuring data integrity for multi-chunk transfers.
Updated Upload Validation Behavior: Modified integration tests to reflect the new behavior where invalid checksums during resumable uploads now result in a GoogleApiException with a BadRequest status, indicating server-side rejection.
Enhanced Stream Handling for Checksum Calculation: Improved the LengthOnlyStream implementation to support seeking and reading, which is crucial for calculating the full CRC32C hash of the stream before the final chunk is sent.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request enables full object checksum validation for resumable uploads by introducing a Crc32cHashInterceptor. This interceptor calculates the CRC32C hash of the upload stream and attaches it to the final chunk of a resumable upload, ensuring end-to-end data integrity. The implementation is solid, with proper handling of the interceptor's lifecycle. The integration tests have been correctly updated to reflect the new server-side validation behavior, where the server now rejects corrupted uploads with a BadRequest status. Additionally, LengthOnlyStream has been updated to be a more compliant Stream implementation. My review includes a couple of suggestions to optimize performance by avoiding string allocations in the new parsing logic.

krishnamd-jkp · 2026-02-23T10:04:02Z

Can you use UploadStreamInterceptor which already calculates the hash and use it here instead of recomputing the hash on final chunk?

mahendra-google · 2026-03-11T07:12:57Z

Can you use UploadStreamInterceptor which already calculates the hash and use it here instead of recomputing the hash on final chunk?

@krishnamd-jkp UploadStreamInterceptor does not calculate hash . Please see https://github.com/googleapis/google-api-dotnet-client/blob/main/Src/Support/Google.Apis/Upload/ResumableUpload.cs#L207

amanda-tarafa · 2026-05-04T23:55:41Z

+            {
+                _hashingStream = ContentStream as HashingStream;
+                _interceptor = new Crc32cHashInterceptor(this, _hashingStream, _service);
+                _service?.HttpClient?.MessageHandler?.AddExecuteInterceptor(_interceptor);


The same service may be used in parallel for many different requests, including for many different uploads. You'll have interceptors that will be applied to all requests???

Even when you check the URI, assuming that check is enough in all cases, you are adding latency to all requests.

What am I missing?

You make a very fair point regarding the service-wide scope of the interceptor.

While I have included logic to remove the interceptor upon upload completion or failure (to prevent long-term memory leaks on line 94 of the CustomMediaUpload class), I recognize that during high-concurrency periods, the interceptor chain could grow. This would indeed force every request—even those unrelated to this upload—to perform the URI check.

I initially assumed the latency of a reference comparison would be negligible, but I see how this architectural choice could be suboptimal for shared service instances. Since I want to ensure this implementation aligns with our performance standards, I am very open to your suggestions.

Updated the URI comparison to include a reference equality check (ReferenceEquals) before performing a value-based .Equals(). This introduces an $O(1)$ short-circuit optimization especially for requests other than upload . For unrelated requests , we can now fail fast at the CPU register level by comparing memory addresses rather than performing a character-by-character string or component validation.This bypasses the $O(n)$ complexity of a full URI equality check, significantly reducing CPU cycles and latency for the majority of requests where the object references do not match.

My recommendation here is that you add an event on the base upload type that triggers on the last request only. This would be in Google.Apis. (Note that this is different to the initial proposal in Google.Apis which meant changing upload logic for all Discovery based libraries).

@amanda-tarafa Recommended approach is being implemented. I will be raising separate PRs for both Google.Apis and storage library soon.

Thanks. Because that is a change in the core libraries the .NET Cloud SDK team will need to review thorughly. A 1-2 pages design document describing the proposed change, before the PR is sent for review, should help expediting things.

amanda-tarafa · 2026-05-11T19:21:58Z

+            {
+                _hashingStream = ContentStream as HashingStream;
+                _interceptor = new Crc32cHashInterceptor(this, _hashingStream, _service);
+                _service?.HttpClient?.MessageHandler?.AddExecuteInterceptor(_interceptor);


My recommendation here is that you add an event on the base upload type that triggers on the last request only. This would be in Google.Apis. (Note that this is different to the initial proposal in Google.Apis which meant changing upload logic for all Discovery based libraries).

amanda-tarafa · 2026-05-11T19:25:20Z

-                GaxPreconditions.CheckEnumValue(validationMode, nameof(UploadObjectOptions.UploadValidationMode));
-                switch (validationMode)
-                {
-                    case UploadValidationMode.DeleteAndThrow:


Does this implementation respects both DeleteAndThrow and ThrowOnly? I don't see where the service would know the difference, so this would be another breaking change, that warrants a new major version bump.

I appreciate you flagging that—ensuring we maintain backward compatibility is definitely a priority.

The implementation does respect both DeleteAndThrow and ThrowOnly. In the CustomMediaUpload class, I am passing the UploadObjectOptions instance directly into the constructor. The logic is scoped so that we only calculate the hash if the UploadValidationMode is set to something other than None. This allows the service to distinguish between the modes effectively without altering the existing contract.

The hash is indeed only calculated if the value is set to other than None, that is correct.

The implementation does not respect the difference between DeleteAndThrow and ThrowOnly. These values are not being passed to the service, and they are not being enforced client side either as far as I can see.

ThrowOnly: Throws an exception if the hashes do not match, but the object remains in storage.

DeleteAndThrow: Throws an exception if the hashes do not match and removes the object from storage.

You are completely right, and thank you for pointing that out. The legacy DeleteAndThrow and ThrowOnly behaviors weren't being properly enforced .

To address this comprehensively and lean into a more robust architecture, I have refactored the implementation to leverage server-side validation instead of relying on client-side cleanup.

Here is a summary of the changes made in this update:

Introduced UploadValidationMode.RejectAndThrow: This leverages server-side validation, ensuring that invalid uploads are rejected automatically before the object is ever created.

Updated Default Upload Validation Mode & Tests: UploadObjectOptions now uses this new mode as the default behavior and UploadObjectTest has been updated to reflect this.

Deprecated Legacy Modes: We have deprecated ThrowOnly and DeleteAndThrow. Because server-side validation prevents the object from being created in the first place . By switching to server-side rejection, the object is blocked at the front door.

It satisfies DeleteAndThrow because the final state of the bucket is perfectly clean—no invalid object exists.

It satisfies (and improves upon) ThrowOnly because it alerts the user via an exception, but removes the dangerous side effect of leaving a corrupted file behind.

It satisfies (and improves upon) ThrowOnly because it alerts the user via an exception, but removes the dangerous side effect of leaving a corrupted file behind.

This is a breaking change. Deprecation is a signal that the behavior will stop working on the next major version. But while an element is deprecated, its behavior must be enforced. And it's not a dangerous side effect, it's a behavior explicitly requested by user when they specify ThrowOnly.

My review shouldn't be binding.

amanda-tarafa · 2026-05-21T17:13:16Z

When you are ready to merge, please use squash and merge so we don't get the 16 iteration commits on the repo history. As an alternative, rebase the PR on top of main into a single commit before merging.

…ploads

amanda-tarafa · 2026-06-05T16:35:24Z

Regarding do not merge see b/461996128#comment9

mahendra-google requested review from a team January 21, 2026 10:57

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

gemini-code-assist Bot reviewed Jan 21, 2026

View reviewed changes

Comment thread apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs Outdated

Comment thread apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs Outdated

mahendra-google force-pushed the enable-json-full-object-checksums branch from f443772 to d4e4802 Compare January 21, 2026 11:35

amanda-tarafa mentioned this pull request Jan 21, 2026

feat: Enable full object checksum validation for multi-chunk resumable uploads googleapis/google-api-dotnet-client#3096

Closed

krishnamd-jkp requested changes Feb 23, 2026

View reviewed changes

mahendra-google requested review from a team as code owners March 18, 2026 10:30

mahendra-google requested a review from krishnamd-jkp March 18, 2026 10:58

mahendra-google force-pushed the enable-json-full-object-checksums branch from 8086e46 to a371f67 Compare March 31, 2026 07:53

krishnamd-jkp reviewed Apr 2, 2026

View reviewed changes

mahendra-google requested a review from krishnamd-jkp April 8, 2026 05:59

krishnamd-jkp reviewed Apr 21, 2026

View reviewed changes

Comment thread apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs Outdated

Comment thread apis/Google.Cloud.Storage.V1/Google.Cloud.Storage.V1/CustomMediaUpload.cs Outdated

mahendra-google force-pushed the enable-json-full-object-checksums branch from 99c6617 to 54d1d1c Compare April 24, 2026 05:10

krishnamd-jkp approved these changes Apr 24, 2026

View reviewed changes

krishnamd-jkp requested a review from amanda-tarafa April 24, 2026 05:46

amanda-tarafa previously requested changes May 5, 2026

View reviewed changes

mahendra-google force-pushed the enable-json-full-object-checksums branch 2 times, most recently from 361aff2 to e4ee73a Compare May 7, 2026 08:59

mahendra-google requested a review from amanda-tarafa May 8, 2026 05:12

amanda-tarafa reviewed May 11, 2026

View reviewed changes

amanda-tarafa self-requested a review May 11, 2026 19:26

mahendra-google requested a review from krishnamd-jkp May 15, 2026 05:17

mahendra-google force-pushed the enable-json-full-object-checksums branch from 02df9af to 23c8b2e Compare May 22, 2026 05:47

feat(Storage): Enable full object checksum validation for resumable u…

1367263

…ploads

mahendra-google force-pushed the enable-json-full-object-checksums branch from 23c8b2e to 1367263 Compare May 22, 2026 05:51

amanda-tarafa added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 5, 2026

Uh oh!

Conversation

mahendra-google commented Jan 21, 2026

Uh oh!

gemini-code-assist Bot commented Jan 21, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krishnamd-jkp commented Feb 23, 2026

Uh oh!

mahendra-google commented Mar 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amanda-tarafa commented May 21, 2026

Uh oh!

amanda-tarafa commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants