Advertise only http/1.1 on the hop to an HTTPS proxy

Author: hyperxpro

client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java

@@ -924,6 +924,22 @@ public Future<Channel> updatePipelineForHttpsTunneling(ChannelPipeline pipeline,
     }
 
     public SslHandler addSslHandler(ChannelPipeline pipeline, Uri uri, String virtualHost, boolean hasSocksProxyHandler) {
+        // A WebSocket connection must not negotiate h2 (no RFC 8441 support), so advertise only http/1.1 in ALPN.
+        return addSslHandler(pipeline, uri, virtualHost, hasSocksProxyHandler, !uri.isWebSocket());
+    }
+
+    /**
+     * Same as {@link #addSslHandler(ChannelPipeline, Uri, String, boolean)}, but with ALPN decided by the
+     * caller rather than derived from {@code uri}.
+     * <p>
+     * Deriving it from the URI only holds for a hop that ends at the origin. On the hop to an HTTPS proxy
+     * {@code uri} names the proxy, so nothing about it says WebSocket, yet everything written there is
+     * HTTP/1.1; such callers pass {@code false}.
+     *
+     * @param http2Allowed whether the ALPN advertisement for this hop may include h2
+     */
+    public SslHandler addSslHandler(ChannelPipeline pipeline, Uri uri, String virtualHost, boolean hasSocksProxyHandler,
+                                    boolean http2Allowed) {
         String peerHost;
         int peerPort;
 
@@ -942,8 +958,7 @@ public SslHandler addSslHandler(ChannelPipeline pipeline, Uri uri, String virtua
             peerPort = uri.getExplicitPort();
         }
 
-        // A WebSocket connection must not negotiate h2 (no RFC 8441 support), so advertise only http/1.1 in ALPN.
-        SslHandler sslHandler = createSslHandler(peerHost, peerPort, !uri.isWebSocket());
+        SslHandler sslHandler = createSslHandler(peerHost, peerPort, http2Allowed);
         // Check if SOCKS handler actually exists in the pipeline before trying to add after it
         if (hasSocksProxyHandler && pipeline.get(SOCKS_HANDLER) != null) {
             pipeline.addAfter(SOCKS_HANDLER, SSL_HANDLER, sslHandler);

client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java

@@ -142,9 +142,13 @@ public void onSuccess(Channel channel, InetSocketAddress remoteAddress) {
         if (proxyServer != null && ProxyType.HTTPS.equals(proxyServer.getProxyType())) {
             SslHandler sslHandler;
             try {
-                sslHandler = channelManager.addSslHandler(channel.pipeline(), 
-                    Uri.create("https://" + proxyServer.getHost() + ":" + proxyServer.getSecuredPort()), 
-                    null, false);
+                // This hop ends at the proxy, and everything written on it is HTTP/1.1: a CONNECT, or an
+                // absolute-URI request for a cleartext target. Advertise only http/1.1, otherwise an
+                // h2-capable proxy selects h2 and then drops the connection on our HTTP/1.1 bytes. The hop
+                // to the origin is set up after CONNECT succeeds and may still negotiate h2.
+                sslHandler = channelManager.addSslHandler(channel.pipeline(),
+                    Uri.create("https://" + proxyServer.getHost() + ":" + proxyServer.getSecuredPort()),
+                    null, false, false);
             } catch (Exception sslError) {
                 onFailure(channel, sslError);
                 return;
@@ -170,6 +174,16 @@ protected void onSuccess(Channel value) {
                         NettyConnectListener.this.onFailure(channel, e);
                         return;
                     }
+                    // The engine advertises only http/1.1, so a conformant proxy cannot have selected h2.
+                    // A user-supplied SslContext is reused as-is and can still do it, in which case the
+                    // proxy rejects our HTTP/1.1 bytes and the request dies of a bare "Remotely closed".
+                    // Name the cause, as the WebSocket path below does.
+                    if (ApplicationProtocolNames.HTTP_2.equals(sslHandler.applicationProtocol())) {
+                        LOGGER.warn("HTTPS proxy {}:{} negotiated HTTP/2; AsyncHttpClient speaks only HTTP/1.1 to a "
+                                        + "proxy, so this connection will likely be rejected. Supply an SslContext "
+                                        + "or SslEngineFactory that advertises http/1.1 for the proxy connection.",
+                                proxyServer.getHost(), proxyServer.getSecuredPort());
+                    }
                     // After SSL handshake to proxy, continue with normal proxy request
                     writeRequest(channel);
                 }

client/src/main/java/org/asynchttpclient/netty/ssl/DefaultSslEngineFactory.java

@@ -35,8 +35,9 @@
 public class DefaultSslEngineFactory extends SslEngineFactoryBase {
 
     private volatile SslContext sslContext;
-    // WebSocket connections use a context that advertises only http/1.1 in ALPN: AsyncHttpClient does not
-    // implement RFC 8441 (WebSocket over HTTP/2), so the server must not be able to negotiate h2 for them.
+    // Connections that must stay on HTTP/1.1 use a context that advertises only http/1.1 in ALPN: WebSocket,
+    // because AsyncHttpClient does not implement RFC 8441 (WebSocket over HTTP/2), and the hop to an HTTPS
+    // proxy, which only ever carries an HTTP/1.1 CONNECT or absolute-URI request.
     private volatile SslContext http1OnlySslContext;
 
     private SslContext buildSslContext(AsyncHttpClientConfig config, boolean http2Allowed) throws SSLException {
@@ -98,16 +99,19 @@ public SSLEngine newSslEngine(AsyncHttpClientConfig config, String peerHost, int
     }
 
     /**
-     * Returns the context for a WebSocket connection, which must advertise only http/1.1 (AsyncHttpClient does
-     * not implement RFC 8441, WebSocket over HTTP/2). Built lazily and cached on first use so a client that
-     * never opens a {@code wss://} connection never pays for a second {@link SslContext}.
+     * Returns the context for a connection that must stay on HTTP/1.1 and therefore advertise only http/1.1:
+     * a WebSocket connection (AsyncHttpClient does not implement RFC 8441, WebSocket over HTTP/2), and the hop
+     * to an HTTPS proxy (that hop only ever carries an HTTP/1.1 {@code CONNECT} or absolute-URI request).
+     * Built lazily and cached on first use so a client that never opens such a connection never pays for a
+     * second {@link SslContext}.
      * <p>
      * Only a self-built, h2-enabled context advertises h2 and therefore needs a separate http/1.1-only variant;
      * a user-supplied context or an h2-disabled one already negotiates http/1.1, so it is reused (which also
      * avoids double-releasing it in {@link #destroy()}). <strong>Note:</strong> a user-supplied
      * {@link AsyncHttpClientConfig#getSslContext()} is used as-is for every connection type — if it advertises
-     * h2 in ALPN, a {@code wss://} connection may still negotiate h2, which AHC cannot speak for WebSocket. A
-     * caller needing WebSocket with a custom context must supply one that negotiates http/1.1.
+     * h2 in ALPN, a {@code wss://} connection or an HTTPS-proxy connection may still negotiate h2, which AHC
+     * cannot speak on either. A caller needing those with a custom context must supply one that negotiates
+     * http/1.1.
      */
     private SslContext http1OnlySslContext(AsyncHttpClientConfig config) {
         SslContext ctx = http1OnlySslContext;