Address comments

.changes/next-release/feature-AWSSDKforJavav2-abb9c7e.json

@@ -1,6 +1,6 @@
 {
     "type": "feature",
-    "category": "AWS SDK for Java v2",
+    "category": "Netty NIO HTTP Client",
     "contributor": "",
     "description": "Adds ALPN H2 support for Netty client"
 }

codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java

@@ -259,9 +259,9 @@ public class CustomizationConfig {
     private boolean generateEndpointClientTests;
 
     /**
-     * Whether to skip using ALPN for H2 service.
+     * Whether to use prior knowledge protocol negotiation for H2
      */
-    private boolean skipAlpn;
+    private boolean usePriorKnowledgeForH2;
 
     /**
      * A mapping from the skipped test's description to the reason why it's being skipped.
@@ -751,12 +751,12 @@ public void setGenerateEndpointClientTests(boolean generateEndpointClientTests)
         this.generateEndpointClientTests = generateEndpointClientTests;
     }
 
-    public boolean isSkipAlpn() {
-        return skipAlpn;
+    public boolean isUsePriorKnowledgeForH2() {
+        return usePriorKnowledgeForH2;
     }
 
-    public void setSkipAlpn(boolean skipAlpn) {
-        this.skipAlpn = skipAlpn;
+    public void setUsePriorKnowledgeForH2(boolean usePriorKnowledgeForH2) {
+        this.usePriorKnowledgeForH2 = usePriorKnowledgeForH2;
     }
 
     public boolean useGlobalEndpoint() {

codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClass.java

@@ -639,23 +639,25 @@ private MethodSpec beanStyleSetServiceConfigurationMethod() {
     private void addServiceHttpConfigIfNeeded(TypeSpec.Builder builder, IntermediateModel model) {
         String serviceDefaultFqcn = model.getCustomizationConfig().getServiceSpecificHttpConfig();
         boolean supportsH2 = model.getMetadata().supportsH2();
-        boolean skipAlpn = model.getCustomizationConfig().isSkipAlpn();
+        boolean usePriorKnowledgeForH2 = model.getCustomizationConfig().isUsePriorKnowledgeForH2();
 
         if (serviceDefaultFqcn != null || supportsH2) {
-            builder.addMethod(serviceSpecificHttpConfigMethod(serviceDefaultFqcn, supportsH2, skipAlpn));
+            builder.addMethod(serviceSpecificHttpConfigMethod(serviceDefaultFqcn, supportsH2, usePriorKnowledgeForH2));
         }
     }
 
-    private MethodSpec serviceSpecificHttpConfigMethod(String serviceDefaultFqcn, boolean supportsH2, boolean skipAlpn) {
+    private MethodSpec serviceSpecificHttpConfigMethod(String serviceDefaultFqcn, boolean supportsH2,
+                                                       boolean usePriorKnowledgeForH2) {
         return MethodSpec.methodBuilder("serviceHttpConfig")
                          .addAnnotation(Override.class)
                          .addModifiers(PROTECTED, FINAL)
                          .returns(AttributeMap.class)
-                         .addCode(serviceSpecificHttpConfigMethodBody(serviceDefaultFqcn, supportsH2, skipAlpn))
+                         .addCode(serviceSpecificHttpConfigMethodBody(serviceDefaultFqcn, supportsH2, usePriorKnowledgeForH2))
                          .build();
     }
 
-    private CodeBlock serviceSpecificHttpConfigMethodBody(String serviceDefaultFqcn, boolean supportsH2, boolean skipAlpn) {
+    private CodeBlock serviceSpecificHttpConfigMethodBody(String serviceDefaultFqcn, boolean supportsH2,
+                                                          boolean usePriorKnowledgeForH2) {
         CodeBlock.Builder builder = CodeBlock.builder();
 
         if (serviceDefaultFqcn != null) {
@@ -671,7 +673,7 @@ private CodeBlock serviceSpecificHttpConfigMethodBody(String serviceDefaultFqcn,
                         + ".put($T.PROTOCOL, $T.HTTP2)",
                         SdkHttpConfigurationOption.class, Protocol.class);
 
-            if (!skipAlpn) {
+            if (!usePriorKnowledgeForH2) {
                 builder.add(".put($T.PROTOCOL_NEGOTIATION, $T.ALPN)",
                             SdkHttpConfigurationOption.class, ProtocolNegotiation.class);
             }

codegen/src/test/java/software/amazon/awssdk/codegen/poet/ClientTestModels.java

@@ -335,11 +335,11 @@ public static IntermediateModel serviceWithH2() {
         return new IntermediateModelBuilder(models).build();
     }
 
-    public static IntermediateModel serviceWithH2SkipAlpn() {
+    public static IntermediateModel serviceWithH2UsePriorKnowledgeForH2() {
         File serviceModel =
-            new File(ClientTestModels.class.getResource("client/c2j/service-with-h2-skipAlpn/service-2.json").getFile());
+            new File(ClientTestModels.class.getResource("client/c2j/service-with-h2-usePriorKnowledgeForH2/service-2.json").getFile());
         File customizationModel =
-            new File(ClientTestModels.class.getResource("client/c2j/service-with-h2-skipAlpn/customization.config")
+            new File(ClientTestModels.class.getResource("client/c2j/service-with-h2-usePriorKnowledgeForH2/customization.config")
                                            .getFile());
         C2jModels models = C2jModels
             .builder()

codegen/src/test/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClassTest.java

@@ -23,7 +23,7 @@
 import static software.amazon.awssdk.codegen.poet.ClientTestModels.queryServiceModelsEndpointAuthParamsWithAllowList;
 import static software.amazon.awssdk.codegen.poet.ClientTestModels.restJsonServiceModels;
 import static software.amazon.awssdk.codegen.poet.ClientTestModels.serviceWithH2;
-import static software.amazon.awssdk.codegen.poet.ClientTestModels.serviceWithH2SkipAlpn;
+import static software.amazon.awssdk.codegen.poet.ClientTestModels.serviceWithH2UsePriorKnowledgeForH2;
 import static software.amazon.awssdk.codegen.poet.ClientTestModels.serviceWithNoAuth;
 import static software.amazon.awssdk.codegen.poet.builder.BuilderClassTestUtils.validateGeneration;
 
@@ -125,8 +125,8 @@ void baseClientBuilderClassWithH2() {
     }
 
     @Test
-    void baseClientBuilderClassWithH2SkipAlpn() {
-        validateBaseClientBuilderClassGeneration(serviceWithH2SkipAlpn(), "test-h2-skipAlpn-service-client-builder-class.java");
+    void baseClientBuilderClassWithH2_usePriorKnowledgeForH2() {
+        validateBaseClientBuilderClassGeneration(serviceWithH2UsePriorKnowledgeForH2(), "test-h2-usePriorKnowledgeForH2-service-client-builder-class.java");
     }
 
     private void validateBaseClientBuilderClassGeneration(IntermediateModel model, String expectedClassName) {

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/service-with-h2-usePriorKnowledgeForH2/customization.config

@@ -0,0 +1,3 @@
+{
+    "usePriorKnowledgeForH2": true
+}

http-client-spi/src/main/java/software/amazon/awssdk/http/ProtocolNegotiation.java

@@ -18,7 +18,7 @@
 import software.amazon.awssdk.annotations.SdkPublicApi;
 
 /**
- * The protocol negotiation used by the HTTP client to establish connections
+ * The protocol negotiation selection scheme used by the HTTP client to establish connections
  */
 @SdkPublicApi
 public enum ProtocolNegotiation {

http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java

@@ -18,7 +18,6 @@
 import static software.amazon.awssdk.crtcore.CrtConfigurationUtils.resolveHttpMonitoringOptions;
 import static software.amazon.awssdk.crtcore.CrtConfigurationUtils.resolveProxy;
 import static software.amazon.awssdk.http.SdkHttpConfigurationOption.PROTOCOL;
-import static software.amazon.awssdk.http.SdkHttpConfigurationOption.PROTOCOL_NEGOTIATION;
 import static software.amazon.awssdk.http.crt.internal.AwsCrtConfigurationUtils.buildSocketOptions;
 import static software.amazon.awssdk.http.crt.internal.AwsCrtConfigurationUtils.resolveCipherPreference;
 import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
@@ -38,7 +37,6 @@
 import software.amazon.awssdk.crt.io.TlsContext;
 import software.amazon.awssdk.crt.io.TlsContextOptions;
 import software.amazon.awssdk.http.Protocol;
-import software.amazon.awssdk.http.ProtocolNegotiation;
 import software.amazon.awssdk.http.SdkHttpConfigurationOption;
 import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.http.crt.internal.AwsCrtClientBuilderBase;
@@ -75,11 +73,6 @@ abstract class AwsCrtHttpClientBase implements SdkAutoCloseable {
                                                + "NettyNioAsyncHttpClient instead.");
         }
 
-        if (config.get(PROTOCOL_NEGOTIATION) == ProtocolNegotiation.ALPN) {
-            throw new UnsupportedOperationException("LPN is not supported in AwsCrtHttpClient yet. Use "
-                                                    + "NettyNioAsyncHttpClient instead.");
-        }
-
         try (ClientBootstrap clientBootstrap = new ClientBootstrap(null, null);
              SocketOptions clientSocketOptions = buildSocketOptions(builder.getTcpKeepAliveConfiguration(),
                                                                     config.get(SdkHttpConfigurationOption.CONNECTION_TIMEOUT));

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

@@ -173,8 +173,8 @@ private ProtocolNegotiation resolveProtocolNegotiation(ProtocolNegotiation userS
         if (userSetValue == ProtocolNegotiation.ALPN) {
             // TODO - remove once we implement support for ALPN with HTTP1
             if (protocol == Protocol.HTTP1_1) {
-                throw new UnsupportedOperationException("ALPN with HTTP1 is not yet supported, use prior knowledge instead with "
-                                                        + "ProtocolNegotiation.ASSUME_PROTOCOL, or use ALPN with H2.");
+                throw new UnsupportedOperationException("ALPN with HTTP/1.1 is not yet supported, use prior knowledge instead "
+                                                        + "with ProtocolNegotiation.ASSUME_PROTOCOL, or use ALPN with H2.");
             }
 
             // throw error if not supported and user set ALPN
@@ -411,8 +411,8 @@ 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 true for services with H2 protocol setting, with the exception of the following services:
-         * Kinesis, TranscribeStreaming, Lex Runtime v2, Q Business
+         * 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>
          * Note: For Java 8, ALPN is only supported in versions 1.8.0_251 and newer.
          * If on unsupported Java version:

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

@@ -119,20 +119,33 @@ public void channelCreated(Channel ch) {
     }
 
     private void configureProtocolHandlers(Channel ch, ChannelPipeline pipeline, Protocol protocol) {
-        if (protocolNegotiation == ProtocolNegotiation.ASSUME_PROTOCOL) {
-            if (protocol == Protocol.HTTP1_1) {
-                configureHttp11(ch, pipeline);
-            } else if (protocol == Protocol.HTTP2) {
-                configureHttp2(ch, pipeline);
-            }
-        } else if (protocolNegotiation == ProtocolNegotiation.ALPN) {
-            if (protocol == Protocol.HTTP1_1) {
-                // TODO - verify functionality once we implement ALPN with H1
-                // we will never reach here since we throw error in NettyNioAsyncHttpClient for ALPN + H1
-                configureAlpnH1(pipeline);
-            } else if (protocol == Protocol.HTTP2) {
-                configureAlpnH2(pipeline);
-            }
+        switch (protocolNegotiation) {
+            case ASSUME_PROTOCOL:
+                configureAssumeProtocol(ch, pipeline, protocol);
+                break;
+            case ALPN:
+                configureAlpn(pipeline, protocol);
+                break;
+            default:
+                throw new UnsupportedOperationException("");
+        }
+    }
+
+    private void configureAlpn(ChannelPipeline pipeline, Protocol protocol) {
+        if (protocol == Protocol.HTTP1_1) {
+            // TODO - remove once we implement support for ALPN with HTTP1
+            throw new UnsupportedOperationException("ALPN with HTTP1 is not yet supported, use prior knowledge instead with "
+                                                    + "ProtocolNegotiation.ASSUME_PROTOCOL, or use ALPN with H2.");
+        } else if (protocol == Protocol.HTTP2) {
+            configureAlpnH2(pipeline);
+        }
+    }
+
+    private void configureAssumeProtocol(Channel ch, ChannelPipeline pipeline, Protocol protocol) {
+        if (protocol == Protocol.HTTP1_1) {
+            configureHttp11(ch, pipeline);
+        } else if (protocol == Protocol.HTTP2) {
+            configureHttp2(ch, pipeline);
         }
     }
 
@@ -174,19 +187,10 @@ private void configureAlpnH2(ChannelPipeline pipeline) {
         pipeline.addLast(new ApplicationProtocolNegotiationHandler("") {
             @Override
             protected void configurePipeline(ChannelHandlerContext ctx, String protocol) {
-                if (ApplicationProtocolNames.HTTP_2.equals(protocol)) {
+                if (protocol.equals(ApplicationProtocolNames.HTTP_2)) {
                     configureHttp2(ctx.channel(), ctx.pipeline());
-                }
-            }
-        });
-    }
-
-    private void configureAlpnH1(ChannelPipeline pipeline) {
-        pipeline.addLast(new ApplicationProtocolNegotiationHandler("") {
-            @Override
-            protected void configurePipeline(ChannelHandlerContext ctx, String protocol) {
-                if (ApplicationProtocolNames.HTTP_1_1.equals(protocol)) {
-                    configureHttp11(ctx.channel(), ctx.pipeline());
+                } else {
+                    throw new UnsupportedOperationException("The server does not support ALPN with H2");
                 }
             }
         });

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

@@ -65,8 +65,8 @@ public SslContext sslContext() {
             if (protocolNegotiation == ProtocolNegotiation.ALPN) {
                 builder.applicationProtocolConfig(
                     new ApplicationProtocolConfig(ApplicationProtocolConfig.Protocol.ALPN,
-                                                  ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE,
-                                                  ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT,
+                                                  ApplicationProtocolConfig.SelectorFailureBehavior.FATAL_ALERT,
+                                                  ApplicationProtocolConfig.SelectedListenerFailureBehavior.FATAL_ALERT,
                                                   resolveNettyProtocol(protocol)));
             }
 

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/utils/NettyUtils.java

@@ -392,20 +392,12 @@ public static void runAndLogError(NettyClientLogger log, String errorMsg, Functi
     //  ALPN supported backported in u251
     //  https://bugs.openjdk.org/browse/JDK-8242894
     public static boolean isAlpnSupported() {
-        String javaVersion = getJavaVersion();
-        String[] versionComponents = javaVersion.split("_");
-        if (versionComponents.length == 2) {
-            try {
-                int buildNumber = Integer.parseInt(versionComponents[1].split("-")[0]);
-                if (javaVersion.startsWith("1.8.0") && buildNumber < 251) {
-                    return false;
-                }
-            } catch (NumberFormatException e) {
-                log.error(() -> "Invalid Java version format: " + javaVersion);
-                throw e;
-            }
+        try {
+            Class.forName("sun.security.ssl.ALPNExtension", true, null);
+            return true;
+        } catch (ClassNotFoundException e) {
+            return false;
         }
-        return true;
     }
 
     public static String getJavaVersion() {

services/kinesis/src/main/resources/codegen-resources/customization.config

@@ -53,5 +53,5 @@
         "Invalid ARN: Kinesis ARNs only support stream arn types": "Test is broken for client tests, need operationInputs.",
         "RegionMismatch: client region should be used for endpoint region": "Test is broken for client tests, need operationInputs."
     },
-    "skipAlpn": true
+    "usePriorKnowledgeForH2": true
 }

services/lexruntimev2/src/main/resources/codegen-resources/customization.config

@@ -3,5 +3,5 @@
         "contentType": "application/json"
     },
     "enableGenerateCompiledEndpointRules": true,
-    "skipAlpn": true
+    "usePriorKnowledgeForH2": true
 }

services/qbusiness/src/main/resources/codegen-resources/customization.config

@@ -1,4 +1,4 @@
 {
     "enableGenerateCompiledEndpointRules": true,
-    "skipAlpn": true
+    "usePriorKnowledgeForH2": true
 }

services/transcribestreaming/src/main/resources/codegen-resources/customization.config

@@ -8,5 +8,5 @@
         "AudioStream": ["AudioEvent"],
         "MedicalTranscriptResultStream": ["TranscriptEvent"]
     },
-    "skipAlpn": true
+    "usePriorKnowledgeForH2": true
 }