Skip to content

Commit e2d16ad

Browse files
joviegasjoviegas
committed
Fix compilation issues
1 parent 1a18785 commit e2d16ad

File tree

2 files changed

+159
-21
lines changed

2 files changed

+159
-21
lines changed

‎http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ private AbortableInputStream getResponseBody(HttpResponse apacheHttpResponse,
318318
classicResponse.close();
319319
responseBody = AbortableInputStream.create(new ByteArrayInputStream(new byte[0]));
320320
} else {
321-
responseBody = createConnectionAwareStream(classicResponse, apacheRequest);
321+
responseBody = toAbortableInputStream(classicResponse, apacheRequest);
322322
}
323323
} else {
324324
// No entity, close the response immediately
@@ -328,23 +328,9 @@ private AbortableInputStream getResponseBody(HttpResponse apacheHttpResponse,
328328
return responseBody;
329329
}
330330

331-
private AbortableInputStream createConnectionAwareStream(ClassicHttpResponse apacheResponse,
332-
HttpUriRequestBase apacheRequest) throws IOException {
333-
InputStream entityStream = null;
334-
try {
335-
entityStream = apacheResponse.getEntity().getContent();
336-
return AbortableInputStream.create(
337-
new ConnectionAwareInputStream(entityStream, apacheResponse),
338-
() -> {
339-
apacheRequest.abort();
340-
IoUtils.closeQuietlyV2(apacheResponse, log);
341-
}
342-
);
343-
} catch (IOException e) {
344-
// Ensure response is closed on error
345-
IoUtils.closeQuietlyV2(apacheResponse, log);
346-
throw e;
347-
}
331+
private AbortableInputStream toAbortableInputStream(ClassicHttpResponse apacheResponse,
332+
HttpUriRequestBase apacheRequest) throws IOException {
333+
return AbortableInputStream.create(apacheResponse.getEntity().getContent(), apacheRequest::abort);
348334
}
349335

350336
private Apache5HttpRequestConfig createRequestConfig(DefaultBuilder builder,

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

Lines changed: 155 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@
2020
import static com.github.tomakehurst.wiremock.client.WireMock.any;
2121
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
2222
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
2324
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
2425
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
2526
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
2627
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
2728
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
2829
import static org.assertj.core.api.Assertions.assertThat;
2930
import static org.assertj.core.api.Assertions.assertThatThrownBy;
31+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
3032

3133
import com.github.tomakehurst.wiremock.WireMockServer;
3234
import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
@@ -236,6 +238,146 @@ public void doesNotRetryOn429StatusCode() throws Exception {
236238
}
237239
}
238240

241+
@Test
242+
public void handlesNoContentResponse() throws Exception {
243+
SdkHttpClient client = createSdkHttpClient();
244+
245+
mockServer.stubFor(any(urlPathEqualTo("/no-content"))
246+
.willReturn(aResponse()
247+
.withStatus(204)));
248+
249+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/no-content",
250+
SdkHttpMethod.DELETE);
251+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
252+
.request(req)
253+
.build())
254+
.call();
255+
256+
assertThat(rsp.httpResponse().statusCode()).isEqualTo(204);
257+
assertThat(rsp.responseBody()).isEmpty();
258+
}
259+
260+
@Test
261+
public void handlesLargeResponseBody() throws Exception {
262+
SdkHttpClient client = createSdkHttpClient();
263+
// Create a large response body (1MB)
264+
byte[] largeBody = new byte[1024 * 1024];
265+
for (int i = 0; i < largeBody.length; i++) {
266+
largeBody[i] = (byte) (i % 256);
267+
}
268+
mockServer.stubFor(any(urlPathEqualTo("/large"))
269+
.willReturn(aResponse()
270+
.withStatus(200)
271+
.withBody(largeBody)));
272+
273+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/large", SdkHttpMethod.GET);
274+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
275+
.request(req)
276+
.build())
277+
.call();
278+
279+
assertThat(rsp.httpResponse().statusCode()).isEqualTo(200);
280+
assertThat(rsp.responseBody()).isPresent();
281+
282+
// Read the entire response and verify
283+
byte[] readBuffer = IoUtils.toByteArray(rsp.responseBody().get());
284+
assertThat(readBuffer).isEqualTo(largeBody);
285+
}
286+
287+
@Test
288+
public void testAbortResponseStream() throws Exception {
289+
SdkHttpClient client = createSdkHttpClient();
290+
291+
mockServer.stubFor(any(urlPathEqualTo("/streaming"))
292+
.willReturn(aResponse()
293+
.withStatus(200)
294+
.withBody("This is a streaming response that should be aborted")));
295+
296+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/streaming", SdkHttpMethod.POST);
297+
ExecutableHttpRequest executableRequest = client.prepareRequest(HttpExecuteRequest.builder()
298+
.request(req)
299+
.contentStreamProvider(req.contentStreamProvider().orElse(null))
300+
.build());
301+
HttpExecuteResponse rsp = executableRequest.call();
302+
303+
assertThat(rsp.httpResponse().statusCode()).isEqualTo(200);
304+
assertThat(rsp.responseBody()).isPresent();
305+
306+
// Verify the stream is abortable
307+
AbortableInputStream stream = rsp.responseBody().get();
308+
assertThat(stream).isInstanceOf(AbortableInputStream.class);
309+
310+
// Read a few bytes
311+
byte[] buffer = new byte[10];
312+
int bytesRead = stream.read(buffer);
313+
assertThat(bytesRead).isGreaterThan(0);
314+
assertThatCode(() -> stream.abort()).doesNotThrowAnyException();
315+
stream.close();
316+
}
317+
318+
@Test
319+
public void handlesMultipleSequentialRequests() throws Exception {
320+
SdkHttpClient client = createSdkHttpClient();
321+
322+
mockServer.stubFor(any(urlPathEqualTo("/sequential"))
323+
.willReturn(aResponse()
324+
.withStatus(200)
325+
.withBody("Response body")));
326+
327+
// Execute multiple requests sequentially
328+
for (int i = 0; i < 5; i++) {
329+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + "/sequential", SdkHttpMethod.GET);
330+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
331+
.request(req)
332+
.build())
333+
.call();
334+
335+
assertThat(rsp.httpResponse().statusCode()).isEqualTo(200);
336+
assertThat(IoUtils.toUtf8String(rsp.responseBody().orElse(null))).isEqualTo("Response body");
337+
}
338+
mockServer.verify(5, getRequestedFor(urlEqualTo("/sequential")));
339+
}
340+
341+
@Test
342+
public void handlesVariousContentLengths() throws Exception {
343+
SdkHttpClient client = createSdkHttpClient();
344+
int[] contentLengths = {0, 1, 100, 1024, 65536};
345+
346+
for (int length : contentLengths) {
347+
String path = "/content-length-" + length;
348+
byte[] body = new byte[length];
349+
for (int i = 0; i < length; i++) {
350+
body[i] = (byte) ('A' + (i % 26));
351+
}
352+
353+
mockServer.stubFor(any(urlPathEqualTo(path))
354+
.willReturn(aResponse()
355+
.withStatus(200)
356+
.withHeader("Content-Length", String.valueOf(length))
357+
.withBody(body)));
358+
359+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port() + path, SdkHttpMethod.GET);
360+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
361+
.request(req)
362+
.build())
363+
.call();
364+
365+
assertThat(rsp.httpResponse().statusCode()).isEqualTo(200);
366+
367+
if (length == 0) {
368+
// Empty body should still have a response body present, but EOF immediately
369+
if (rsp.responseBody().isPresent()) {
370+
assertThat(rsp.responseBody().get().read()).isEqualTo(-1);
371+
}
372+
} else {
373+
assertThat(rsp.responseBody()).isPresent();
374+
byte[] readBody = IoUtils.toByteArray(rsp.responseBody().get());
375+
assertThat(readBody).isEqualTo(body);
376+
}
377+
}
378+
}
379+
380+
239381
private void validateStatusCodeWithRetryCheck(SdkHttpClient client,
240382
int expectedStatusCode,
241383
int expectedRequestCount) throws IOException {
@@ -359,13 +501,23 @@ private static SdkHttpFullRequest.Builder mockSdkRequestBuilder(String uriString
359501
return requestBuilder;
360502
}
361503

362-
363504
protected SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) {
364-
SdkHttpFullRequest.Builder requestBuilder = mockSdkRequestBuilder(uriString, method);
365-
if (method != SdkHttpMethod.HEAD) {
505+
URI uri = URI.create(uriString);
506+
SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder()
507+
.uri(uri)
508+
.method(method)
509+
.putHeader("Host", uri.getHost())
510+
.putHeader("User-Agent", "hello-world!");
511+
512+
// Only add body for methods that typically have a body
513+
if (method != SdkHttpMethod.HEAD && method != SdkHttpMethod.GET && method != SdkHttpMethod.DELETE) {
366514
byte[] content = "Body".getBytes(StandardCharsets.UTF_8);
367515
requestBuilder.putHeader("Content-Length", Integer.toString(content.length));
368516
requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(content));
517+
} else if (method == SdkHttpMethod.GET || method == SdkHttpMethod.DELETE) {
518+
// For GET and DELETE, explicitly set Content-Length to 0 or don't set it at all
519+
// Some clients like AWS CRT are strict about this
520+
requestBuilder.contentStreamProvider(() -> new ByteArrayInputStream(new byte[0]));
369521
}
370522

371523
return requestBuilder.build();

0 commit comments

Comments
 (0)