Skip to content

Commit 323b216

Browse files
committed
Fixed an issue in Apache HTTP client where a request with path parameter as a single slash threw invalid host name error.
1 parent d23f9e7 commit 323b216

File tree

4 files changed

+82
-12
lines changed

4 files changed

+82
-12
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "Apache HTTP Client",
3+
"type": "bugfix",
4+
"description": "Fixed an issue in Apache HTTP client where a request with path parameter as a single slash threw invalid host name error."
5+
}

http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactory.java

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,10 @@
4747
@SdkInternalApi
4848
public class ApacheHttpRequestFactory {
4949

50-
private static final String DEFAULT_ENCODING = "UTF-8";
51-
5250
private static final List<String> IGNORE_HEADERS = Arrays.asList(HttpHeaders.CONTENT_LENGTH, HttpHeaders.HOST);
5351

5452
public HttpRequestBase create(final HttpExecuteRequest request, final ApacheHttpRequestConfig requestConfig) {
55-
URI uri = request.httpRequest().getUri();
56-
57-
HttpRequestBase base = createApacheRequest(request, sanitizeUri(uri));
53+
HttpRequestBase base = createApacheRequest(request, sanitizeUri(request.httpRequest()));
5854
addHeadersToRequest(base, request.httpRequest());
5955
addRequestConfig(base, request.httpRequest(), requestConfig);
6056

@@ -66,12 +62,28 @@ public HttpRequestBase create(final HttpExecuteRequest request, final ApacheHttp
6662
* and other AWS services, this is allowed and required. This methods replaces
6763
* any occurrence of "//" in the URI path with "/%2F".
6864
*
69-
* @param uri The existing URI with double slashes not sanitized for Apache.
65+
* @see SdkHttpRequest#getUri()
66+
* @param request The existing request
7067
* @return a new String containing the modified URI
7168
*/
72-
private String sanitizeUri(URI uri) {
73-
String newPath = uri.getPath().replace("//", "/%2F");
74-
return uri.toString().replace(uri.getPath(), newPath);
69+
private String sanitizeUri(SdkHttpRequest request) {
70+
String path = request.encodedPath();
71+
if (path.contains("//")) {
72+
int port = request.port();
73+
String protocol = request.protocol();
74+
String newPath = path.replace("//", "/%2F");
75+
String encodedQueryString = SdkHttpUtils.encodeAndFlattenQueryParameters(request.rawQueryParameters())
76+
.map(value -> "?" + value)
77+
.orElse("");
78+
79+
// Do not include the port in the URI when using the default port for the protocol.
80+
String portString = SdkHttpUtils.isUsingStandardPort(protocol, port) ?
81+
"" : ":" + port;
82+
83+
return URI.create(protocol + "://" + request.host() + portString + newPath + encodedQueryString).toString();
84+
}
85+
86+
return request.getUri().toString();
7587
}
7688

7789
private void addRequestConfig(final HttpRequestBase base,

http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/internal/impl/ApacheHttpRequestFactoryTest.java

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

1616
package software.amazon.awssdk.http.apache.internal.impl;
1717

18+
import static org.assertj.core.api.Assertions.assertThat;
1819
import static org.junit.Assert.assertEquals;
1920
import static org.junit.Assert.assertNotNull;
2021

@@ -91,4 +92,37 @@ public void defaultHttpPortsAreNotInDefaultHostHeader() {
9192
assertEquals(1, hostHeaders.length);
9293
assertEquals("localhost", hostHeaders[0].getValue());
9394
}
95+
96+
@Test
97+
public void pathWithLeadingSlash_shouldEncode() {
98+
assertThat(sanitizedUri("/foobar")).isEqualTo("http://localhost/%2Ffoobar");
99+
}
100+
101+
@Test
102+
public void pathWithOnlySlash_shouldEncode() {
103+
assertThat(sanitizedUri("/")).isEqualTo("http://localhost/%2F");
104+
}
105+
106+
@Test
107+
public void pathWithoutSlash_shouldReturnSameUri() {
108+
assertThat(sanitizedUri("path")).isEqualTo("http://localhost/path");
109+
}
110+
111+
@Test
112+
public void pathWithSpecialChars_shouldPreserveEncoding() {
113+
assertThat(sanitizedUri("/special-chars-%40%24%25")).isEqualTo("http://localhost/%2Fspecial-chars-%40%24%25");
114+
}
115+
116+
private String sanitizedUri(String path) {
117+
SdkHttpRequest sdkRequest = SdkHttpRequest.builder()
118+
.uri(URI.create("http://localhost:80"))
119+
.encodedPath("/" + path)
120+
.method(SdkHttpMethod.HEAD)
121+
.build();
122+
HttpExecuteRequest request = HttpExecuteRequest.builder()
123+
.request(sdkRequest)
124+
.build();
125+
126+
return instance.create(request, requestConfig).getURI().toString();
127+
}
94128
}

services/s3/src/it/java/software/amazon/awssdk/services/s3/KeysWithLeadingSlashIntegrationTest.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
public class KeysWithLeadingSlashIntegrationTest extends S3IntegrationTestBase {
2727

2828
private static final String BUCKET = temporaryBucketName(KeysWithLeadingSlashIntegrationTest.class);
29-
private static final String KEY = "/stupidkeywithillegalleadingslashthatsucks";
29+
private static final String KEY = "/keyWithLeadingSlash";
30+
private static final String SLASH_KEY = "/";
31+
private static final String KEY_WITH_SLASH_AND_SPECIAL_CHARS = "/special-chars-@$%";
3032
private static final byte[] CONTENT = "Hello".getBytes(StandardCharsets.UTF_8);
3133

3234
@BeforeClass
@@ -42,9 +44,26 @@ public static void cleanup() {
4244

4345
@Test
4446
public void putObject_KeyWithLeadingSlash_Succeeds() {
45-
s3.putObject(r -> r.bucket(BUCKET).key(KEY), RequestBody.fromBytes(CONTENT));
47+
verify(KEY);
48+
}
49+
50+
@Test
51+
public void slashKey_shouldSucceed() {
52+
verify(SLASH_KEY);
53+
}
54+
55+
@Test
56+
public void slashKeyWithSpecialChar_shouldSucceed() {
57+
verify(KEY_WITH_SLASH_AND_SPECIAL_CHARS);
58+
}
59+
60+
private void verify(String key) {
61+
s3.putObject(r -> r.bucket(BUCKET).key(key), RequestBody.fromBytes(CONTENT));
62+
63+
assertThat(s3.getObjectAsBytes(r -> r.bucket(BUCKET).key(key)).asByteArray()).isEqualTo(CONTENT);
64+
4665
String retrievedKey = s3.listObjects(r -> r.bucket(BUCKET)).contents().get(0).key();
4766

48-
assertThat(retrievedKey).isEqualTo(KEY);
67+
assertThat(retrievedKey).isEqualTo(key);
4968
}
5069
}

0 commit comments

Comments
 (0)