diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannel.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannel.java index 25757921ba54..7c1ccbb21570 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannel.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannel.java @@ -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); diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageRpcHasherHelper.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageRpcHasherHelper.java index 7d1bf13ff629..fd1134ad5553 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageRpcHasherHelper.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageRpcHasherHelper.java @@ -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; @@ -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; @@ -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); } 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; } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannelTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannelTest.java index ac0a7c70fd73..4295be137471 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannelTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/ApiaryUnbufferedReadableByteChannelTest.java @@ -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; @@ -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; @@ -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 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() { + @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(); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/HttpStorageRpcHasherHelperTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/HttpStorageRpcHasherHelperTest.java index 6a8777d5ba92..6b8718085a24 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/HttpStorageRpcHasherHelperTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/HttpStorageRpcHasherHelperTest.java @@ -129,6 +129,8 @@ public void testShouldValidate_successStatus200() throws IOException { public void testShouldValidate_successStatus206_fullContentRange() throws IOException { Map 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)); @@ -138,6 +140,8 @@ public void testShouldValidate_successStatus206_fullContentRange() throws IOExce public void testShouldValidate_successStatus206_partialContentRange() throws IOException { Map 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)); @@ -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 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 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 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 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 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 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 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 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 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 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 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)); } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/spi/v1/HttpStorageRpcChecksumTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/spi/v1/HttpStorageRpcChecksumTest.java index 540e14eb045e..95ff583f92b8 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/spi/v1/HttpStorageRpcChecksumTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/spi/v1/HttpStorageRpcChecksumTest.java @@ -21,6 +21,7 @@ import com.google.api.client.testing.http.MockLowLevelHttpResponse; import com.google.cloud.NoCredentials; +import com.google.cloud.ReadChannel; import com.google.cloud.TransportOptions; import com.google.cloud.http.HttpTransportOptions; import com.google.cloud.storage.BlobId; @@ -32,7 +33,10 @@ import com.google.common.io.BaseEncoding; import com.google.common.primitives.Ints; import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.zip.GZIPOutputStream; import org.junit.Test; public final class HttpStorageRpcChecksumTest { @@ -55,8 +59,14 @@ public void testReadAllBytes_successfulCrc32cValidation() throws Exception { MockLowLevelHttpResponse response = new MockLowLevelHttpResponse() .setContentType("application/octet-stream") - .setHeaderNames(ImmutableList.of("x-goog-hash")) - .setHeaderValues(ImmutableList.of("crc32c=" + CONTENT_CRC32C_BASE64)) + .setHeaderNames( + ImmutableList.of( + "x-goog-hash", + "x-goog-stored-content-encoding", + "x-goog-stored-content-length")) + .setHeaderValues( + ImmutableList.of( + "crc32c=" + CONTENT_CRC32C_BASE64, "identity", String.valueOf(CONTENT.length))) .setContent(new String(CONTENT, StandardCharsets.UTF_8)) .setStatusCode(200); @@ -71,8 +81,14 @@ public void testReadAllBytes_failedCrc32cValidation() throws Exception { MockLowLevelHttpResponse response = new MockLowLevelHttpResponse() .setContentType("application/octet-stream") - .setHeaderNames(ImmutableList.of("x-goog-hash")) - .setHeaderValues(ImmutableList.of("crc32c=" + BAD_CRC32C_BASE64)) + .setHeaderNames( + ImmutableList.of( + "x-goog-hash", + "x-goog-stored-content-encoding", + "x-goog-stored-content-length")) + .setHeaderValues( + ImmutableList.of( + "crc32c=" + BAD_CRC32C_BASE64, "identity", String.valueOf(CONTENT.length))) .setContent(new String(CONTENT, StandardCharsets.UTF_8)) .setStatusCode(200); @@ -93,8 +109,14 @@ public void testDownloadTo_successfulCrc32cValidation() throws Exception { MockLowLevelHttpResponse response = new MockLowLevelHttpResponse() .setContentType("application/octet-stream") - .setHeaderNames(ImmutableList.of("x-goog-hash")) - .setHeaderValues(ImmutableList.of("crc32c=" + CONTENT_CRC32C_BASE64)) + .setHeaderNames( + ImmutableList.of( + "x-goog-hash", + "x-goog-stored-content-encoding", + "x-goog-stored-content-length")) + .setHeaderValues( + ImmutableList.of( + "crc32c=" + CONTENT_CRC32C_BASE64, "identity", String.valueOf(CONTENT.length))) .setContent(new String(CONTENT, StandardCharsets.UTF_8)) .setStatusCode(200); @@ -110,8 +132,14 @@ public void testDownloadTo_failedCrc32cValidation() throws Exception { MockLowLevelHttpResponse response = new MockLowLevelHttpResponse() .setContentType("application/octet-stream") - .setHeaderNames(ImmutableList.of("x-goog-hash")) - .setHeaderValues(ImmutableList.of("crc32c=" + BAD_CRC32C_BASE64)) + .setHeaderNames( + ImmutableList.of( + "x-goog-hash", + "x-goog-stored-content-encoding", + "x-goog-stored-content-length")) + .setHeaderValues( + ImmutableList.of( + "crc32c=" + BAD_CRC32C_BASE64, "identity", String.valueOf(CONTENT.length))) .setContent(new String(CONTENT, StandardCharsets.UTF_8)) .setStatusCode(200); @@ -128,6 +156,65 @@ public void testDownloadTo_failedCrc32cValidation() throws Exception { } } + @Test + public void testReader_storedContentHeaders_failedCrc32cValidation() throws Exception { + MockLowLevelHttpResponse response = + new MockLowLevelHttpResponse() + .setContentType("application/octet-stream") + .addHeader("x-goog-hash", "crc32c=" + BAD_CRC32C_BASE64) + .addHeader("x-goog-stored-content-encoding", "identity") + .addHeader("x-goog-stored-content-length", String.valueOf(CONTENT.length)) + .addHeader("Content-Length", String.valueOf(CONTENT.length)) + .addHeader("x-goog-generation", "12345") + .setContent(CONTENT) + .setStatusCode(200); + + try (Storage storage = createMockStorage(response); + ReadChannel reader = storage.reader(BlobId.of(BUCKET, BLOB))) { + ByteBuffer buffer = ByteBuffer.allocate(4096); + IOException ex = + assertThrows( + IOException.class, + () -> { + while (reader.read(buffer) != -1) { + buffer.clear(); + } + }); + assertThat(ex.getMessage()).contains("Mismatch checksum value"); + } + } + + @Test + public void testReadAllBytes_upperCaseGzipDecompressedByClient_skipsCrc32cValidation() + throws Exception { + ByteArrayOutputStream gzipped = new ByteArrayOutputStream(); + try (GZIPOutputStream gz = new GZIPOutputStream(gzipped)) { + gz.write(CONTENT); + } + byte[] stored = gzipped.toByteArray(); + String storedCrc32cBase64 = + BaseEncoding.base64() + .encode( + Ints.toByteArray( + com.google.common.hash.Hashing.crc32c().hashBytes(stored).asInt())); + // The crc32c describes the stored gzip bytes, but the client returns them decompressed. + MockLowLevelHttpResponse response = + new MockLowLevelHttpResponse() + .setContentType("text/plain") + .addHeader("x-goog-hash", "crc32c=" + storedCrc32cBase64) + .addHeader("x-goog-stored-content-encoding", "GZIP") + .addHeader("x-goog-stored-content-length", String.valueOf(stored.length)) + .addHeader("Content-Encoding", "GZIP") + .setContentEncoding("GZIP") + .setContent(stored) + .setStatusCode(200); + + try (Storage storage = createMockStorage(response)) { + byte[] bytes = storage.readAllBytes(BlobId.of(BUCKET, BLOB)); + assertThat(bytes).isEqualTo(CONTENT); + } + } + @Test public void testReadAllBytes_noChecksumHeader_expectSuccess() throws Exception { MockLowLevelHttpResponse response =