diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-a3c92c2.json b/.changes/next-release/bugfix-AWSSDKforJavav2-a3c92c2.json new file mode 100644 index 000000000000..554e42602d64 --- /dev/null +++ b/.changes/next-release/bugfix-AWSSDKforJavav2-a3c92c2.json @@ -0,0 +1,6 @@ +{ + "category": "AWS SDK for Java v2", + "contributor": "Bennett-Lynch", + "type": "bugfix", + "description": "[UrlConnectionHttpClient] Wrap erroneous NPE from HttpURLConnection as IOException" +} diff --git a/http-clients/url-connection-client/src/main/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClient.java b/http-clients/url-connection-client/src/main/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClient.java index 37840ce1c4ce..b14f00e4e69b 100644 --- a/http-clients/url-connection-client/src/main/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClient.java +++ b/http-clients/url-connection-client/src/main/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClient.java @@ -207,7 +207,7 @@ public HttpExecuteResponse call() throws IOException { request.contentStreamProvider().ifPresent(provider -> invokeSafely(() -> IoUtils.copy(provider.newStream(), connection.getOutputStream()))); - int responseCode = connection.getResponseCode(); + int responseCode = getResponseCodeSafely(connection); boolean isErrorResponse = HttpStatusFamily.of(responseCode).isOneOf(CLIENT_ERROR, SERVER_ERROR); InputStream content = !isErrorResponse ? connection.getInputStream() : connection.getErrorStream(); AbortableInputStream responseBody = content != null ? @@ -224,6 +224,24 @@ public HttpExecuteResponse call() throws IOException { .build(); } + /** + * {@link sun.net.www.protocol.http.HttpURLConnection#getInputStream0()} has been observed to intermittently throw + * {@link NullPointerException}s for reasons that still require further investigation, but are assumed to be due to a + * bug in the JDK. Propagating such NPEs is confusing for users and are not subject to being retried on by the default + * retry policy configuration, so instead we bias towards propagating these as {@link IOException}s. + *

+ * TODO: Determine precise root cause of intermittent NPEs, submit JDK bug report if applicable, and consider applying + * this behavior only on unpatched JVM runtime versions. + */ + private static int getResponseCodeSafely(HttpURLConnection connection) throws IOException { + Validate.paramNotNull(connection, "connection"); + try { + return connection.getResponseCode(); + } catch (NullPointerException e) { + throw new IOException("Unexpected NullPointerException when trying to read response from HttpURLConnection", e); + } + } + private Map> extractHeaders(HttpURLConnection response) { return response.getHeaderFields().entrySet().stream() .filter(e -> e.getKey() != null) diff --git a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java index 6ce76ee84c70..e64392dee52f 100644 --- a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java +++ b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java @@ -14,17 +14,30 @@ */ package software.amazon.awssdk.http.urlconnection; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.spy; import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; +import static software.amazon.awssdk.utils.FunctionalUtils.safeFunction; +import java.io.IOException; import java.net.HttpURLConnection; +import java.util.function.Function; import org.junit.Ignore; +import org.junit.Test; import software.amazon.awssdk.http.SdkHttpClient; import software.amazon.awssdk.http.SdkHttpClientTestSuite; public final class UrlConnectionHttpClientWithCustomCreateWireMockTest extends SdkHttpClientTestSuite { + + private Function connectionInterceptor = Function.identity(); + @Override protected SdkHttpClient createSdkHttpClient(SdkHttpClientOptions options) { - return UrlConnectionHttpClient.create((uri) -> invokeSafely(() -> (HttpURLConnection) uri.toURL().openConnection())); + return UrlConnectionHttpClient.create(uri -> invokeSafely(() -> { + HttpURLConnection connection = (HttpURLConnection) uri.toURL().openConnection(); + return connectionInterceptor.apply(connection); + })); } @Ignore // Not supported when using custom factory @@ -41,4 +54,18 @@ public void testTrustAllWorks() { @Override public void testCustomTlsTrustManagerAndTrustAllFails() { } + + @Test + public void testGetResponseCodeNpeIsWrappedAsIo() throws Exception { + connectionInterceptor = safeFunction(connection -> { + connection = spy(connection); + doThrow(new NullPointerException()).when(connection).getResponseCode(); + return connection; + }); + + assertThatThrownBy(() -> testForResponseCode(HttpURLConnection.HTTP_OK)) + .isInstanceOf(IOException.class) + .hasMessage("Unexpected NullPointerException when trying to read response from HttpURLConnection") + .hasCauseInstanceOf(NullPointerException.class); + } } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index a9f1feacec23..074fd75bec12 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -152,7 +152,7 @@ public void testCustomTlsTrustManagerAndTrustAllFails() throws Exception { assertThatThrownBy(() -> createSdkHttpClient(httpClientOptions)).isInstanceOf(IllegalArgumentException.class); } - private void testForResponseCode(int returnCode) throws Exception { + protected void testForResponseCode(int returnCode) throws Exception { testForResponseCode(returnCode, SdkHttpMethod.POST); }