Skip to content

Commit cfacbd8

Browse files
zoewanggzoewangg
authored
Set Accept header for UrlConnectionHttpClient (#3294)
1 parent 5b86ed3 commit cfacbd8

File tree

5 files changed

+70
-10
lines changed

5 files changed

+70
-10
lines changed

‎.changes/next-release/bugfix-URLConnectionClient-98f14e7.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "URL Connection Client",
4+
"contributor": "",
5+
"description": "Set the `Accept` header for `UrlConnectionHttpClient` because the default one does not comply with RFC 7231. See https://bugs.openjdk.org/browse/JDK-8163921"
6+
}

‎http-client-spi/src/main/java/software/amazon/awssdk/http/Header.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ public final class Header {
3939

4040
public static final String KEEP_ALIVE_VALUE = "keep-alive";
4141

42+
public static final String ACCEPT = "Accept";
43+
4244
private Header() {
4345
}
4446

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.http.urlconnection;
1717

18+
import static software.amazon.awssdk.http.Header.ACCEPT;
1819
import static software.amazon.awssdk.http.Header.CONTENT_LENGTH;
1920
import static software.amazon.awssdk.http.HttpStatusFamily.CLIENT_ERROR;
2021
import static software.amazon.awssdk.http.HttpStatusFamily.SERVER_ERROR;
@@ -60,6 +61,7 @@
6061
import software.amazon.awssdk.http.HttpStatusFamily;
6162
import software.amazon.awssdk.http.SdkHttpClient;
6263
import software.amazon.awssdk.http.SdkHttpConfigurationOption;
64+
import software.amazon.awssdk.http.SdkHttpRequest;
6365
import software.amazon.awssdk.http.SdkHttpResponse;
6466
import software.amazon.awssdk.http.TlsKeyManagersProvider;
6567
import software.amazon.awssdk.http.TlsTrustManagersProvider;
@@ -148,10 +150,17 @@ public String clientName() {
148150
}
149151

150152
private HttpURLConnection createAndConfigureConnection(HttpExecuteRequest request) {
151-
HttpURLConnection connection = connectionFactory.createConnection(request.httpRequest().getUri());
152-
request.httpRequest()
153-
.forEachHeader((key, values) -> values.forEach(value -> connection.setRequestProperty(key, value)));
154-
invokeSafely(() -> connection.setRequestMethod(request.httpRequest().method().name()));
153+
SdkHttpRequest sdkHttpRequest = request.httpRequest();
154+
HttpURLConnection connection = connectionFactory.createConnection(sdkHttpRequest.getUri());
155+
sdkHttpRequest.forEachHeader((key, values) -> values.forEach(value -> connection.setRequestProperty(key, value)));
156+
157+
if (!sdkHttpRequest.firstMatchingHeader(ACCEPT).isPresent()) {
158+
// Override Accept header because the default one in JDK does not comply with RFC 7231
159+
// See: https://bugs.openjdk.org/browse/JDK-8163921
160+
connection.setRequestProperty(ACCEPT, "*/*");
161+
}
162+
163+
invokeSafely(() -> connection.setRequestMethod(sdkHttpRequest.method().name()));
155164
if (request.contentStreamProvider().isPresent()) {
156165
connection.setDoOutput(true);
157166
}
@@ -160,8 +169,8 @@ private HttpURLConnection createAndConfigureConnection(HttpExecuteRequest reques
160169
// See: https://github.com/aws/aws-sdk-java-v2/issues/975
161170
connection.setInstanceFollowRedirects(false);
162171

163-
request.httpRequest().firstMatchingHeader(CONTENT_LENGTH).map(Long::parseLong)
164-
.ifPresent(connection::setFixedLengthStreamingMode);
172+
sdkHttpRequest.firstMatchingHeader(CONTENT_LENGTH).map(Long::parseLong)
173+
.ifPresent(connection::setFixedLengthStreamingMode);
165174

166175
return connection;
167176
}

‎http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWireMockTest.java

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@
1414
*/
1515
package software.amazon.awssdk.http.urlconnection;
1616

17-
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
18-
import static com.github.tomakehurst.wiremock.client.WireMock.any;
17+
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
18+
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
1919
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
20+
import static software.amazon.awssdk.http.Header.ACCEPT;
2021
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES;
2122

23+
import java.io.IOException;
2224
import javax.net.ssl.HttpsURLConnection;
2325
import javax.net.ssl.SSLSocketFactory;
2426
import org.junit.After;
2527
import org.junit.Test;
28+
import software.amazon.awssdk.http.HttpExecuteRequest;
29+
import software.amazon.awssdk.http.HttpExecuteResponse;
2630
import software.amazon.awssdk.http.SdkHttpClient;
2731
import software.amazon.awssdk.http.SdkHttpClientTestSuite;
32+
import software.amazon.awssdk.http.SdkHttpFullRequest;
33+
import software.amazon.awssdk.http.SdkHttpMethod;
2834
import software.amazon.awssdk.utils.AttributeMap;
2935

3036
public final class UrlConnectionHttpClientWireMockTest extends SdkHttpClientTestSuite {
@@ -50,6 +56,43 @@ public void connectionsAreNotReusedOn5xxErrors() {
5056
// We cannot support this because the URL connection client doesn't allow us to disable connection reuse
5157
}
5258

59+
// https://bugs.openjdk.org/browse/JDK-8163921
60+
@Test
61+
public void noAcceptHeader_shouldSet() throws IOException {
62+
SdkHttpClient client = createSdkHttpClient();
63+
64+
stubForMockRequest(200);
65+
66+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
67+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
68+
.request(req)
69+
.contentStreamProvider(req.contentStreamProvider()
70+
.orElse(null))
71+
.build())
72+
.call();
73+
74+
mockServer.verify(postRequestedFor(urlPathEqualTo("/")).withHeader(ACCEPT, equalTo("*/*")));
75+
}
76+
77+
@Test
78+
public void hasAcceptHeader_shouldNotOverride() throws IOException {
79+
SdkHttpClient client = createSdkHttpClient();
80+
81+
stubForMockRequest(200);
82+
83+
SdkHttpFullRequest req = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
84+
req = req.toBuilder().putHeader(ACCEPT, "text/html").build();
85+
HttpExecuteResponse rsp = client.prepareRequest(HttpExecuteRequest.builder()
86+
.request(req)
87+
.contentStreamProvider(req.contentStreamProvider()
88+
.orElse(null))
89+
.build())
90+
.call();
91+
92+
mockServer.verify(postRequestedFor(urlPathEqualTo("/")).withHeader(ACCEPT, equalTo("text/html")));
93+
}
94+
95+
5396
@After
5497
public void reset() {
5598
HttpsURLConnection.setDefaultSSLSocketFactory((SSLSocketFactory) SSLSocketFactory.getDefault());

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ protected void testForResponseCodeUsingHttps(SdkHttpClient client, int returnCod
239239
validateResponse(rsp, returnCode, sdkHttpMethod);
240240
}
241241

242-
private void stubForMockRequest(int returnCode) {
242+
protected void stubForMockRequest(int returnCode) {
243243
ResponseDefinitionBuilder responseBuilder = aResponse().withStatus(returnCode)
244244
.withHeader("Some-Header", "With Value")
245245
.withBody("hello");
@@ -277,7 +277,7 @@ private void validateResponse(HttpExecuteResponse response, int returnCode, SdkH
277277
mockServer.resetMappings();
278278
}
279279

280-
private SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) {
280+
protected SdkHttpFullRequest mockSdkRequest(String uriString, SdkHttpMethod method) {
281281
URI uri = URI.create(uriString);
282282
SdkHttpFullRequest.Builder requestBuilder = SdkHttpFullRequest.builder()
283283
.uri(uri)

0 commit comments

Comments
 (0)