Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ALPN H2 Support for Netty Client #5794

Merged
merged 24 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
050d7ce
ALPN H2 support in Netty client
davidh44 Jan 14, 2025
815a0da
Add codegen customizations to skip ALPN for existing H2 services
davidh44 Jan 14, 2025
6df6a6c
Add changelog
davidh44 Jan 14, 2025
1a01e77
Add benchmarks
davidh44 Jan 14, 2025
1e99886
Add tests
davidh44 Jan 14, 2025
93d096a
Update test
davidh44 Jan 14, 2025
b557813
Update test
davidh44 Jan 14, 2025
59c8096
Address comments
davidh44 Jan 14, 2025
d54d3ac
Update test file name
davidh44 Jan 14, 2025
a0eab5f
Revert to checking Java version for ALPN support
davidh44 Jan 15, 2025
86b6638
Update Kinesis integ tests
davidh44 Jan 15, 2025
d2b2d3a
Propagate exception and close channel
davidh44 Jan 15, 2025
deb0bcd
Add tests and update ALPN support check for OpenSsl
davidh44 Jan 15, 2025
c041db7
Set max streams when completing protocol future
davidh44 Jan 15, 2025
5b60892
Add test dependencies
davidh44 Jan 15, 2025
40c3bfc
Do not use FATAL_ALERT if OpenSSL is used
davidh44 Jan 16, 2025
65353bb
Remove completing future in ALPN handler
davidh44 Jan 17, 2025
a3af0ca
Remove import
davidh44 Jan 17, 2025
f79e297
Merge branch 'master' into feature/master/alpn-h2-netty-client
davidh44 Jan 21, 2025
6caf668
Update ALPN support check to check for getApplicationProtocol method …
davidh44 Jan 21, 2025
0629830
Address comments
davidh44 Jan 22, 2025
1dca786
Merge branch 'master' into feature/master/alpn-h2-netty-client
davidh44 Jan 22, 2025
422b6ab
Use Lazy for alpn support check
davidh44 Jan 22, 2025
edee03f
Merge branch 'master' into feature/master/alpn-h2-netty-client
davidh44 Jan 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,14 @@ public final class NettyNioAsyncHttpClient implements SdkAsyncHttpClient {
private final SdkEventLoopGroup sdkEventLoopGroup;
private final SdkChannelPoolMap<URI, ? extends SdkChannelPool> pools;
private final NettyConfiguration configuration;
private final ProtocolNegotiation protocolNegotiation;

private NettyNioAsyncHttpClient(DefaultBuilder builder, AttributeMap serviceDefaultsMap) {
this.configuration = new NettyConfiguration(serviceDefaultsMap);
Protocol protocol = serviceDefaultsMap.get(SdkHttpConfigurationOption.PROTOCOL);
SslProvider sslProvider = resolveSslProvider(builder);
ProtocolNegotiation protocolNegotiation = resolveProtocolNegotiation(builder.protocolNegotiation, serviceDefaultsMap,
protocol, sslProvider);
this.protocolNegotiation = resolveProtocolNegotiation(builder.protocolNegotiation, serviceDefaultsMap,
protocol, sslProvider);
this.sdkEventLoopGroup = eventLoopGroup(builder);

Http2Configuration http2Configuration = builder.http2Configuration;
Expand All @@ -117,19 +118,29 @@ private NettyNioAsyncHttpClient(DefaultBuilder builder, AttributeMap serviceDefa
@SdkTestInternalApi
NettyNioAsyncHttpClient(SdkEventLoopGroup sdkEventLoopGroup,
SdkChannelPoolMap<URI, ? extends SdkChannelPool> pools,
NettyConfiguration configuration) {
NettyConfiguration configuration,
ProtocolNegotiation protocolNegotiation) {
this.sdkEventLoopGroup = sdkEventLoopGroup;
this.pools = pools;
this.configuration = configuration;
this.protocolNegotiation = protocolNegotiation;
}

@Override
public CompletableFuture<Void> execute(AsyncExecuteRequest request) {
failIfAlpnUsedWithHttp(request);
RequestContext ctx = createRequestContext(request);
ctx.metricCollector().reportMetric(HTTP_CLIENT_NAME, clientName()); // TODO: Can't this be done in core?
return new NettyRequestExecutor(ctx).execute();
}

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.");
davidh44 marked this conversation as resolved.
Show resolved Hide resolved
}
}

public static Builder builder() {
return new DefaultBuilder();
}
Expand Down Expand Up @@ -410,15 +421,20 @@ public interface Builder extends SdkAsyncHttpClient.Builder<NettyNioAsyncHttpCli

/**
* If set to {@code ProtocolNegotiation.ALPN}, the request will be made using ALPN, without a fallback protocol.
* If ALPN is not supported by the server an exception will be thrown.
* <p>
* Default value is {@code ProtocolNegotiation.ALPN} for services with H2 protocol setting, with the exception of the
* following services: Kinesis, TranscribeStreaming, Lex Runtime v2, Q Business
* <p>
* If ALPN is not supported by the server, an exception will be thrown.
* <p>Default values:</p>
* <ol>
* <li>For services with H2 protocol setting, the default value is {@code ProtocolNegotiation.ALPN},
* with the exception of the following services: Kinesis, Transcribe Streaming, Lex Runtime v2, Q Business.</li>
* <li>For all other services, the default value is {@code ProtocolNegotiation.ASSUME_PROTOCOL}, in which case the SDK
* will use prior knowledge to establish connections.</li>
* </ol>
* Note: For Java 8, ALPN is only supported in versions 1.8.0_251 and newer.
* If on unsupported Java version:
* 1) Default SDK setting of true -> SDK will fallback to prior knowledge and not use ALPN
* 2) User explicitly sets value of true -> Exception will be thrown
* If on an unsupported Java version and using {@code SslProvider.JDK}:
* <ol>
* <li>Default SDK setting of ALPN → SDK will fallback to prior knowledge and not use ALPN.</li>
* <li>User explicitly sets value of ALPN → Exception will be thrown.</li>
* </ol>
*/
Builder protocolNegotiation(ProtocolNegotiation protocolNegotiation);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private void configureProtocolHandlers(Channel ch, ChannelPipeline pipeline, Pro
configureAlpn(pipeline, protocol);
break;
default:
throw new UnsupportedOperationException("");
throw new UnsupportedOperationException("Unsupported ProtocolNegotiation: " + protocolNegotiation);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.lang.invoke.MethodType;
import java.nio.channels.ClosedChannelException;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.time.Duration;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -68,6 +69,8 @@ public final class NettyUtils {
+ "read or written in a timely manner.";
private static final Logger log = Logger.loggerFor(NettyUtils.class);

private static volatile Boolean alpnSupported;

private NettyUtils() {
}

Expand Down Expand Up @@ -403,6 +406,20 @@ public static boolean isAlpnSupported(SslProvider sslProvider) {
return true;
}

Boolean supported = alpnSupported;
if (supported != null) {
return supported;
}

synchronized (NettyUtils.class) {
if (alpnSupported == null) {
alpnSupported = checkAlpnSupport();
}
davidh44 marked this conversation as resolved.
Show resolved Hide resolved
return alpnSupported;
}
}

private static boolean checkAlpnSupport() {
try {
SSLContext context = SSLContext.getInstance("TLS");
davidh44 marked this conversation as resolved.
Show resolved Hide resolved
context.init(null, null, null);
Expand All @@ -411,15 +428,15 @@ public static boolean isAlpnSupported(SslProvider sslProvider) {

MethodHandle getApplicationProtocol = AccessController.doPrivileged(
(PrivilegedExceptionAction<MethodHandle>) () ->
lookup.findVirtual(SSLEngine.class, "getApplicationProtocol", MethodType.methodType(String.class)));
lookup.findVirtual(SSLEngine.class, "getApplicationProtocol", MethodType.methodType(String.class)));

getApplicationProtocol.invoke(engine);
return true;
} catch (NoSuchMethodException e) {
log.info(() -> "ALPN not supported: SSLEngine.getApplicationProtocol() method not found.");
} catch (PrivilegedActionException e) {
log.debug(() -> "ALPN not supported: SSLEngine.getApplicationProtocol() method not found: " + e);
return false;
} catch (Throwable t) {
log.info(() -> "ALPN support check failed: " + t.getMessage());
log.debug(() -> "ALPN support check failed: " + t);
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClientTestUtils.createRequest;

import io.netty.handler.ssl.SslProvider;
import java.net.URI;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -56,22 +57,22 @@ private static void initServer(boolean useAlpn) throws Exception {
public void alpnClientJdkProvider_serverWithAlpnSupport_requestSucceeds() throws Exception {
initClient(ProtocolNegotiation.ALPN, SslProvider.JDK);
initServer(true);
makeSimpleRequest();
makeHttpsRequest();
}

@Test
public void alpnClientOpenSslProvider_serverWithAlpnSupport_requestSucceeds() throws Exception {
initClient(ProtocolNegotiation.ALPN, SslProvider.OPENSSL);
initServer(true);
makeSimpleRequest();
makeHttpsRequest();
}

@Test
@EnabledIf("alpnSupported")
public void alpnClient_serverWithoutAlpnSupport_throwsException() throws Exception {
initClient(ProtocolNegotiation.ALPN, SslProvider.JDK);
initServer(false);
ExecutionException e = assertThrows(ExecutionException.class, this::makeSimpleRequest);
ExecutionException e = assertThrows(ExecutionException.class, this::makeHttpsRequest);
assertThat(e).hasCauseInstanceOf(UnsupportedOperationException.class);
assertThat(e.getMessage()).contains("The server does not support ALPN with H2");
}
Expand All @@ -81,18 +82,35 @@ public void alpnClient_serverWithoutAlpnSupport_throwsException() throws Excepti
public void priorKnowledgeClient_serverWithAlpnSupport_requestSucceeds() throws Exception {
initClient(ProtocolNegotiation.ASSUME_PROTOCOL, SslProvider.JDK);
initServer(true);
makeSimpleRequest();
makeHttpsRequest();
}

@Test
public void priorKnowledgeClient_serverWithoutAlpnSupport_requestSucceeds() throws Exception {
initClient(ProtocolNegotiation.ASSUME_PROTOCOL, SslProvider.JDK);
initServer(false);
makeSimpleRequest();
makeHttpsRequest();
}

private void makeSimpleRequest() throws Exception {
SdkHttpRequest request = createRequest(mockServer.getHttpsUri());
@Test
@EnabledIf("alpnSupported")
public void alpnClient_httpRequest_throwsException() throws Exception {
initClient(ProtocolNegotiation.ALPN, SslProvider.JDK);
initServer(true);
UnsupportedOperationException e = assertThrows(UnsupportedOperationException.class, this::makeHttpRequest);
assertThat(e.getMessage()).isEqualTo("ALPN can only be used with HTTPS, not HTTP.");
}

private void makeHttpsRequest() throws Exception {
makeSimpleRequest(mockServer.getHttpsUri());
}

private void makeHttpRequest() throws Exception {
makeSimpleRequest(mockServer.getHttpUri());
}

private void makeSimpleRequest(URI uri) throws Exception {
SdkHttpRequest request = createRequest(uri);
RecordingResponseHandler recorder = new RecordingResponseHandler();
sdkHttpClient.execute(AsyncExecuteRequest.builder().request(request).requestContentPublisher(createProvider("")).responseHandler(recorder).build());
recorder.completeFuture.get(5, TimeUnit.SECONDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import org.mockito.stubbing.Answer;
import software.amazon.awssdk.http.HttpMetric;
import software.amazon.awssdk.http.HttpTestUtils;
import software.amazon.awssdk.http.ProtocolNegotiation;
import software.amazon.awssdk.http.SdkHttpConfigurationOption;
import software.amazon.awssdk.http.SdkHttpFullRequest;
import software.amazon.awssdk.http.SdkHttpMethod;
Expand Down Expand Up @@ -310,7 +311,7 @@ protected SdkChannelPool newPool(URI key) {
NettyConfiguration nettyConfiguration = new NettyConfiguration(AttributeMap.empty());

SdkAsyncHttpClient customerClient =
new NettyNioAsyncHttpClient(eventLoopGroup, sdkChannelPoolMap, nettyConfiguration);
new NettyNioAsyncHttpClient(eventLoopGroup, sdkChannelPoolMap, nettyConfiguration, ProtocolNegotiation.ASSUME_PROTOCOL);

customerClient.close();
assertThat(eventLoopGroup.eventLoopGroup().isShuttingDown()).isTrue();
Expand Down