From fe505766fd8923017cb7960b92fe8122cecc5486 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 20 Mar 2023 10:09:58 +0100 Subject: [PATCH] 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 --- .../client/http/HttpChannelOverHTTP.java | 17 +++--- .../proxy/AbstractConnectHandlerTest.java | 2 +- .../jetty/proxy/ConnectHandlerSSLTest.java | 56 ++++++++++++++++++- .../jetty/proxy/ConnectHandlerTest.java | 4 +- 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java index 7ede2d4299b..c4cffd0bb1a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java +++ b/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 && !connect) + if (!keepAlive && !isTunnel) closeReason = "http/1.0"; } else { - // HTTP 1.1 closes only if it has an explicit close. - if (responseHeaders.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())) + // 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()) && !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(method, status)) + if (status == HttpStatus.SWITCHING_PROTOCOLS_101 || isTunnel) connection.remove(); else release(); diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AbstractConnectHandlerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AbstractConnectHandlerTest.java index 2f08a656e9a..ff3a1912f37 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AbstractConnectHandlerTest.java +++ b/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); diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerSSLTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerSSLTest.java index 4f323aa4620..fcbb2314f1c 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerSSLTest.java +++ b/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(); - sslContextFactory.setKeyStorePath(keyStorePath); + Path keyStorePath = MavenTestingUtils.getTestResourcePath("server_keystore.p12").toAbsolutePath(); + 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(); 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 2390a548612..ac8638232c6 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 @@ -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();