Skip to content

Commit fbfb70f

Browse files
joviegasjoviegas
authored
Fix architecture test failures for apache5.x (#6140)
* Fix architecture test failures for apache5.x * Checkstyle issues
1 parent 62150d6 commit fbfb70f

File tree

6 files changed

+230
-57
lines changed

6 files changed

+230
-57
lines changed

‎http-clients/apache-client/src/test/java/software/amazon/awssdk/http/apache/ApacheHttpClientUriNormalizationTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http.apache;
17+
18+
import software.amazon.awssdk.http.HttpClientUriNormalizationTestSuite;
19+
import software.amazon.awssdk.http.SdkHttpClient;
20+
21+
public class ApacheHttpClientUriNormalizationTest extends HttpClientUriNormalizationTestSuite {
22+
23+
24+
@Override
25+
protected SdkHttpClient createSdkHttpClient() {
26+
return ApacheHttpClient.create();
27+
}
28+
}
29+

‎http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/impl/Apache5HttpRequestFactory.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,6 @@ private void addRequestConfig(HttpUriRequestBase base,
100100
.setResponseTimeout(saturatedCast(requestConfig.socketTimeout().toMillis()), TimeUnit.MILLISECONDS);
101101
// TODO as part of removed API : .setLocalAddress(requestConfig.localAddress());
102102

103-
Apache5Utils.disableNormalizeUri(requestConfigBuilder);
104-
105103
/*
106104
* Enable 100-continue support for PUT operations, since this is
107105
* where we're potentially uploading large amounts of data and want

‎http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/internal/utils/Apache5Utils.java

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -29,29 +29,9 @@
2929
import org.apache.hc.core5.http.io.entity.BufferedHttpEntity;
3030
import software.amazon.awssdk.annotations.SdkInternalApi;
3131
import software.amazon.awssdk.http.apache5.ProxyConfiguration;
32-
import software.amazon.awssdk.utils.Logger;
33-
import software.amazon.awssdk.utils.ReflectionMethodInvoker;
3432

3533
@SdkInternalApi
3634
public final class Apache5Utils {
37-
private static final Logger logger = Logger.loggerFor(Apache5Utils.class);
38-
private static final ReflectionMethodInvoker<RequestConfig.Builder, RequestConfig.Builder> NORMALIZE_URI_INVOKER;
39-
40-
static {
41-
// Attempt to initialize the invoker once on class-load. If it fails, it will not be attempted again, but we'll
42-
// use that opportunity to log a warning.
43-
NORMALIZE_URI_INVOKER =
44-
new ReflectionMethodInvoker<>(RequestConfig.Builder.class,
45-
RequestConfig.Builder.class,
46-
"setNormalizeUri",
47-
boolean.class);
48-
49-
try {
50-
NORMALIZE_URI_INVOKER.initialize();
51-
} catch (NoSuchMethodException ignored) {
52-
noSuchMethodThrownByNormalizeUriInvoker();
53-
}
54-
}
5535

5636
private Apache5Utils() {
5737
}
@@ -79,37 +59,11 @@ public static HttpClientContext newClientContext(ProxyConfiguration proxyConfigu
7959
addPreemptiveAuthenticationProxy(clientContext, proxyConfiguration);
8060

8161
RequestConfig.Builder builder = RequestConfig.custom();
82-
disableNormalizeUri(builder);
83-
8462
clientContext.setRequestConfig(builder.build());
8563
return clientContext;
8664

8765
}
8866

89-
/**
90-
* From Apache v4.5.8, normalization should be disabled or AWS requests with special characters in URI path will fail
91-
* with Signature Errors.
92-
* <p>
93-
* setNormalizeUri is added only in 4.5.8, so customers using the latest version of SDK with old versions (4.5.6 or less)
94-
* of Apache httpclient will see NoSuchMethodError. Hence this method will suppress the error.
95-
*
96-
* Do not use Apache version 4.5.7 as it breaks URI paths with special characters and there is no option
97-
* to disable normalization.
98-
* </p>
99-
*
100-
* For more information, See https://github.com/aws/aws-sdk-java/issues/1919
101-
*/
102-
public static void disableNormalizeUri(RequestConfig.Builder requestConfigBuilder) {
103-
// For efficiency, do not attempt to call the invoker again if it failed to initialize on class-load
104-
if (NORMALIZE_URI_INVOKER.isInitialized()) {
105-
try {
106-
NORMALIZE_URI_INVOKER.invoke(requestConfigBuilder, false);
107-
} catch (NoSuchMethodException ignored) {
108-
noSuchMethodThrownByNormalizeUriInvoker();
109-
}
110-
}
111-
}
112-
11367
/**
11468
* Returns a new Credentials Provider for use with proxy authentication.
11569
*/
@@ -154,13 +108,4 @@ private static void addPreemptiveAuthenticationProxy(HttpClientContext clientCon
154108
}
155109
}
156110

157-
// Just log and then swallow the exception
158-
private static void noSuchMethodThrownByNormalizeUriInvoker() {
159-
// setNormalizeUri method was added in httpclient 4.5.8
160-
logger.warn(() -> "NoSuchMethodException was thrown when disabling normalizeUri. This indicates you are using "
161-
+ "an old version (< 4.5.8) of Apache http client. It is recommended to use http client "
162-
+ "version >= 4.5.9 to avoid the breaking change introduced in apache client 4.5.7 and "
163-
+ "the latency in exception handling. See https://github.com/aws/aws-sdk-java/issues/1919"
164-
+ " for more information");
165-
}
166111
}

‎http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientUriNormalizationTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http.apache5;
17+
18+
import software.amazon.awssdk.http.HttpClientUriNormalizationTestSuite;
19+
import software.amazon.awssdk.http.SdkHttpClient;
20+
21+
public class Apache5HttpClientUriNormalizationTest extends HttpClientUriNormalizationTestSuite {
22+
23+
24+
@Override
25+
protected SdkHttpClient createSdkHttpClient() {
26+
return Apache5HttpClient.create();
27+
}
28+
}
29+

‎test/architecture-tests/archunit_store/4195d6e3-8849-4e5a-848d-04f810577cd3

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@ Method <software.amazon.awssdk.core.retry.ClockSkew.getServerTime(software.amazo
1111
Method <software.amazon.awssdk.http.FileStoreTlsKeyManagersProvider.keyManagers()> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (FileStoreTlsKeyManagersProvider.java:52)
1212
Method <software.amazon.awssdk.http.SystemPropertyTlsKeyManagersProvider.keyManagers()> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (SystemPropertyTlsKeyManagersProvider.java:61)
1313
Method <software.amazon.awssdk.http.apache.ApacheHttpClient$ApacheConnectionManagerFactory.getSslContext(software.amazon.awssdk.utils.AttributeMap)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (ApacheHttpClient.java:699)
14+
Method <software.amazon.awssdk.http.apache5.Apache5HttpClient$ApacheConnectionManagerFactory.getSslContext(software.amazon.awssdk.utils.AttributeMap)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (Apache5HttpClient.java:741)
1415
Method <software.amazon.awssdk.http.apache.internal.RepeatableInputStreamRequestEntity.parseContentLength(java.lang.String)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (RepeatableInputStreamRequestEntity.java:113)
1516
Method <software.amazon.awssdk.http.apache.internal.utils.ApacheUtils.noSuchMethodThrownByNormalizeUriInvoker()> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (ApacheUtils.java:162)
17+
Method <software.amazon.awssdk.http.apache5.internal.RepeatableInputStreamRequestEntity.parseContentLength(java.lang.String)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (RepeatableInputStreamRequestEntity.java:131)
18+
Method <software.amazon.awssdk.http.apache5.internal.RepeatableInputStreamRequestEntity.parseContentType(java.lang.String)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (RepeatableInputStreamRequestEntity.java:143)
1619
Method <software.amazon.awssdk.http.nio.netty.internal.utils.NettyUtils.warnIfNotInEventLoop(io.netty.channel.EventLoop)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (NettyUtils.java:289)
1720
Method <software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient.getSslContext(software.amazon.awssdk.utils.AttributeMap)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier)> in (UrlConnectionHttpClient.java:263)
1821
Method <software.amazon.awssdk.metrics.publishers.cloudwatch.CloudWatchMetricPublisher.publish(software.amazon.awssdk.metrics.MetricCollection)> calls method <software.amazon.awssdk.utils.Logger.warn(java.util.function.Supplier, java.lang.Throwable)> in (CloudWatchMetricPublisher.java:293)

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

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.http;
17+
18+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
19+
import static com.github.tomakehurst.wiremock.client.WireMock.any;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.anyRequestedFor;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
23+
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
24+
import static org.assertj.core.api.Assertions.assertThat;
25+
26+
import com.github.tomakehurst.wiremock.WireMockServer;
27+
import com.github.tomakehurst.wiremock.client.WireMock;
28+
import com.github.tomakehurst.wiremock.verification.LoggedRequest;
29+
import java.net.URI;
30+
import java.util.List;
31+
import java.util.stream.Stream;
32+
import org.junit.jupiter.api.AfterAll;
33+
import org.junit.jupiter.api.AfterEach;
34+
import org.junit.jupiter.api.BeforeAll;
35+
import org.junit.jupiter.api.BeforeEach;
36+
import org.junit.jupiter.api.DisplayName;
37+
import org.junit.jupiter.params.ParameterizedTest;
38+
import org.junit.jupiter.params.provider.Arguments;
39+
import org.junit.jupiter.params.provider.MethodSource;
40+
41+
public abstract class HttpClientUriNormalizationTestSuite {
42+
43+
protected static SdkHttpClient httpClient;
44+
private static WireMockServer wireMockServer;
45+
46+
@BeforeAll
47+
static void setUp() {
48+
wireMockServer = new WireMockServer(options().dynamicPort());
49+
wireMockServer.start();
50+
WireMock.configureFor("localhost", wireMockServer.port());
51+
}
52+
53+
@BeforeEach
54+
void prepare() {
55+
wireMockServer.stubFor(any(urlMatching(".*"))
56+
.willReturn(aResponse()
57+
.withStatus(200)
58+
.withBody("success")));
59+
}
60+
61+
@AfterEach
62+
void reset() {
63+
wireMockServer.resetAll();
64+
}
65+
66+
@AfterAll
67+
static void tearDown() {
68+
if (httpClient != null) {
69+
httpClient.close();
70+
}
71+
if (wireMockServer != null) {
72+
wireMockServer.stop();
73+
}
74+
}
75+
76+
private static Stream<Arguments> uriTestCases() {
77+
return Stream.of(
78+
Arguments.of(
79+
"Encoded spaces",
80+
"/path/with%20spaces/file.txt",
81+
"%20"
82+
),
83+
Arguments.of(
84+
"Encoded slashes",
85+
"/path/with%2Fslash/file.txt",
86+
"%2F"
87+
),
88+
Arguments.of(
89+
"Encoded plus",
90+
"/path/with%2Bplus/file.txt",
91+
"%2B"
92+
),
93+
Arguments.of(
94+
"Plus sign",
95+
"/path/with+plus/file.txt",
96+
"+"
97+
),
98+
Arguments.of(
99+
"Encoded question mark",
100+
"/path/with%3Fquery/file.txt",
101+
"%3F"
102+
),
103+
Arguments.of(
104+
"Encoded ampersand",
105+
"/path/with%26ampersand/file.txt",
106+
"%26"
107+
),
108+
Arguments.of(
109+
"Encoded equals",
110+
"/path/with%3Dequals/file.txt",
111+
"%3D"
112+
),
113+
Arguments.of(
114+
"AWS S3 style path",
115+
"/my-bucket/folder%2Fsubfolder/file%20name.txt",
116+
"%2F"
117+
)
118+
);
119+
}
120+
121+
@ParameterizedTest
122+
@MethodSource("uriTestCases")
123+
@DisplayName("Verify URI normalization is disabled (encoded characters are preserved)")
124+
void testUriNormalizationDisabled(String testName, String path, String encodedChar) throws Exception {
125+
httpClient = createSdkHttpClient();
126+
127+
// Create and execute request
128+
HttpExecuteRequest request = createTestRequest(path);
129+
ExecutableHttpRequest executableRequest = httpClient.prepareRequest(request);
130+
HttpExecuteResponse response = executableRequest.call();
131+
132+
// Verify response was successful
133+
assertThat(response.httpResponse().statusCode()).isEqualTo(200);
134+
135+
// Capture the actual request sent to server
136+
List<LoggedRequest> requests = wireMockServer.findAll(anyRequestedFor(anyUrl()));
137+
assertThat(requests).hasSize(1);
138+
139+
String actualPathSent = requests.get(0).getUrl();
140+
assertThat(actualPathSent).contains(encodedChar);
141+
}
142+
143+
private HttpExecuteRequest createTestRequest(String path) {
144+
String baseUrl = "http://localhost:" + wireMockServer.port();
145+
return HttpExecuteRequest.builder()
146+
.request(SdkHttpRequest.builder()
147+
.method(SdkHttpMethod.GET)
148+
.uri(URI.create(baseUrl + path))
149+
.build())
150+
.build();
151+
}
152+
153+
@ParameterizedTest
154+
@MethodSource("uriTestCases")
155+
@DisplayName("Test end-to-end execution flow with client context")
156+
void testExecuteFlowWithClientContext(String testName, String path, String encodedChar) throws Exception {
157+
httpClient = createSdkHttpClient();
158+
HttpExecuteRequest request = createTestRequest(path);
159+
HttpExecuteResponse response = httpClient.prepareRequest(request).call();
160+
assertThat(response.httpResponse().statusCode()).isEqualTo(200);
161+
List<LoggedRequest> requests = wireMockServer.findAll(anyRequestedFor(anyUrl()));
162+
assertThat(requests).hasSize(1);
163+
164+
String actualUrl = requests.get(0).getUrl();
165+
assertThat(actualUrl).contains(encodedChar);
166+
}
167+
168+
protected abstract SdkHttpClient createSdkHttpClient();
169+
}

0 commit comments

Comments
 (0)