Skip to content

Commit 4a84eaa

Browse files
davidh44davidh44
authored
Skip ALPN if request is HTTP endpoint (#5854)
* Skip ALPN if request is HTTP endpoint * Skip ALPN if request is HTTP endpoint * Update boolean naming * Address comments * Minor updates * Update test message assertion
1 parent 672a315 commit 4a84eaa

File tree

6 files changed

+153
-8
lines changed

6 files changed

+153
-8
lines changed

‎.changes/next-release/feature-NettyNIOHTTPClient-8f8589d.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "feature",
3+
"category": "Netty NIO HTTP Client",
4+
"contributor": "",
5+
"description": "Fallback to prior knowledge if default client setting is ALPN and request has HTTP endpoint"
6+
}

‎http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public final class NettyNioAsyncHttpClient implements SdkAsyncHttpClient {
8686
private final SdkChannelPoolMap<URI, ? extends SdkChannelPool> pools;
8787
private final NettyConfiguration configuration;
8888
private final ProtocolNegotiation protocolNegotiation;
89+
private boolean isAlpnUserConfigured;
8990

9091
private NettyNioAsyncHttpClient(DefaultBuilder builder, AttributeMap serviceDefaultsMap) {
9192
this.configuration = new NettyConfiguration(serviceDefaultsMap);
@@ -135,9 +136,9 @@ public CompletableFuture<Void> execute(AsyncExecuteRequest request) {
135136
}
136137

137138
private void failIfAlpnUsedWithHttp(AsyncExecuteRequest request) {
138-
if (protocolNegotiation == ProtocolNegotiation.ALPN
139-
&& "http".equals(request.request().protocol())) {
140-
throw new UnsupportedOperationException("ALPN can only be used with HTTPS, not HTTP.");
139+
if (isAlpnUserConfigured && "http".equals(request.request().protocol())) {
140+
throw new UnsupportedOperationException("ALPN can only be used with HTTPS, not HTTP. "
141+
+ "Use ProtocolNegotiation.ASSUME_PROTOCOL instead.");
141142
}
142143
}
143144

@@ -183,6 +184,7 @@ private SslProvider resolveSslProvider(DefaultBuilder builder) {
183184
private ProtocolNegotiation resolveProtocolNegotiation(ProtocolNegotiation userSetValue, AttributeMap serviceDefaultsMap,
184185
Protocol protocol, SslProvider sslProvider) {
185186
if (userSetValue == ProtocolNegotiation.ALPN) {
187+
this.isAlpnUserConfigured = true;
186188
// TODO - remove once we implement support for ALPN with HTTP1
187189
if (protocol == Protocol.HTTP1_1) {
188190
throw new UnsupportedOperationException("ALPN with HTTP/1.1 is not yet supported, use prior knowledge instead "

‎http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,9 @@ public void channelCreated(Channel ch) {
9999
ch.attr(CHANNEL_DIAGNOSTICS).set(new ChannelDiagnostics(ch));
100100
ch.attr(PROTOCOL_FUTURE).set(new CompletableFuture<>());
101101
ChannelPipeline pipeline = ch.pipeline();
102-
if (sslCtx != null) {
102+
103+
boolean sslCtxPresent = sslCtx != null;
104+
if (sslCtxPresent) {
103105

104106
SslHandler sslHandler = newSslHandler(sslCtx, ch.alloc(), poolKey.getHost(), poolKey.getPort(),
105107
configuration.tlsHandshakeTimeout());
@@ -114,11 +116,16 @@ public void channelCreated(Channel ch) {
114116
}
115117
}
116118

117-
configureProtocolHandlers(ch, pipeline, protocol);
119+
configureProtocolHandlers(ch, pipeline, protocol, sslCtxPresent);
118120
configurePostProtocolHandlers(pipeline, protocol);
119121
}
120122

121-
private void configureProtocolHandlers(Channel ch, ChannelPipeline pipeline, Protocol protocol) {
123+
private void configureProtocolHandlers(Channel ch, ChannelPipeline pipeline, Protocol protocol, boolean sslContextPresent) {
124+
if (!sslContextPresent) {
125+
configureAssumeProtocol(ch, pipeline, protocol);
126+
return;
127+
}
128+
122129
switch (protocolNegotiation) {
123130
case ASSUME_PROTOCOL:
124131
configureAssumeProtocol(ch, pipeline, protocol);

‎http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientAlpnTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ public void priorKnowledgeClient_serverWithoutAlpnSupport_requestSucceeds() thro
9696

9797
@Test
9898
@EnabledIf("alpnSupported")
99-
public void alpnClient_httpRequest_throwsException() throws Exception {
99+
public void clientWithUserConfiguredAlpn_httpRequest_throwsException() throws Exception {
100100
initClient(ProtocolNegotiation.ALPN, SslProvider.JDK);
101101
initServer(true);
102102
UnsupportedOperationException e = assertThrows(UnsupportedOperationException.class, this::makeHttpRequest);
103-
assertThat(e.getMessage()).isEqualTo("ALPN can only be used with HTTPS, not HTTP.");
103+
assertThat(e.getMessage()).isEqualTo("ALPN can only be used with HTTPS, not HTTP. Use ProtocolNegotiation.ASSUME_PROTOCOL instead.");
104104
}
105105

106106
private void makeHttpsRequest() throws Exception {

‎test/codegen-generated-classes-test/src/main/resources/codegen-resources/h2/service-2.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"version":"2.0",
3+
"metadata":{
4+
"apiVersion":"2016-03-11",
5+
"endpointPrefix":"h2-service",
6+
"jsonVersion":"1.1",
7+
"protocol":"rest-json",
8+
"protocolSettings":{"h2":"eventstream"},
9+
"serviceAbbreviation":"H2 Service",
10+
"serviceFullName":"H2 Test Service",
11+
"serviceId":"H2 Service",
12+
"signatureVersion":"v4",
13+
"targetPrefix":"ProtocolTestsService",
14+
"uid":"restjson-2016-03-11"
15+
},
16+
"operations":{
17+
"OneOperation":{
18+
"name":"OneOperation",
19+
"http":{
20+
"method":"POST",
21+
"requestUri":"/2016-03-11/oneoperation"
22+
},
23+
"input":{"shape":"OneShape"}
24+
}
25+
},
26+
"shapes": {
27+
"OneShape": {
28+
"type": "structure",
29+
"members": {
30+
"StringMember": {
31+
"shape": "String"
32+
}
33+
}
34+
},
35+
"String":{"type":"string"}
36+
}
37+
}

‎test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/h2/AlpnHttpTest.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
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.services.h2;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
21+
import java.lang.invoke.MethodHandle;
22+
import java.lang.invoke.MethodHandles;
23+
import java.lang.invoke.MethodType;
24+
import java.net.ConnectException;
25+
import java.net.URI;
26+
import java.security.AccessController;
27+
import java.security.PrivilegedExceptionAction;
28+
import java.util.concurrent.CompletionException;
29+
import javax.net.ssl.SSLContext;
30+
import javax.net.ssl.SSLEngine;
31+
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.api.condition.EnabledIf;
33+
import software.amazon.awssdk.core.exception.SdkClientException;
34+
import software.amazon.awssdk.http.Protocol;
35+
import software.amazon.awssdk.http.ProtocolNegotiation;
36+
import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient;
37+
38+
public class AlpnHttpTest {
39+
40+
@Test
41+
@EnabledIf("alpnSupported")
42+
public void clientWithDefaultSettingAlpn_httpRequest_doesNotThrowUnsupportedOperationException() {
43+
H2AsyncClient client = clientBuilderWithHttpEndpoint().build();
44+
45+
CompletionException e = assertThrows(CompletionException.class, () -> makeRequest(client));
46+
assertThat(e).hasCauseInstanceOf(SdkClientException.class);
47+
assertThat(e.getCause()).hasCauseInstanceOf(ConnectException.class);
48+
assertThat(e.getMessage()).contains("Connection refused");
49+
assertThat(e.getMessage()).doesNotContain("ALPN can only be used with HTTPS, not HTTP. Use ProtocolNegotiation.ASSUME_PROTOCOL instead.");
50+
}
51+
52+
@Test
53+
@EnabledIf("alpnSupported")
54+
public void clientWithUserConfiguredAlpn_httpRequest_throwsUnsupportedOperationException() {
55+
H2AsyncClient client = clientBuilderWithHttpEndpoint().httpClient(NettyNioAsyncHttpClient.builder()
56+
.protocolNegotiation(ProtocolNegotiation.ALPN)
57+
.protocol(Protocol.HTTP2)
58+
.build())
59+
.build();
60+
61+
CompletionException e = assertThrows(CompletionException.class, () -> makeRequest(client));
62+
assertThat(e).hasCauseInstanceOf(SdkClientException.class);
63+
assertThat(e.getCause()).hasCauseInstanceOf(UnsupportedOperationException.class);
64+
assertThat(e.getMessage()).contains("ALPN can only be used with HTTPS, not HTTP. Use ProtocolNegotiation.ASSUME_PROTOCOL instead.");
65+
}
66+
67+
private H2AsyncClientBuilder clientBuilderWithHttpEndpoint() {
68+
return H2AsyncClient.builder()
69+
.endpointOverride(URI.create("http://localhost:8080"));
70+
}
71+
72+
private void makeRequest(H2AsyncClient client) {
73+
client.oneOperation(c -> c.stringMember("payload")).join();
74+
}
75+
76+
private static boolean alpnSupported() {
77+
try {
78+
SSLContext context = SSLContext.getInstance("TLS");
79+
context.init(null, null, null);
80+
SSLEngine engine = context.createSSLEngine();
81+
MethodHandles.Lookup lookup = MethodHandles.lookup();
82+
83+
MethodHandle getApplicationProtocol = AccessController.doPrivileged(
84+
(PrivilegedExceptionAction<MethodHandle>) () ->
85+
lookup.findVirtual(SSLEngine.class, "getApplicationProtocol", MethodType.methodType(String.class)));
86+
87+
getApplicationProtocol.invoke(engine);
88+
return true;
89+
} catch (Throwable t) {
90+
return false;
91+
}
92+
}
93+
}

0 commit comments

Comments
 (0)