Repository navigation
Conversation
…essed GCS returns x-goog-stored-content-encoding and x-goog-stored-content-length on every object download, but HttpStorageRpcHasherHelper treated the presence of either header as transcoding, so CRC32C validation was skipped for all HTTP reads (readAllBytes, downloadTo, reader). Only skip validation when the object is stored gzip-compressed and was served decompressed, or when the HTTP client decompressed it. Fixes googleapis#14773
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the transcoding detection logic in HttpStorageRpcHasherHelper to prevent skipping checksum validation on normal downloads. Previously, the presence of x-goog-stored-content-encoding or x-goog-stored-content-length headers (which GCS returns on all downloads) incorrectly flagged responses as transcoded. The updated logic specifically checks if the object was stored as gzip but served decompressed. Corresponding unit tests have been added and updated to ensure proper checksum validation behavior. I have no further feedback to provide as the changes are well-implemented and covered by tests.
…) download Now that CRC32C validation runs on HTTP downloads, a reader() download that is resumed after a broken stream kept the hashing stream from the first (full object) response. The resumed response is partial and is not validated, so at EOF the hash of only the first part was compared with the full object checksum, reporting a false ChecksumMismatchException. Clear the hashing stream when the current response is not validated.
…g client decompression GCS echoes an object's Content-Encoding metadata as-is (e.g. "GZIP"), and the HTTP client decompresses when the value matches gzip case-insensitively. isDecompressedByClient used a case-sensitive check, so such responses were validated against the stored object's checksum after being decompressed, reporting a false ChecksumMismatchException.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GCS returns
x-goog-stored-content-encodingandx-goog-stored-content-lengthon every object download, including objects that are not compressed:HttpStorageRpcHasherHelper.isTranscoded()treated the presence of either header as a sign of decompressive transcoding. SoshouldValidate()always returnedfalse, and CRC32C validation was skipped for all HTTP reads (readAllBytes,downloadTo,reader).In this pr we fix it by checking only the following:
x-goog-stored-content-encoding: gzip) and was served decompressed (noContent-Encoding: gzip), orisDecompressedByClient).Reference: Transcoding of gzip-compressed files. GCS only decompresses objects stored gzip-compressed with
Content-Encoding: gzip.Once validation is enabled, a
reader()download that is resumed after a broken stream would fail incorrectly:Related fix: case-insensitive
Content-EncodinginisDecompressedByClientGCS echoes an object's
Content-Encodingmetadata as-is, so an object uploaded withContent-Encoding: GZIPis served withContent-Encoding: GZIP. The HTTP client decompresses when the value matchesgzip/x-gzipcase-insensitively, butisDecompressedByClientused a case-sensitivecontains("gzip"). With validation now running, such an object read withshouldReturnRawInputStream(false)reported a falseChecksumMismatchException(verified against GCS). Both checks now shareisGzip(), which trims and lowercases the value.