Fixes #9501 - jetty client with proxy Connection: close (#9508)

Now Connection: close is ignored for 2xx responses to a CONNECT method.
In this way the tunnel is kept open, and bad proxies that were sending
Connection: close are now supported as apparently they are still out there.

Fixes also #6483.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java

@@ -22,7 +22,6 @@ import org.eclipse.jetty.client.api.Result;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpHeaderValue;
-import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MetaData;
@@ -98,14 +97,16 @@ public class HttpChannelOverHTTP extends HttpChannel
 
         String method = exchange.getRequest().getMethod();
         Response response = result.getResponse();
+        int status = response.getStatus();
         HttpFields responseHeaders = response.getHeaders();
+        boolean isTunnel = isTunnel(method, status);
 
         String closeReason = null;
         if (result.isFailed())
             closeReason = "failure";
         else if (receiver.isShutdown())
             closeReason = "server close";
-        else if (sender.isShutdown() && response.getStatus() != HttpStatus.SWITCHING_PROTOCOLS_101)
+        else if (sender.isShutdown() && status != HttpStatus.SWITCHING_PROTOCOLS_101)
             closeReason = "client close";
 
         if (closeReason == null)
@@ -113,16 +114,15 @@ public class HttpChannelOverHTTP extends HttpChannel
             if (response.getVersion().compareTo(HttpVersion.HTTP_1_1) < 0)
             {
                 // HTTP 1.0 must close the connection unless it has
-                // an explicit keep alive or it's a CONNECT method.
+                // an explicit keep alive or it is a CONNECT tunnel.
                 boolean keepAlive = responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString());
-                boolean connect = HttpMethod.CONNECT.is(method);
+                if (!keepAlive && !isTunnel)
-                if (!keepAlive && !connect)
                     closeReason = "http/1.0";
             }
             else
             {
-                // HTTP 1.1 closes only if it has an explicit close.
+                // HTTP 1.1 closes only if it has an explicit close, unless it is a CONNECT tunnel.
-                if (responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()))
+                if (responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()) && !isTunnel)
                     closeReason = "http/1.1";
             }
         }
@@ -138,8 +138,7 @@ public class HttpChannelOverHTTP extends HttpChannel
         }
         else
         {
-            int status = response.getStatus();
+            if (status == HttpStatus.SWITCHING_PROTOCOLS_101 || isTunnel)
-            if (status == HttpStatus.SWITCHING_PROTOCOLS_101 || isTunnel(method, status))
                 connection.remove();
             else
                 release();

jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AbstractConnectHandlerTest.java

@@ -31,7 +31,7 @@ public abstract class AbstractConnectHandlerTest
     protected void prepareProxy() throws Exception
     {
         proxy = new Server();
-        proxyConnector = new ServerConnector(proxy);
+        proxyConnector = new ServerConnector(proxy, 1, 1);
         proxy.addConnector(proxyConnector);
         connectHandler = new ConnectHandler();
         proxy.setHandler(connectHandler);

jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerSSLTest.java

@@ -19,6 +19,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.Socket;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocket;
 import javax.net.ssl.SSLSocketFactory;
@@ -27,8 +28,18 @@ import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.HttpClientTransport;
+import org.eclipse.jetty.client.HttpProxy;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
+import org.eclipse.jetty.client.util.StringRequestContent;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpHeaderValue;
+import org.eclipse.jetty.http.HttpScheme;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.HttpTester;
+import org.eclipse.jetty.io.ClientConnector;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
@@ -39,6 +50,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
 {
@@ -48,11 +60,11 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
     public void prepare() throws Exception
     {
         sslContextFactory = new SslContextFactory.Server();
-        String keyStorePath = MavenTestingUtils.getTestResourceFile("server_keystore.p12").getAbsolutePath();
+        Path keyStorePath = MavenTestingUtils.getTestResourcePath("server_keystore.p12").toAbsolutePath();
-        sslContextFactory.setKeyStorePath(keyStorePath);
+        sslContextFactory.setKeyStorePath(keyStorePath.toString());
         sslContextFactory.setKeyStorePassword("storepwd");
         server = new Server();
-        serverConnector = new ServerConnector(server, sslContextFactory);
+        serverConnector = new ServerConnector(server, 1, 1, sslContextFactory);
         server.addConnector(serverConnector);
         server.setHandler(new ServerHandler());
         server.start();
@@ -76,6 +88,7 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
 
             // Expect 200 OK from the CONNECT request
             HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(socket.getInputStream()));
+            assertNotNull(response);
             assertEquals(HttpStatus.OK_200, response.getStatus());
 
             // Upgrade the socket to SSL
@@ -91,6 +104,7 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
                 output.flush();
 
                 response = HttpTester.parseResponse(HttpTester.from(sslSocket.getInputStream()));
+                assertNotNull(response);
                 assertEquals(HttpStatus.OK_200, response.getStatus());
                 assertEquals("GET /echo", response.getContent());
             }
@@ -114,6 +128,7 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
 
             // Expect 200 OK from the CONNECT request
             HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(socket.getInputStream()));
+            assertNotNull(response);
             assertEquals(HttpStatus.OK_200, response.getStatus());
 
             // Upgrade the socket to SSL
@@ -133,6 +148,7 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
                     output.flush();
 
                     response = HttpTester.parseResponse(HttpTester.from(sslSocket.getInputStream()));
+                    assertNotNull(response);
                     assertEquals(HttpStatus.OK_200, response.getStatus());
                     assertEquals("POST /echo?param=" + i + "\r\nHELLO", response.getContent());
                 }
@@ -140,6 +156,40 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest
         }
     }
 
+    @Test
+    public void testCONNECTWithConnectionClose() throws Exception
+    {
+        disposeProxy();
+        connectHandler = new ConnectHandler()
+        {
+            @Override
+            protected void onConnectSuccess(ConnectContext connectContext, UpstreamConnection upstreamConnection)
+            {
+                // Add Connection: close to the 200 response.
+                connectContext.getResponse().setHeader(HttpHeader.CONNECTION.asString(), HttpHeaderValue.CLOSE.asString());
+                super.onConnectSuccess(connectContext, upstreamConnection);
+            }
+        };
+        proxy.setHandler(connectHandler);
+        proxy.start();
+
+        ClientConnector connector = new ClientConnector();
+        connector.setSslContextFactory(new SslContextFactory.Client(true));
+        HttpClientTransport transport = new HttpClientTransportOverHTTP(connector);
+        HttpClient httpClient = new HttpClient(transport);
+        httpClient.getProxyConfiguration().addProxy(new HttpProxy("localhost", proxyConnector.getLocalPort()));
+        httpClient.start();
+
+        ContentResponse response = httpClient.newRequest("localhost", serverConnector.getLocalPort())
+            .scheme(HttpScheme.HTTPS.asString())
+            .path("/echo")
+            .body(new StringRequestContent("hello"))
+            .send();
+
+        assertEquals(HttpStatus.OK_200, response.getStatus());
+        assertEquals("GET /echo\r\nhello", response.getContentAsString());
+    }
+
     private SSLSocket wrapSocket(Socket socket) throws Exception
     {
         SSLContext sslContext = sslContextFactory.getSslContext();

jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerTest.java

@@ -58,7 +58,7 @@ public class ConnectHandlerTest extends AbstractConnectHandlerTest
     public void prepare() throws Exception
     {
         server = new Server();
-        serverConnector = new ServerConnector(server);
+        serverConnector = new ServerConnector(server, 1, 1);
         server.addConnector(serverConnector);
         server.setHandler(new ServerHandler());
         server.start();
@@ -140,7 +140,7 @@ public class ConnectHandlerTest extends AbstractConnectHandlerTest
     }
 
     @Test
-    public void testCONNECTwithIPv6() throws Exception
+    public void testCONNECTWithIPv6() throws Exception
     {
         Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());
         String hostPort = "[::1]:" + serverConnector.getLocalPort();