Skip to content

Commit

Permalink
Fixes #9501 - jetty client with proxy Connection: close
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@sbordet
sbordet committed Mar 16, 2023
1 parent e6de810 commit 315dd99
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 15 deletions.
17 changes: 8 additions & 9 deletions jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
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;
Expand Down Expand Up @@ -98,31 +97,32 @@ public void exchangeTerminated(HttpExchange exchange, Result result)

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)
{
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";
}
}
Expand All @@ -138,8 +138,7 @@ else if (sender.isShutdown() && response.getStatus() != HttpStatus.SWITCHING_PRO
}
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();
Expand Down
2 changes: 1 addition & 1 deletion jetty-proxy/src/test/java/org/eclipse/jetty/proxy/AbstractConnectHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
56 changes: 53 additions & 3 deletions jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerSSLTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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;
Expand All @@ -27,8 +28,18 @@
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;
Expand All @@ -39,6 +50,7 @@
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
{
Expand All @@ -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();
Expand All @@ -76,6 +88,7 @@ public void testGETRequest() throws Exception

// 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
Expand All @@ -91,6 +104,7 @@ public void testGETRequest() throws Exception
output.flush();

response = HttpTester.parseResponse(HttpTester.from(sslSocket.getInputStream()));
assertNotNull(response);
assertEquals(HttpStatus.OK_200, response.getStatus());
assertEquals("GET /echo", response.getContent());
}
Expand All @@ -114,6 +128,7 @@ public void testPOSTRequests() throws Exception

// 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
Expand All @@ -133,13 +148,48 @@ public void testPOSTRequests() throws Exception
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());
}
}
}
}

@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();
Expand Down
4 changes: 2 additions & 2 deletions jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ConnectHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -140,7 +140,7 @@ protected void handleConnect(Request baseRequest, HttpServletRequest request, Ht
}

@Test
public void testCONNECTwithIPv6() throws Exception
public void testCONNECTWithIPv6() throws Exception
{
Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable());
String hostPort = "[::1]:" + serverConnector.getLocalPort();
Expand Down

0 comments on commit 315dd99

Please sign in to comment.