From 4b838ea32e523466dc884909083020319c31e535 Mon Sep 17 00:00:00 2001 From: hdavidh Date: Mon, 3 Feb 2025 15:17:21 -0800 Subject: [PATCH] Skip ALPN if request is HTTP endpoint --- .../nio/netty/NettyNioAsyncHttpClient.java | 13 ++- .../internal/ChannelPipelineInitializer.java | 15 ++- .../http/nio/netty/NettyClientAlpnTest.java | 2 +- .../codegen-resources/h2/service-2.json | 37 ++++++++ .../awssdk/services/h2/AlpnHttpTest.java | 93 +++++++++++++++++++ 5 files changed, 152 insertions(+), 8 deletions(-) create mode 100644 test/codegen-generated-classes-test/src/main/resources/codegen-resources/h2/service-2.json create mode 100644 test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/h2/AlpnHttpTest.java diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java index ab395b54251..a6f092433c4 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/NettyNioAsyncHttpClient.java @@ -86,6 +86,7 @@ public final class NettyNioAsyncHttpClient implements SdkAsyncHttpClient { private final SdkChannelPoolMap pools; private final NettyConfiguration configuration; private final ProtocolNegotiation protocolNegotiation; + private boolean userSetAlpn; private NettyNioAsyncHttpClient(DefaultBuilder builder, AttributeMap serviceDefaultsMap) { this.configuration = new NettyConfiguration(serviceDefaultsMap); @@ -135,9 +136,14 @@ public CompletableFuture execute(AsyncExecuteRequest request) { } private void failIfAlpnUsedWithHttp(AsyncExecuteRequest request) { - if (protocolNegotiation == ProtocolNegotiation.ALPN - && "http".equals(request.request().protocol())) { - throw new UnsupportedOperationException("ALPN can only be used with HTTPS, not HTTP."); + if ("http".equals(request.request().protocol())) { + if (userSetAlpn) { + throw new UnsupportedOperationException("ALPN can only be used with HTTPS, not HTTP."); + } + if (protocolNegotiation == ProtocolNegotiation.ALPN) { + log.debug(null, () -> "Default client protocol negotiation is ALPN, but request endpoint is HTTP. Prior " + + "knowledge negotiation will be used instead"); + } } } @@ -183,6 +189,7 @@ private SslProvider resolveSslProvider(DefaultBuilder builder) { private ProtocolNegotiation resolveProtocolNegotiation(ProtocolNegotiation userSetValue, AttributeMap serviceDefaultsMap, Protocol protocol, SslProvider sslProvider) { if (userSetValue == ProtocolNegotiation.ALPN) { + this.userSetAlpn = true; // TODO - remove once we implement support for ALPN with HTTP1 if (protocol == Protocol.HTTP1_1) { throw new UnsupportedOperationException("ALPN with HTTP/1.1 is not yet supported, use prior knowledge instead " diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java index 6401a8ecc58..6364c7bb915 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java @@ -99,7 +99,9 @@ public void channelCreated(Channel ch) { ch.attr(CHANNEL_DIAGNOSTICS).set(new ChannelDiagnostics(ch)); ch.attr(PROTOCOL_FUTURE).set(new CompletableFuture<>()); ChannelPipeline pipeline = ch.pipeline(); - if (sslCtx != null) { + + boolean sslCtxPresent = sslCtx != null; + if (sslCtxPresent) { SslHandler sslHandler = newSslHandler(sslCtx, ch.alloc(), poolKey.getHost(), poolKey.getPort(), configuration.tlsHandshakeTimeout()); @@ -114,11 +116,16 @@ public void channelCreated(Channel ch) { } } - configureProtocolHandlers(ch, pipeline, protocol); + configureProtocolHandlers(ch, pipeline, protocol, sslCtxPresent); configurePostProtocolHandlers(pipeline, protocol); } - private void configureProtocolHandlers(Channel ch, ChannelPipeline pipeline, Protocol protocol) { + private void configureProtocolHandlers(Channel ch, ChannelPipeline pipeline, Protocol protocol, boolean sslContextPresent) { + if (!sslContextPresent) { + configureAssumeProtocol(ch, pipeline, protocol); + return; + } + switch (protocolNegotiation) { case ASSUME_PROTOCOL: configureAssumeProtocol(ch, pipeline, protocol); @@ -184,7 +191,7 @@ private void configureHttp11(Channel ch, ChannelPipeline pipeline) { } private void configureAlpnH2(ChannelPipeline pipeline) { - pipeline.addLast(new ApplicationProtocolNegotiationHandler("") { + pipeline.addLast(new ApplicationProtocolNegotiationHandler(ApplicationProtocolNames.HTTP_2) { @Override protected void configurePipeline(ChannelHandlerContext ctx, String protocol) { if (protocol.equals(ApplicationProtocolNames.HTTP_2)) { diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientAlpnTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientAlpnTest.java index 280bb60fa56..d9dd1b65a0a 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientAlpnTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/NettyClientAlpnTest.java @@ -96,7 +96,7 @@ public void priorKnowledgeClient_serverWithoutAlpnSupport_requestSucceeds() thro @Test @EnabledIf("alpnSupported") - public void alpnClient_httpRequest_throwsException() throws Exception { + public void clientWithUserConfiguredAlpn_httpRequest_throwsException() throws Exception { initClient(ProtocolNegotiation.ALPN, SslProvider.JDK); initServer(true); UnsupportedOperationException e = assertThrows(UnsupportedOperationException.class, this::makeHttpRequest); diff --git a/test/codegen-generated-classes-test/src/main/resources/codegen-resources/h2/service-2.json b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/h2/service-2.json new file mode 100644 index 00000000000..20d9e3d1dba --- /dev/null +++ b/test/codegen-generated-classes-test/src/main/resources/codegen-resources/h2/service-2.json @@ -0,0 +1,37 @@ +{ + "version":"2.0", + "metadata":{ + "apiVersion":"2016-03-11", + "endpointPrefix":"h2-service", + "jsonVersion":"1.1", + "protocol":"rest-json", + "protocolSettings":{"h2":"eventstream"}, + "serviceAbbreviation":"H2 Service", + "serviceFullName":"H2 Test Service", + "serviceId":"H2 Service", + "signatureVersion":"v4", + "targetPrefix":"ProtocolTestsService", + "uid":"restjson-2016-03-11" + }, + "operations":{ + "OneOperation":{ + "name":"OneOperation", + "http":{ + "method":"POST", + "requestUri":"/2016-03-11/oneoperation" + }, + "input":{"shape":"OneShape"} + } + }, + "shapes": { + "OneShape": { + "type": "structure", + "members": { + "StringMember": { + "shape": "String" + } + } + }, + "String":{"type":"string"} + } +} \ No newline at end of file diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/h2/AlpnHttpTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/h2/AlpnHttpTest.java new file mode 100644 index 00000000000..a9f660be5c2 --- /dev/null +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/h2/AlpnHttpTest.java @@ -0,0 +1,93 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.services.h2; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.net.ConnectException; +import java.net.URI; +import java.security.AccessController; +import java.security.PrivilegedExceptionAction; +import java.util.concurrent.CompletionException; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIf; +import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.http.Protocol; +import software.amazon.awssdk.http.ProtocolNegotiation; +import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; + +public class AlpnHttpTest { + + @Test + @EnabledIf("alpnSupported") + public void clientWithDefaultSettingAlpn_httpRequest_doesNotThrowUnsupportedOperationException() { + H2AsyncClient client = clientBuilderWithHttpEndpoint().build(); + + CompletionException e = assertThrows(CompletionException.class, () -> makeRequest(client)); + assertThat(e).hasCauseInstanceOf(SdkClientException.class); + assertThat(e.getCause()).hasCauseInstanceOf(ConnectException.class); + assertThat(e.getMessage()).contains("Connection refused"); + assertThat(e.getMessage()).doesNotContain("ALPN can only be used with HTTPS, not HTTP"); + } + + @Test + @EnabledIf("alpnSupported") + public void clientWithUserConfiguredAlpn_httpRequest_throwsUnsupportedOperationException() { + H2AsyncClient client = clientBuilderWithHttpEndpoint().httpClient(NettyNioAsyncHttpClient.builder() + .protocolNegotiation(ProtocolNegotiation.ALPN) + .protocol(Protocol.HTTP2) + .build()) + .build(); + + CompletionException e = assertThrows(CompletionException.class, () -> makeRequest(client)); + assertThat(e).hasCauseInstanceOf(SdkClientException.class); + assertThat(e.getCause()).hasCauseInstanceOf(UnsupportedOperationException.class); + assertThat(e.getMessage()).contains("ALPN can only be used with HTTPS, not HTTP"); + } + + private H2AsyncClientBuilder clientBuilderWithHttpEndpoint() { + return H2AsyncClient.builder() + .endpointOverride(URI.create("http://localhost:8080")); + } + + private void makeRequest(H2AsyncClient client) { + client.oneOperation(c -> c.stringMember("payload")).join(); + } + + private static boolean alpnSupported() { + try { + SSLContext context = SSLContext.getInstance("TLS"); + context.init(null, null, null); + SSLEngine engine = context.createSSLEngine(); + MethodHandles.Lookup lookup = MethodHandles.lookup(); + + MethodHandle getApplicationProtocol = AccessController.doPrivileged( + (PrivilegedExceptionAction) () -> + lookup.findVirtual(SSLEngine.class, "getApplicationProtocol", MethodType.methodType(String.class))); + + getApplicationProtocol.invoke(engine); + return true; + } catch (Throwable t) { + return false; + } + } +}