Skip to content

fix(storage): detect damaged data in normal downloads #14768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
diwu-sf wants to merge 3 commits into
base: main
Choose a base branch
from
+328 −14
Open

fix(storage): detect damaged data in normal downloads #14768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11b1829
fix(storage): detect damaged data in normal downloads
diwu-sf Oct 25, 2026
5198dd6
fix(storage): avoid false checksum errors on resumed and GZIP reads
diwu-sf Oct 25, 2026
18b1522
fix(storage): trim encoding headers before comparing them
diwu-sf Oct 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...d-storage/src/main/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,10 @@ private ScatteringByteChannel open() {
if (shouldValidate && expectedCrc32cBase64 != null) {
this.hashingInputStream = new HashingInputStream(Hashing.crc32c(), content);
content = this.hashingInputStream;
} else {
// A resumed read returns only the remaining bytes, which can't be checked against the
// full-object crc32c. Drop the hash from the previous response so it isn't compared.
this.hashingInputStream = null;
}

ReadableByteChannel rbc = Channels.newChannel(content);
Expand Down
22 changes: 17 additions & 5 deletions ...ogle-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageRpcHasherHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.storage;

import com.google.api.client.http.HttpHeaders;
import com.google.api.client.http.HttpResponse;
import com.google.api.core.InternalApi;
import com.google.common.hash.Hashing;
Expand All @@ -25,6 +26,7 @@
import java.io.IOException;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.util.Locale;
import java.util.Map;
import java.util.function.Supplier;

Expand Down Expand Up @@ -106,19 +108,29 @@ private static boolean isFullObjectResponse(HttpResponse response) {
}

private boolean isTranscoded(HttpResponse response) {
com.google.api.client.http.HttpHeaders headers = response.getHeaders();
HttpHeaders headers = response.getHeaders();
String storedEncoding =
HttpClientContext.firstHeaderValue(headers, "x-goog-stored-content-encoding");
String storedLength =
HttpClientContext.firstHeaderValue(headers, "x-goog-stored-content-length");
return storedEncoding != null || storedLength != null || isDecompressedByClient(response);
String encoding = headers.getContentEncoding();
// Stored-content headers are also present on untranscoded responses. Only a change in
// encoding indicates server-side transcoding; an absent Content-Encoding means identity.
return (storedEncoding != null
&& !storedEncoding
.trim()
.equalsIgnoreCase(encoding == null ? "identity" : encoding.trim()))
|| isDecompressedByClient(response);
Comment on lines +117 to +121

@gemini-code-assist gemini-code-assist Bot Oct 25, 2026

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.

medium

If the Content-Encoding header contains leading or trailing whitespace (e.g., " GZIP " or "gzip "), the case-insensitive comparison storedEncoding.equalsIgnoreCase(...) will fail because storedEncoding (e.g., "gzip") is not trimmed. This causes isTranscoded to incorrectly return true even when the client requested raw bytes (returnRaw = true) and the data is not transcoded. As a result, checksum validation will be skipped. Trimming both storedEncoding and encoding before comparison ensures robust and correct validation behavior.

Suggested change
return (storedEncoding != null
&& !storedEncoding.equalsIgnoreCase(encoding == null ? "identity" : encoding))
|| isDecompressedByClient(response);
return (storedEncoding != null
&& !storedEncoding.trim().equalsIgnoreCase(encoding == null ? "identity" : encoding.trim()))
|| isDecompressedByClient(response);

}

private boolean isDecompressedByClient(HttpResponse response) {
boolean returnRaw = response.getRequest().getResponseReturnRawInputStream();
if (!returnRaw) {
String encoding = response.getHeaders().getContentEncoding();
return encoding != null && encoding.contains("gzip");
if (encoding == null) {
return false;
}
// Match the normalization HttpResponse#getContent uses to decide whether to gunzip.
String normalized = encoding.trim().toLowerCase(Locale.ENGLISH);
return normalized.equals("gzip") || normalized.equals("x-gzip");
}
return false;
}
Expand Down
107 changes: 107 additions & 0 deletions ...orage/src/test/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@
import static org.junit.Assert.assertTrue;

import com.google.api.client.http.HttpTransport;
import com.google.api.client.http.LowLevelHttpRequest;
import com.google.api.client.http.LowLevelHttpResponse;
import com.google.api.client.json.gson.GsonFactory;
import com.google.api.client.testing.http.MockLowLevelHttpRequest;
import com.google.api.client.testing.http.MockLowLevelHttpResponse;
import com.google.api.core.SettableApiFuture;
import com.google.api.gax.retrying.BasicResultRetryAlgorithm;
import com.google.api.services.storage.Storage;
import com.google.api.services.storage.model.StorageObject;
import com.google.cloud.storage.ApiaryUnbufferedReadableByteChannel.ApiaryReadRequest;
Expand All @@ -33,12 +37,17 @@
import com.google.cloud.storage.UnbufferedReadableByteChannelSession.UnbufferedReadableByteChannel;
import com.google.cloud.storage.spi.v1.AuditingHttpTransport;
import com.google.common.collect.ImmutableMap;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.SequenceInputStream;
import java.net.SocketException;
import java.nio.ByteBuffer;
import java.nio.channels.Channels;
import java.nio.channels.WritableByteChannel;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.logging.Handler;
Expand Down Expand Up @@ -228,6 +237,104 @@ public void testRead_partialRangeNoCrc32cValidation() throws IOException {
}
}

@Test
public void testRead_resumedAfterFailure_skipsCrc32cValidation() throws IOException {
// GCS sometimes returns the full-object crc32c on the resumed response.
assertResumedReadReturnsFullContent(true);
}

@Test
public void testRead_resumedAfterFailure_noChecksumOnResume() throws IOException {
// GCS sometimes omits x-goog-hash on the resumed response.
assertResumedReadReturnsFullContent(false);
}

private void assertResumedReadReturnsFullContent(boolean checksumOnResume) throws IOException {
int splitAt = 5;
List<String> rangeHeaders = new ArrayList<>();
HttpTransport transport =
new HttpTransport() {
@Override
protected LowLevelHttpRequest buildRequest(String method, String url) {
return new MockLowLevelHttpRequest() {
@Override
public LowLevelHttpResponse execute() {
rangeHeaders.add(getFirstHeaderValue("range"));
MockLowLevelHttpResponse response =
new MockLowLevelHttpResponse()
.addHeader("x-goog-generation", "12345")
.addHeader("x-goog-stored-content-encoding", "identity")
.addHeader(
"x-goog-stored-content-length", String.valueOf(CONTENT_BYTES.length));
boolean isResume = rangeHeaders.size() > 1;
if (!isResume || checksumOnResume) {
response.addHeader("x-goog-hash", "crc32c=" + CORRECT_CRC32C_BASE64);
}
if (!isResume) {
// The connection drops after the first few bytes.
return response
.setStatusCode(200)
.setContent(
new SequenceInputStream(
new ByteArrayInputStream(CONTENT_BYTES, 0, splitAt),
new InputStream() {
@Override
public int read() throws IOException {
throw new SocketException("Connection reset");
}
}));
}
return response
.setStatusCode(206)
.addHeader(
"Content-Range",
String.format(
"bytes %d-%d/%d",
splitAt, CONTENT_BYTES.length - 1, CONTENT_BYTES.length))
.setContent(Arrays.copyOfRange(CONTENT_BYTES, splitAt, CONTENT_BYTES.length));
}
};
}
};
Storage storageClient =
new Storage.Builder(transport, GsonFactory.getDefaultInstance(), null)
.setApplicationName("test")
.build();

StorageObject from = new StorageObject().setBucket("bucket").setName("blob");
ApiaryReadRequest apiaryReadRequest =
new ApiaryReadRequest(from, ImmutableMap.of(), ByteRangeSpec.nullRange());

try (ApiaryUnbufferedReadableByteChannel channel =
new ApiaryUnbufferedReadableByteChannel(
apiaryReadRequest,
storageClient,
SettableApiFuture.create(),
RetrierWithAlg.attemptOnce(),
// Retry the dropped connection only, so a checksum mismatch surfaces.
new BasicResultRetryAlgorithm<Object>() {
@Override
public boolean shouldRetry(Throwable previousThrowable, Object previousResponse) {
return previousThrowable instanceof SocketException;
}
},
Hasher.defaultHasher())) {

ByteArrayOutputStream out = new ByteArrayOutputStream();
try (WritableByteChannel w = Channels.newChannel(out)) {
ByteBuffer buf = ByteBuffer.allocate(4096);
while (channel.read(new ByteBuffer[] {buf}, 0, 1) != -1) {
buf.flip();
w.write(buf);
buf.clear();
}
}

assertArrayEquals(CONTENT_BYTES, out.toByteArray());
assertThat(rangeHeaders).containsExactly(null, "bytes=" + splitAt + "-").inOrder();
}
}

@Test
public void logsWarning_whenReceivingMoreBytesThanRequested() throws IOException {
byte[] content = "0123456789extra_bytes".getBytes();
Expand Down
106 changes: 105 additions & 1 deletion ...-cloud-storage/src/test/java/com/google/cloud/storage/HttpStorageRpcHasherHelperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ public void testShouldValidate_successStatus200() throws IOException {
public void testShouldValidate_successStatus206_fullContentRange() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("Content-Range", "bytes 0-499/500");
headers.put("x-goog-stored-content-encoding", "identity");
headers.put("x-goog-stored-content-length", "500");
HttpResponse response = createHttpResponse(206, headers, true);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
Expand All @@ -138,6 +140,8 @@ public void testShouldValidate_successStatus206_fullContentRange() throws IOExce
public void testShouldValidate_successStatus206_partialContentRange() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("Content-Range", "bytes 100-499/500");
headers.put("x-goog-stored-content-encoding", "identity");
headers.put("x-goog-stored-content-length", "500");
HttpResponse response = createHttpResponse(206, headers, true);

assertFalse(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
Expand All @@ -153,11 +157,111 @@ public void testShouldValidate_transcoded_storedEncoding() throws IOException {
}

@Test
public void testShouldValidate_transcoded_storedLength() throws IOException {
public void testShouldValidate_storedLength() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-length", "123");
HttpResponse response = createHttpResponse(200, headers, true);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_matchingStoredLength() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-length", "123");
headers.put("Content-Length", "123");
HttpResponse response = createHttpResponse(200, headers, true);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_storedIdentityEncoding() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "identity");
HttpResponse response = createHttpResponse(200, headers, false);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_matchingStoredEncoding_rawInputStream() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "gzip");
headers.put("Content-Encoding", "gzip");
HttpResponse response = createHttpResponse(200, headers, true);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_storedEncodingWithWhitespace_rawInputStream() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", " \tgzip \t");
headers.put("Content-Encoding", "GZIP");
HttpResponse response = createHttpResponse(200, headers, true);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_responseEncodingWithWhitespace_rawInputStream()
throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "gzip");
headers.put("Content-Encoding", " \tGZIP \t");
HttpResponse response = createHttpResponse(200, headers, true);

assertTrue(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_encodingWithWhitespace_decompressedByClient() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", " \tgzip \t");
headers.put("Content-Encoding", " \tGZIP \t");
HttpResponse response = createHttpResponse(200, headers, false);

assertFalse(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_transcoded_identityEncoding() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "gzip");
headers.put("Content-Encoding", "identity");
HttpResponse response = createHttpResponse(200, headers, true);

assertFalse(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_matchingStoredEncoding_decompressedByClient() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "gzip");
headers.put("Content-Encoding", "gzip");
HttpResponse response = createHttpResponse(200, headers, false);

assertFalse(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_upperCaseStoredEncoding_decompressedByClient() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "GZIP");
headers.put("Content-Encoding", "GZIP");
HttpResponse response = createHttpResponse(200, headers, false);

assertFalse(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

@Test
public void testShouldValidate_xGzipEncoding_decompressedByClient() throws IOException {
Map<String, String> headers = new HashMap<>();
headers.put("x-goog-stored-content-encoding", "x-gzip");
headers.put("Content-Encoding", "x-gzip");
HttpResponse response = createHttpResponse(200, headers, false);

assertFalse(HttpStorageRpcHasherHelper.INSTANCE.shouldValidate(response));
}

Expand Down
Loading
Loading