Fixes #9183 - ConnectHandler may close the connection instead of send… (#9184)

* Fixes #9183 - ConnectHandler may close the connection instead of sending 200 OK.

Delaying the call to UpstreamConnection.fillInterested() until the 200 OK response has been sent.

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

jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java

@@ -26,6 +26,8 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.Executor;
 import javax.servlet.AsyncContext;
+import javax.servlet.AsyncEvent;
+import javax.servlet.AsyncListener;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -560,9 +562,9 @@ public class ConnectHandler extends HandlerWrapper
         }
     }
 
-    public class UpstreamConnection extends ProxyConnection
+    public class UpstreamConnection extends ProxyConnection implements AsyncListener
     {
-        private ConnectContext connectContext;
+        private final ConnectContext connectContext;
 
         public UpstreamConnection(EndPoint endPoint, Executor executor, ByteBufferPool bufferPool, ConnectContext connectContext)
         {
@@ -574,8 +576,9 @@ public class ConnectHandler extends HandlerWrapper
         public void onOpen()
         {
             super.onOpen();
+            // Delay fillInterested() until the 200 OK response has been sent.
+            connectContext.asyncContext.addListener(this);
             onConnectSuccess(connectContext, UpstreamConnection.this);
-            fillInterested();
         }
 
         @Override
@@ -589,6 +592,28 @@ public class ConnectHandler extends HandlerWrapper
         {
             ConnectHandler.this.write(endPoint, buffer, callback, getContext());
         }
+
+        @Override
+        public void onComplete(AsyncEvent event)
+        {
+            fillInterested();
+        }
+
+        @Override
+        public void onTimeout(AsyncEvent event)
+        {
+        }
+
+        @Override
+        public void onError(AsyncEvent event)
+        {
+            close(event.getThrowable());
+        }
+
+        @Override
+        public void onStartAsync(AsyncEvent event)
+        {
+        }
     }
 
     public class DownstreamConnection extends ProxyConnection implements Connection.UpgradeTo

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

@@ -18,6 +18,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.InetAddress;
+import java.net.ServerSocket;
 import java.net.Socket;
 import java.net.UnknownHostException;
 import java.nio.ByteBuffer;
@@ -47,6 +48,7 @@ import org.junit.jupiter.api.Test;
 
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -84,6 +86,59 @@ public class ConnectHandlerTest extends AbstractConnectHandlerTest
         }
     }
 
+    @Test
+    public void testCONNECTAndClose() throws Exception
+    {
+        disposeProxy();
+        connectHandler = new ConnectHandler()
+        {
+            @Override
+            protected void handleConnect(Request baseRequest, HttpServletRequest request, HttpServletResponse response, String serverAddress)
+            {
+                try
+                {
+                    super.handleConnect(baseRequest, request, response, serverAddress);
+                    // Delay the return of this method to trigger the race
+                    // with the server closing the connection immediately.
+                    Thread.sleep(500);
+                }
+                catch (InterruptedException x)
+                {
+                    throw new RuntimeException(x);
+                }
+            }
+        };
+        proxy.setHandler(connectHandler);
+        proxy.start();
+
+        try (ServerSocket server = new ServerSocket(0))
+        {
+            String hostPort = "localhost:" + server.getLocalPort();
+            String request =
+                "CONNECT " + hostPort + " HTTP/1.1\r\n" +
+                "Host: " + hostPort + "\r\n" +
+                "\r\n";
+            try (Socket socket = newSocket())
+            {
+                OutputStream output = socket.getOutputStream();
+                output.write(request.getBytes(StandardCharsets.UTF_8));
+                output.flush();
+
+                Socket serverSocket = server.accept();
+                // Close immediately to trigger the race with
+                // the return from ConnectHandler.handle().
+                serverSocket.close();
+
+                // Expect 200 OK from the CONNECT request
+                HttpTester.Response response = HttpTester.parseResponse(HttpTester.from(socket.getInputStream()));
+                assertNotNull(response);
+                assertEquals(HttpStatus.OK_200, response.getStatus());
+                // Expect the connection to be closed.
+                assertEquals(-1, socket.getInputStream().read());
+            }
+        }
+    }
+
     @Test
     public void testCONNECTwithIPv6() throws Exception
     {