Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimone Bordet2015-10-05 05:52:58 -0400
committerSimone Bordet2015-10-05 05:52:58 -0400
commit399755b352b37e02f5baf211870de14072d8a0df (patch)
tree7706f6cfee9474835a59a062090abdb1a98a74e4
parent6aae1265bd71c87c671061d9c70a25fd5056d18d (diff)
downloadorg.eclipse.jetty.project-399755b352b37e02f5baf211870de14072d8a0df.tar.gz
org.eclipse.jetty.project-399755b352b37e02f5baf211870de14072d8a0df.tar.xz
org.eclipse.jetty.project-399755b352b37e02f5baf211870de14072d8a0df.zip
479026 - Wrong CONNECT request idle timeout.
Explicitly set the CONNECT request idle timeout instead of inheriting HttpClient's.
-rw-r--r--jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java35
-rw-r--r--jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java9
-rw-r--r--jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java57
3 files changed, 76 insertions, 25 deletions
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java
index c46bc6cd35..c8b5556af4 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java
@@ -26,7 +26,6 @@ import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.client.api.Connection;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
-import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.http.HttpConnectionOverHTTP;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
@@ -119,7 +118,7 @@ public class HttpProxy extends ProxyConfiguration.Proxy
{
String message = String.format("Cannot perform requests over SSL, no %s in %s",
SslContextFactory.class.getSimpleName(), HttpClient.class.getSimpleName());
- promise.failed(new IllegalStateException(message));
+ tunnelFailed(new IllegalStateException(message));
}
}
else
@@ -131,7 +130,7 @@ public class HttpProxy extends ProxyConfiguration.Proxy
@Override
public void failed(Throwable x)
{
- promise.failed(x);
+ tunnelFailed(x);
}
private void tunnel(HttpDestination destination, final Connection connection)
@@ -139,33 +138,31 @@ public class HttpProxy extends ProxyConfiguration.Proxy
String target = destination.getOrigin().getAddress().asString();
Origin.Address proxyAddress = destination.getConnectAddress();
HttpClient httpClient = destination.getHttpClient();
+ long connectTimeout = httpClient.getConnectTimeout();
Request connect = httpClient.newRequest(proxyAddress.getHost(), proxyAddress.getPort())
.scheme(HttpScheme.HTTP.asString())
.method(HttpMethod.CONNECT)
.path(target)
.header(HttpHeader.HOST, target)
- .timeout(httpClient.getConnectTimeout(), TimeUnit.MILLISECONDS);
+ .idleTimeout(2 * connectTimeout, TimeUnit.MILLISECONDS)
+ .timeout(connectTimeout, TimeUnit.MILLISECONDS);
- connection.send(connect, new Response.CompleteListener()
+ connection.send(connect, result ->
{
- @Override
- public void onComplete(Result result)
+ if (result.isFailed())
{
- if (result.isFailed())
+ tunnelFailed(result.getFailure());
+ }
+ else
+ {
+ Response response = result.getResponse();
+ if (response.getStatus() == 200)
{
- tunnelFailed(result.getFailure());
+ tunnelSucceeded();
}
else
{
- Response response = result.getResponse();
- if (response.getStatus() == 200)
- {
- tunnelSucceeded();
- }
- else
- {
- tunnelFailed(new HttpResponseException("Received " + response + " for " + result.getRequest(), response));
- }
+ tunnelFailed(new HttpResponseException("Received " + response + " for " + result.getRequest(), response));
}
}
});
@@ -198,7 +195,7 @@ public class HttpProxy extends ProxyConfiguration.Proxy
private void tunnelFailed(Throwable failure)
{
endPoint.close();
- failed(failure);
+ promise.failed(failure);
}
}
}
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java
index 3030a97658..9f0aff8703 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java
@@ -168,11 +168,11 @@ public abstract class PoolingHttpDestination<C extends Connection> extends HttpD
}
@Override
- public void close(Connection oldConnection)
+ public void close(Connection connection)
{
- super.close(oldConnection);
+ super.close(connection);
- connectionPool.remove(oldConnection);
+ boolean removed = connectionPool.remove(connection);
if (getHttpExchanges().isEmpty())
{
@@ -192,7 +192,8 @@ public abstract class PoolingHttpDestination<C extends Connection> extends HttpD
// We need to execute queued requests even if this connection failed.
// We may create a connection that is not needed, but it will eventually
// idle timeout, so no worries.
- process();
+ if (removed)
+ process();
}
}
diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java
index caec583278..e563962b6d 100644
--- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java
+++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyTunnellingTest.java
@@ -18,8 +18,6 @@
package org.eclipse.jetty.proxy;
-import static org.junit.Assert.assertEquals;
-
import java.io.IOException;
import java.net.ConnectException;
import java.net.Socket;
@@ -64,6 +62,8 @@ import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+
public class ProxyTunnellingTest
{
@Rule
@@ -279,6 +279,59 @@ public class ProxyTunnellingTest
}
@Test
+ public void testShortIdleTimeoutOverriddenByRequest() throws Exception
+ {
+ // Short idle timeout for HttpClient.
+ long idleTimeout = 500;
+
+ startSSLServer(new ServerHandler());
+ startProxy(new ConnectHandler()
+ {
+ @Override
+ protected void handleConnect(Request baseRequest, HttpServletRequest request, HttpServletResponse response, String serverAddress)
+ {
+ try
+ {
+ // Make sure the proxy remains idle enough.
+ Thread.sleep(2 * idleTimeout);
+ super.handleConnect(baseRequest, request, response, serverAddress);
+ }
+ catch (InterruptedException x)
+ {
+ onConnectFailure(request, response, null, x);
+ }
+ }
+ });
+
+ HttpClient httpClient = new HttpClient(sslContextFactory);
+ httpClient.getProxyConfiguration().getProxies().add(new HttpProxy("localhost", proxyPort()));
+ // Short idle timeout for HttpClient.
+ httpClient.setIdleTimeout(idleTimeout);
+ httpClient.start();
+
+ try
+ {
+ String host = "localhost";
+ String body = "BODY";
+ ContentResponse response = httpClient.newRequest(host, serverConnector.getLocalPort())
+ .scheme(HttpScheme.HTTPS.asString())
+ .method(HttpMethod.GET)
+ .path("/echo?body=" + URLEncoder.encode(body, "UTF-8"))
+ // Long idle timeout for the request.
+ .idleTimeout(10 * idleTimeout, TimeUnit.MILLISECONDS)
+ .send();
+
+ assertEquals(HttpStatus.OK_200, response.getStatus());
+ String content = response.getContentAsString();
+ assertEquals(body, content);
+ }
+ finally
+ {
+ httpClient.stop();
+ }
+ }
+
+ @Test
public void testProxyDown() throws Exception
{
startSSLServer(new ServerHandler());

Back to the top