475546 - ClosedChannelException when connecting to HTTPS over HTTP proxy with CONNECT.

Not closing the connection if the request method is CONNECT.

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

@@ -27,6 +27,7 @@ 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.HttpVersion;
 
 public class HttpChannelOverHTTP extends HttpChannel
@@ -96,26 +97,42 @@ public class HttpChannelOverHTTP extends HttpChannel
 
         Response response = result.getResponse();
         HttpFields responseHeaders = response.getHeaders();
-        boolean close = result.isFailed() || receiver.isShutdown();
 
-        if (!close)
+        String closeReason = null;
+        if (result.isFailed())
+            closeReason = "failure";
+        else if (receiver.isShutdown())
+            closeReason = "server close";
+
+        if (closeReason == null)
         {
             if (response.getVersion().compareTo(HttpVersion.HTTP_1_1) < 0)
             {
-                // HTTP 1.0 must close the connection unless it has an explicit keep alive.
+                // HTTP 1.0 must close the connection unless it has
-                close = !responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString());
+                // an explicit keep alive or it's a CONNECT method.
+                boolean keepAlive = responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.KEEP_ALIVE.asString());
+                boolean connect = HttpMethod.CONNECT.is(exchange.getRequest().getMethod());
+                if (!keepAlive && !connect)
+                    closeReason = "http/1.0";
             }
             else
             {
                 // HTTP 1.1 or greater closes only if it has an explicit close.
-                close = responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString());
+                if (responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString()))
+                    closeReason = "http/1.1";
             }
         }
 
-        if (close)
+        if (closeReason != null)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("Closing, reason: {} - {}", closeReason, connection);
             connection.close();
+        }
         else
+        {
             release();
+        }
     }
 
     @Override

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java

@@ -24,6 +24,8 @@ import java.io.OutputStream;
 import java.net.HttpCookie;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
 import java.net.URI;
 import java.net.URLEncoder;
 import java.net.UnknownHostException;
@@ -328,7 +330,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
             public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
             {
                 baseRequest.setHandled(true);
-                consume(request.getInputStream());
+                consume(request.getInputStream(), true);
                 String value = request.getParameter(paramName);
                 if (paramValue.equals(value))
                 {
@@ -359,7 +361,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
             public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
             {
                 baseRequest.setHandled(true);
-                consume(request.getInputStream());
+                consume(request.getInputStream(), true);
             }
         });
 
@@ -393,7 +395,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
             public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
             {
                 baseRequest.setHandled(true);
-                consume(request.getInputStream());
+                consume(request.getInputStream(), true);
             }
         });
 
@@ -1410,7 +1412,7 @@ public class HttpClientTest extends AbstractHttpClientServerTest
                 int count = requests.incrementAndGet();
                 if (count == maxRetries)
                     baseRequest.setHandled(true);
-                consume(request.getInputStream());
+                consume(request.getInputStream(), true);
             }
         });
 
@@ -1476,11 +1478,72 @@ public class HttpClientTest extends AbstractHttpClientServerTest
         Assert.assertTrue(completeLatch.await(5, TimeUnit.SECONDS));
     }
 
-    private void consume(InputStream input) throws IOException
+    @Test
+    public void testCONNECTWithHTTP10() throws Exception
     {
+        ServerSocket server = new ServerSocket(0);
+
+        startClient();
+
+        String host = "localhost";
+        int port = server.getLocalPort();
+
+        Request request = client.newRequest(host, port)
+                .method(HttpMethod.CONNECT)
+                .version(HttpVersion.HTTP_1_0);
+        FuturePromise<Connection> promise = new FuturePromise<>();
+        client.getDestination("http", host, port).newConnection(promise);
+        Connection connection = promise.get(5, TimeUnit.SECONDS);
+        FutureResponseListener listener = new FutureResponseListener(request);
+        connection.send(request, listener);
+
+        try (Socket socket = server.accept())
+        {
+            InputStream input = socket.getInputStream();
+            consume(input, false);
+
+            // HTTP/1.0 response, the client must not close the connection.
+            String httpResponse = "" +
+                    "HTTP/1.0 200 OK\r\n" +
+                    "\r\n";
+            OutputStream output = socket.getOutputStream();
+            output.write(httpResponse.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            ContentResponse response = listener.get(5, TimeUnit.SECONDS);
+            Assert.assertEquals(200, response.getStatus());
+
+            // Test that I can send another request on the same connection.
+            request = client.newRequest(host, port);
+            listener = new FutureResponseListener(request);
+            connection.send(request, listener);
+
+            consume(input, false);
+
+            httpResponse = "" +
+                    "HTTP/1.1 200 OK\r\n" +
+                    "Content-Length: 0\r\n" +
+                    "\r\n";
+            output.write(httpResponse.getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            listener.get(5, TimeUnit.SECONDS);
+        }
+    }
+
+    private void consume(InputStream input, boolean eof) throws IOException
+    {
+        int crlfs = 0;
         while (true)
         {
-            if (input.read() < 0)
+            int read = input.read();
+            if (read == '\r' || read == '\n')
+                ++crlfs;
+            else
+                crlfs = 0;
+            if (!eof && crlfs == 4)
+                break;
+            if (read < 0)
                 break;
         }
     }