diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-6d1ba46.json b/.changes/next-release/bugfix-AWSSDKforJavav2-6d1ba46.json new file mode 100644 index 000000000000..30273f39a4af --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-6d1ba46.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS SDK for Java v2", + "contributor": "", + "description": "Fixed an issue where responses with a non-zero x-amz-crc32 header but no response body were silently returned to the caller as empty results. The SDK now throws Crc32MismatchException that is retryable when a non-zero CRC32 is claimed but no body is delivered, matching v1 SDK behavior." +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/http/Crc32Validation.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/http/Crc32Validation.java index a42bbbef0959..62aa23303f9b 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/http/Crc32Validation.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/http/Crc32Validation.java @@ -20,6 +20,7 @@ import java.util.Optional; import java.util.zip.GZIPInputStream; import software.amazon.awssdk.annotations.SdkProtectedApi; +import software.amazon.awssdk.core.exception.Crc32MismatchException; import software.amazon.awssdk.core.internal.util.Crc32ChecksumValidatingInputStream; import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.http.SdkHttpFullResponse; @@ -37,6 +38,16 @@ public static SdkHttpFullResponse validate(boolean calculateCrc32FromCompressedD SdkHttpFullResponse httpResponse) { if (!httpResponse.content().isPresent()) { + // CRC32 of zero bytes is 0, so a 0 header is a valid match for an empty body. + // A non-zero header with no content means the Crc32 mismatch error. + Optional expectedChecksum = getCrc32Checksum(httpResponse); + if (expectedChecksum.isPresent() && expectedChecksum.get() != 0L) { + throw Crc32MismatchException.builder() + .message(String.format("Expected %d as the Crc32 checksum but the response " + + "had no content", + expectedChecksum.get())) + .build(); + } return httpResponse; } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/http/Crc32ValidationTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/http/Crc32ValidationTest.java index 8bf251df03fc..8cc89505b74f 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/http/Crc32ValidationTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/http/Crc32ValidationTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.core.http; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.io.InputStream; @@ -27,6 +28,7 @@ import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; import org.unitils.util.ReflectionUtils; +import software.amazon.awssdk.core.exception.Crc32MismatchException; import software.amazon.awssdk.core.internal.util.Crc32ChecksumValidatingInputStream; import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.http.SdkHttpFullResponse; @@ -109,18 +111,41 @@ public void adapt_InvalidGzipContent_ThrowsException() throws UnsupportedEncodin } @Test - public void adapt_ResponseWithCrc32Header_And_NoContent_DoesNotThrowNPE() throws UnsupportedEncodingException { + public void adapt_ResponseWithNonZeroCrc32Header_AndNoContent_ThrowsCrc32Mismatch() { SdkHttpFullResponse httpResponse = SdkHttpFullResponse.builder() .statusCode(200) .putHeader("x-amz-crc32", "1234") .build(); + assertThatThrownBy(() -> adapt(httpResponse)) + .isInstanceOf(Crc32MismatchException.class) + .hasMessageContaining("1234") + .hasMessageContaining("no content"); + } + + @Test + public void adapt_ResponseWithZeroCrc32Header_AndNoContent_PassesThrough() { + SdkHttpFullResponse httpResponse = SdkHttpFullResponse.builder() + .statusCode(200) + .putHeader("x-amz-crc32", "0") + .build(); + SdkHttpFullResponse adapted = adapt(httpResponse); assertThat(adapted.content().isPresent()).isFalse(); } @Test - public void adapt_ResponseGzipEncoding_And_NoContent_DoesNotThrowNPE() throws IOException { + public void adapt_ResponseWithoutCrc32Header_AndNoContent_PassesThrough() { + SdkHttpFullResponse httpResponse = SdkHttpFullResponse.builder() + .statusCode(200) + .build(); + + SdkHttpFullResponse adapted = adapt(httpResponse); + assertThat(adapted.content().isPresent()).isFalse(); + } + + @Test + public void adapt_ResponseGzipEncoding_AndNoContent_PassesThrough() throws IOException { SdkHttpFullResponse httpResponse = SdkHttpFullResponse.builder() .statusCode(200) .putHeader("Content-Encoding", "gzip") @@ -130,6 +155,18 @@ public void adapt_ResponseGzipEncoding_And_NoContent_DoesNotThrowNPE() throws IO assertThat(adapted.content().isPresent()).isFalse(); } + @Test + public void adapt_ResponseGzip_NonZeroCrc32_AndNoContent_ThrowsCrc32Mismatch() { + SdkHttpFullResponse httpResponse = SdkHttpFullResponse.builder() + .statusCode(200) + .putHeader("Content-Encoding", "gzip") + .putHeader("x-amz-crc32", "1234") + .build(); + + assertThatThrownBy(() -> adapt(httpResponse)) + .isInstanceOf(Crc32MismatchException.class); + } + private SdkHttpFullResponse adapt(SdkHttpFullResponse httpResponse) { return Crc32Validation.validate(false, httpResponse); } diff --git a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonAsyncCrc32ChecksumTests.java b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonAsyncCrc32ChecksumTests.java index 308daa04795d..227ae4e11b48 100644 --- a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonAsyncCrc32ChecksumTests.java +++ b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonAsyncCrc32ChecksumTests.java @@ -166,4 +166,15 @@ public void useGzipFalse_WhenCrc32IsInvalid_ThrowException() throws Exception { assertThatThrownBy(() -> jsonRpcAsync.allTypes(AllTypesRequest.builder().build()).get()) .hasRootCauseInstanceOf(Crc32MismatchException.class); } + + @Test + public void emptyBody_WhenCrc32HeaderIsNonZero_ThrowsCrc32Mismatch() { + stubFor(post(urlEqualTo("/")).willReturn(aResponse() + .withStatus(200) + .withHeader("x-amz-crc32", JSON_BODY_Crc32_CHECKSUM) + .withHeader("Content-Length", "0"))); + + assertThatThrownBy(() -> jsonRpcAsync.allTypes(AllTypesRequest.builder().build()).get()) + .hasRootCauseInstanceOf(Crc32MismatchException.class); + } } diff --git a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonCrc32ChecksumTests.java b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonCrc32ChecksumTests.java index 94700e129295..a0c4cedcc6ee 100644 --- a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonCrc32ChecksumTests.java +++ b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/AwsJsonCrc32ChecksumTests.java @@ -19,6 +19,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.post; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.github.tomakehurst.wiremock.common.SingleRootFileSource; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; @@ -29,7 +30,7 @@ import org.junit.Test; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.core.exception.Crc32MismatchException; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.protocoljsonrpc.ProtocolJsonRpcClient; import software.amazon.awssdk.services.protocoljsonrpc.model.AllTypesRequest; @@ -97,7 +98,7 @@ public void clientCalculatesCrc32FromCompressedData_ExtraData_WhenCrc32IsValid() Assert.assertEquals("foo", result.stringMember()); } - @Test(expected = SdkClientException.class) + @Test public void clientCalculatesCrc32FromCompressedData_WhenCrc32IsInvalid_ThrowsException() { stubFor(post(urlEqualTo("/")).willReturn(aResponse() .withStatus(200) @@ -111,7 +112,8 @@ public void clientCalculatesCrc32FromCompressedData_WhenCrc32IsInvalid_ThrowsExc .endpointOverride(URI.create("http://localhost:" + mockServer.port())) .build(); - jsonRpc.simple(SimpleRequest.builder().build()); + assertThatThrownBy(() -> jsonRpc.simple(SimpleRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); } @Test @@ -133,7 +135,7 @@ public void clientCalculatesCrc32FromDecompressedData_WhenCrc32IsValid() { Assert.assertEquals("foo", result.stringMember()); } - @Test(expected = SdkClientException.class) + @Test public void clientCalculatesCrc32FromDecompressedData_WhenCrc32IsInvalid_ThrowsException() { stubFor(post(urlEqualTo("/")).willReturn(aResponse() .withStatus(200) @@ -147,7 +149,8 @@ public void clientCalculatesCrc32FromDecompressedData_WhenCrc32IsInvalid_ThrowsE .endpointOverride(URI.create("http://localhost:" + mockServer.port())) .build(); - jsonRpc.allTypes(AllTypesRequest.builder().build()); + assertThatThrownBy(() -> jsonRpc.allTypes(AllTypesRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); } @Test @@ -168,7 +171,7 @@ public void useGzipFalse_WhenCrc32IsValid() { Assert.assertEquals("foo", result.stringMember()); } - @Test(expected = SdkClientException.class) + @Test public void useGzipFalse_WhenCrc32IsInvalid_ThrowException() { stubFor(post(urlEqualTo("/")).willReturn(aResponse() .withStatus(200) @@ -181,6 +184,24 @@ public void useGzipFalse_WhenCrc32IsInvalid_ThrowException() { .endpointOverride(URI.create("http://localhost:" + mockServer.port())) .build(); - jsonRpc.allTypes(AllTypesRequest.builder().build()); + assertThatThrownBy(() -> jsonRpc.allTypes(AllTypesRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); + } + + @Test + public void emptyBody_WhenCrc32HeaderIsNonZero_ThrowsCrc32Mismatch() { + stubFor(post(urlEqualTo("/")).willReturn(aResponse() + .withStatus(200) + .withHeader("x-amz-crc32", JSON_BODY_Crc32_CHECKSUM) + .withHeader("Content-Length", "0"))); + + ProtocolJsonRpcClient jsonRpc = ProtocolJsonRpcClient.builder() + .credentialsProvider(FAKE_CREDENTIALS_PROVIDER) + .region(Region.US_EAST_1) + .endpointOverride(URI.create("http://localhost:" + mockServer.port())) + .build(); + + assertThatThrownBy(() -> jsonRpc.allTypes(AllTypesRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); } } diff --git a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/RestJsonCrc32ChecksumTests.java b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/RestJsonCrc32ChecksumTests.java index cdf611bd8f8f..debb4a8ae866 100644 --- a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/RestJsonCrc32ChecksumTests.java +++ b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/crc32/RestJsonCrc32ChecksumTests.java @@ -19,6 +19,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.post; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.github.tomakehurst.wiremock.common.SingleRootFileSource; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; @@ -30,7 +31,7 @@ import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.core.exception.Crc32MismatchException; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient; import software.amazon.awssdk.services.protocolrestjson.model.AllTypesRequest; @@ -71,7 +72,7 @@ public void clientCalculatesCrc32FromCompressedData_WhenCrc32IsValid() { Assert.assertEquals("foo", result.stringMember()); } - @Test(expected = SdkClientException.class) + @Test public void clientCalculatesCrc32FromCompressedData_WhenCrc32IsInvalid_ThrowsException() { stubFor(post(urlEqualTo(RESOURCE_PATH)).willReturn(aResponse() .withStatus(200) @@ -84,7 +85,8 @@ public void clientCalculatesCrc32FromCompressedData_WhenCrc32IsInvalid_ThrowsExc .endpointOverride(URI.create("http://localhost:" + mockServer.port())) .build(); - client.simple(SimpleRequest.builder().build()); + assertThatThrownBy(() -> client.simple(SimpleRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); } @Test @@ -105,7 +107,7 @@ public void clientCalculatesCrc32FromDecompressedData_WhenCrc32IsValid() { Assert.assertEquals("foo", result.stringMember()); } - @Test(expected = SdkClientException.class) + @Test public void clientCalculatesCrc32FromDecompressedData_WhenCrc32IsInvalid_ThrowsException() { stubFor(post(urlEqualTo(RESOURCE_PATH)).willReturn(aResponse() .withStatus(200) @@ -118,7 +120,8 @@ public void clientCalculatesCrc32FromDecompressedData_WhenCrc32IsInvalid_ThrowsE .endpointOverride(URI.create("http://localhost:" + mockServer.port())) .build(); - client.allTypes(AllTypesRequest.builder().build()); + assertThatThrownBy(() -> client.allTypes(AllTypesRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); } @Test @@ -138,7 +141,7 @@ public void useGzipFalse_WhenCrc32IsValid() { Assert.assertEquals("foo", result.stringMember()); } - @Test(expected = SdkClientException.class) + @Test public void useGzipFalse_WhenCrc32IsInvalid_ThrowException() { stubFor(post(urlEqualTo(RESOURCE_PATH)).willReturn(aResponse() .withStatus(200) @@ -151,6 +154,24 @@ public void useGzipFalse_WhenCrc32IsInvalid_ThrowException() { .endpointOverride(URI.create("http://localhost:" + mockServer.port())) .build(); - client.allTypes(AllTypesRequest.builder().build()); + assertThatThrownBy(() -> client.allTypes(AllTypesRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); + } + + @Test + public void emptyBody_WhenCrc32HeaderIsNonZero_ThrowsCrc32Mismatch() { + stubFor(post(urlEqualTo(RESOURCE_PATH)).willReturn(aResponse() + .withStatus(200) + .withHeader("x-amz-crc32", JSON_BODY_Crc32_CHECKSUM) + .withHeader("Content-Length", "0"))); + + ProtocolRestJsonClient client = ProtocolRestJsonClient.builder() + .credentialsProvider(FAKE_CREDENTIALS_PROVIDER) + .region(Region.US_EAST_1) + .endpointOverride(URI.create("http://localhost:" + mockServer.port())) + .build(); + + assertThatThrownBy(() -> client.allTypes(AllTypesRequest.builder().build())) + .isInstanceOf(Crc32MismatchException.class); } }