Skip to content

feat(go): Add full object checksum for negative offsets > size#20026

Merged
krishnamd-jkp merged 3 commits into
googleapis:mainfrom
krishnamd-jkp:read_checksum_negative_offset
Jun 22, 2026
Merged

feat(go): Add full object checksum for negative offsets > size#20026
krishnamd-jkp merged 3 commits into
googleapis:mainfrom
krishnamd-jkp:read_checksum_negative_offset

Conversation

@krishnamd-jkp

Copy link
Copy Markdown
Contributor

No description provided.

@krishnamd-jkp krishnamd-jkp requested review from a team as code owners June 19, 2026 07:17

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the gRPC storage client to use the resolved startOffset instead of params.offset when determining whether to perform CRC checksum verification, ensuring that CRC checks are correctly enabled when reading an entire object with a negative offset that resolves to 0. A test case was also expanded to cover negative offsets. However, a critical bug was introduced in storage/grpc_client.go where an arbitrary + 1 was added to the CRC32C calculation, which will corrupt the checksum and cause verification to fail for all reads. This must be reverted.

Comment thread storage/grpc_client.go Outdated
@krishnamd-jkp krishnamd-jkp force-pushed the read_checksum_negative_offset branch from cbd08ce to 2a63107 Compare June 19, 2026 07:19
@krishnamd-jkp krishnamd-jkp added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2026
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2026
@krishnamd-jkp

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the gRPC storage client and reader to correctly calculate the start offset before performing CRC checksum checks. This ensures that negative offsets resolving to the beginning of the object are properly checksummed. Additionally, the test suite is expanded to cover negative offsets. The review feedback suggests closing both the gRPC client and the range reader in the newly updated tests to prevent resource and connection leaks.

Comment thread storage/client_test.go
Comment thread storage/client_test.go
Comment thread storage/grpc_client.go
Comment thread storage/grpc_reader.go
Comment thread storage/grpc_reader.go

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if any documentation need to be updated for this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are just covering a corner case, should not need any documentation change.

@krishnamd-jkp krishnamd-jkp merged commit a04d980 into googleapis:main Jun 22, 2026
16 checks passed
krishnamd-jkp added a commit that referenced this pull request Jun 26, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.63.0](storage/v1.62.2...storage/v1.63.0)
(2026-06-25)


### Features

* **go:** Add full object checksum for negative offsets > size
([#20026](#20026))
([a04d980](a04d980))
* **storage:** Add client feature tracking in HTTP client
([#14691](#14691))
([319cc4c](319cc4c))
* **storage:** App Centric Observability
([#14685](#14685))
([c3273bb](c3273bb))
* **storage:** Read checksums in gRPC partial reads
([#14586](#14586))
([d29f68a](d29f68a))
* **storage:** Support deleteSourceObjects option in object compose
([#14704](#14704))
([0d2d680](0d2d680))
* Update API sources and regenerate
([#14701](#14701))
([a9b7921](a9b7921))


### Bug Fixes

* **storage:** Add server closed idle connection to retriable errors
([#14594](#14594))
([a6bd392](a6bd392))
* **storage:** Fix race condition during retries in gRPC writer
([#14649](#14649))
([c781a75](c781a75))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Krishnam Dhanush <krishnamd@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants