Skip to content

Commit e9d5dc5

Browse files
Austin Brooksspfink
Austin Brooks
authored and
spfink
committed
Fix NullPointer of AbortableInputStream delegate if there is no body within UrlConnectionHttpClient
1 parent dad20c0 commit e9d5dc5

File tree

3 files changed

+74
-39
lines changed

3 files changed

+74
-39
lines changed

‎.changes/next-release/bugfix-URLConnectionHTTPClient-d70dbbf.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "URLConnection HTTP Client",
3+
"type": "bugfix",
4+
"description": "Fix NullPointer of AbortableInputStream delegate if there is no body within UrlConnectionHttpClient"
5+
}

‎http-clients/url-connection-client/src/main/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClient.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ public HttpExecuteResponse call() throws IOException {
146146
int responseCode = connection.getResponseCode();
147147
boolean isErrorResponse = HttpStatusFamily.of(responseCode).isOneOf(CLIENT_ERROR, SERVER_ERROR);
148148
InputStream content = !isErrorResponse ? connection.getInputStream() : connection.getErrorStream();
149+
AbortableInputStream responseBody = content != null ?
150+
AbortableInputStream.create(content) : null;
149151

150152
return HttpExecuteResponse.builder()
151153
.response(SdkHttpResponse.builder()
@@ -154,7 +156,7 @@ public HttpExecuteResponse call() throws IOException {
154156
// TODO: Don't ignore abort?
155157
.headers(extractHeaders(connection))
156158
.build())
157-
.responseBody(AbortableInputStream.create(content))
159+
.responseBody(responseBody)
158160
.build();
159161
}
160162

‎test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java

Lines changed: 66 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static com.github.tomakehurst.wiremock.client.WireMock.any;
2020
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
2121
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
22-
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
2322
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
2423
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
2524
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
@@ -29,7 +28,9 @@
2928
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3029

3130
import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
31+
import com.github.tomakehurst.wiremock.http.RequestMethod;
3232
import com.github.tomakehurst.wiremock.junit.WireMockRule;
33+
import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder;
3334
import java.io.ByteArrayInputStream;
3435
import java.io.IOException;
3536
import java.net.HttpURLConnection;
@@ -39,7 +40,6 @@
3940
import org.junit.Rule;
4041
import org.junit.Test;
4142
import org.junit.runner.RunWith;
42-
import org.mockito.Mock;
4343
import org.mockito.runners.MockitoJUnitRunner;
4444
import software.amazon.awssdk.utils.IoUtils;
4545

@@ -54,14 +54,17 @@ public abstract class SdkHttpClientTestSuite {
5454
@Rule
5555
public WireMockRule mockServer = new WireMockRule(wireMockConfig().dynamicPort().dynamicHttpsPort());
5656

57-
@Mock
58-
private SdkRequestContext requestContext;
59-
6057
@Test
6158
public void supportsResponseCode200() throws Exception {
6259
testForResponseCode(HttpURLConnection.HTTP_OK);
6360
}
6461

62+
@Test
63+
public void supportsResponseCode200HEAD() throws Exception {
64+
// HEAD is special due to closing of the connection immediately and streams are null
65+
testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD);
66+
}
67+
6568
@Test
6669
public void supportsResponseCode202() throws Exception {
6770
testForResponseCode(HttpURLConnection.HTTP_ACCEPTED);
@@ -72,6 +75,11 @@ public void supportsResponseCode403() throws Exception {
7275
testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN);
7376
}
7477

78+
@Test
79+
public void supportsResponseCode403HEAD() throws Exception {
80+
testForResponseCode(HttpURLConnection.HTTP_FORBIDDEN, SdkHttpMethod.HEAD);
81+
}
82+
7583
@Test
7684
public void supportsResponseCode301() throws Exception {
7785
testForResponseCode(HttpURLConnection.HTTP_MOVED_PERM);
@@ -91,42 +99,45 @@ public void supportsResponseCode500() throws Exception {
9199
public void validatesHttpsCertificateIssuer() throws Exception {
92100
SdkHttpClient client = createSdkHttpClient();
93101

94-
SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort());
102+
SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), SdkHttpMethod.POST);
95103

96104
assertThatThrownBy(client.prepareRequest(HttpExecuteRequest.builder().request(request).build())::call)
97105
.isInstanceOf(SSLHandshakeException.class);
98106
}
99107

100108
private void testForResponseCode(int returnCode) throws Exception {
109+
testForResponseCode(returnCode, SdkHttpMethod.POST);
110+
}
111+
112+
private void testForResponseCode(int returnCode, SdkHttpMethod method) throws Exception {
101113
SdkHttpClient client = createSdkHttpClient();
102114

103115
stubForMockRequest(returnCode);
104116

105-
SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port());
106-
HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder()
107-
.request(request)
108-
.contentStreamProvider(
109-
request.contentStreamProvider()
110-
.get())
111-
.build())
112-
.call();
117+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), method);
118+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
119+
.request(req)
120+
.contentStreamProvider(req.contentStreamProvider()
121+
.orElse(null))
122+
.build())
123+
.call();
113124

114-
validateResponse(response, returnCode);
125+
validateResponse(rsp, returnCode, method);
115126
}
116127

117128
protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCode) throws Exception {
129+
SdkHttpMethod sdkHttpMethod = SdkHttpMethod.POST;
118130
stubForMockRequest(returnCode);
119131

120-
SdkHttpFullRequest request = mockSdkRequest("https://localhost:" + mockServer.httpsPort());
121-
HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder()
122-
.request(request)
123-
.contentStreamProvider(
124-
request.contentStreamProvider()
125-
.get())
126-
.build())
127-
.call();
132+
SdkHttpFullRequest req = mockSdkRequest("https://localhost:" + mockServer.httpsPort(), sdkHttpMethod);
133+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
134+
.request(req)
135+
.contentStreamProvider(req.contentStreamProvider()
136+
.orElse(null))
137+
.build())
138+
.call();
128139

129-
validateResponse(response, returnCode);
140+
validateResponse(rsp, returnCode, sdkHttpMethod);
130141
}
131142

132143
private void stubForMockRequest(int returnCode) {
@@ -141,27 +152,44 @@ private void stubForMockRequest(int returnCode) {
141152
stubFor(any(urlPathEqualTo("/")).willReturn(responseBuilder));
142153
}
143154

144-
private void validateResponse(HttpExecuteResponse response, int returnCode) throws IOException {
145-
verify(1, postRequestedFor(urlMatching("/"))
146-
.withHeader("Host", containing("localhost"))
147-
.withHeader("User-Agent", equalTo("hello-world!"))
148-
.withRequestBody(equalTo("Body")));
155+
private void validateResponse(HttpExecuteResponse response, int returnCode, SdkHttpMethod method) throws IOException {
156+
RequestMethod requestMethod = RequestMethod.fromString(method.name());
157+
158+
RequestPatternBuilder patternBuilder = RequestPatternBuilder.newRequestPattern(requestMethod, urlMatching("/"))
159+
.withHeader("Host", containing("localhost"))
160+
.withHeader("User-Agent", equalTo("hello-world!"));
161+
162+
if (method == SdkHttpMethod.HEAD) {
163+
patternBuilder.withRequestBody(equalTo(""));
164+
} else {
165+
patternBuilder.withRequestBody(equalTo("Body"));
166+
}
167+
168+
verify(1, patternBuilder);
169+
170+
if (method == SdkHttpMethod.HEAD) {
171+
assertThat(response.responseBody()).isEmpty();
172+
} else {
173+
assertThat(IoUtils.toUtf8String(response.responseBody().orElse(null))).isEqualTo("hello");
174+
}
149175

150-
assertThat(IoUtils.toUtf8String(response.responseBody().orElse(null))).isEqualTo("hello");
151176
assertThat(response.httpResponse().firstMatchingHeader("Some-Header")).contains("With Value");
152177
assertThat(response.httpResponse().statusCode()).isEqualTo(returnCode);
153178
mockServer.resetMappings();
154179
}
155180

156-
private SdkHttpFullRequest mockSdkRequest(String uriString) {
181+
private SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) {
157182
URI uri = URI.create(uriString);
158-
return SdkHttpFullRequest.builder()
159-
.uri(uri)
160-
.method(SdkHttpMethod.POST)
161-
.putHeader("Host", uri.getHost())
162-
.putHeader("User-Agent", "hello-world!")
163-
.contentStreamProvider(() -> new ByteArrayInputStream("Body".getBytes(StandardCharsets.UTF_8)))
164-
.build();
183+
SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder()
184+
.uri(uri)
185+
.method(method)
186+
.putHeader("Host", uri.getHost())
187+
.putHeader("User-Agent", "hello-world!");
188+
if (method != SdkHttpMethod.HEAD) {
189+
requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream("Body".getBytes(StandardCharsets.UTF_8)));
190+
}
191+
192+
return requestBuilder.build();
165193
}
166194

167195
/**

0 commit comments

Comments
 (0)