From 7fd62668d7335b3271a573a083880fe6d9cf4f1a Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 19 Jan 2023 15:07:22 +0100 Subject: [PATCH] =?UTF-8?q?Fixes=20#9183=20-=20ConnectHandler=20may=20clos?= =?UTF-8?q?e=20the=20connection=20instead=20of=20send=E2=80=A6=20(#9184)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../eclipse/jetty/proxy/ConnectHandler.java | 31 ++++++++++- .../jetty/proxy/ConnectHandlerTest.java | 55 +++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java index b69e64b9361..e2d82b73c76 100644 --- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java +++ b/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 diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerTest.java index cb2c887a229..f15b89a2502 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerTest.java +++ b/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 {