From f60bbf3b11548977f5bdcc8a11b61f2103682053 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Oct 2022 17:17:33 +1100 Subject: [PATCH 1/4] expand jetty properties when generating dry-run command line Signed-off-by: Lachlan Roberts --- .../src/main/java/org/eclipse/jetty/start/StartArgs.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index ca98f7f67d3..df6c8bc2921 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java @@ -817,7 +817,7 @@ public class StartArgs { for (Prop p : properties) { - cmd.addRawArg(CommandLineBuilder.quote(p.key) + "=" + CommandLineBuilder.quote(p.value)); + cmd.addRawArg(CommandLineBuilder.quote(p.key) + "=" + CommandLineBuilder.quote(properties.expand(p.value))); } } else if (properties.size() > 0) From 7e85c2a54c87b5555732202f1f9389623c5b64e1 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Oct 2022 17:43:02 +1100 Subject: [PATCH 2/4] add distribution test to reproduce issue with dry-run Signed-off-by: Lachlan Roberts --- .../tests/distribution/DistributionTests.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index 92be9255198..470685a7439 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -25,6 +25,7 @@ import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.util.Arrays; import java.util.List; +import java.util.Queue; import java.util.UUID; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -64,6 +65,7 @@ import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -1229,4 +1231,32 @@ public class DistributionTests extends AbstractJettyHomeTest openIdProvider.stop(); } } + + @Test + public void testDryRunProperties() throws Exception + { + Path jettyBase = newTestJettyBaseDirectory(); + String jettyVersion = System.getProperty("jettyVersion"); + JettyHomeTester distribution = JettyHomeTester.Builder.newInstance() + .jettyVersion(jettyVersion) + .jettyBase(jettyBase) + .mavenLocalRepository(System.getProperty("mavenRepoPath")) + .build(); + + String[] args1 = {"--add-to-start=server,logging-jetty"}; + try (JettyHomeTester.Run run1 = distribution.start(args1)) + { + assertTrue(run1.awaitFor(10, TimeUnit.SECONDS)); + assertEquals(0, run1.getExitValue()); + + String[] args2 = {"--dry-run"}; + try (JettyHomeTester.Run run2 = distribution.start(args2)) + { + run2.awaitFor(5, TimeUnit.SECONDS); + Queue logs = run2.getLogs(); + assertThat(logs.size(), equalTo(1)); + assertThat(logs.poll(), not(containsString("${jetty.home.uri}"))); + } + } + } } From 58f0e0744b818f6b1b2ab449ffbb247b25ac348b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 30 Sep 2022 16:38:02 +0200 Subject: [PATCH 3/4] Fixes #8584 - HttpRequest.send() never returns Fixed handling of the idle timeout in case the SOCKS proxy does not reply to the SOCKS bytes. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/Socks4Proxy.java | 12 ++- .../eclipse/jetty/client/Socks4ProxyTest.java | 93 +++++++++++++++++-- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java b/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java index 529715ffde4..212653c6061 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java @@ -19,6 +19,7 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -140,12 +141,21 @@ public class Socks4Proxy extends ProxyConfiguration.Proxy @Override public void failed(Throwable x) { - close(); + if (LOG.isDebugEnabled()) + LOG.debug("SOCKS4 failure", x); + getEndPoint().close(x); @SuppressWarnings("unchecked") Promise promise = (Promise)context.get(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY); promise.failed(x); } + @Override + public boolean onIdleExpired() + { + failed(new TimeoutException("Idle timeout expired")); + return false; + } + @Override public void onFillable() { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java index 3b5b70fbbae..97770100b26 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/Socks4ProxyTest.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.client; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.InetSocketAddress; @@ -21,11 +22,15 @@ import java.nio.channels.ServerSocketChannel; import java.nio.channels.SocketChannel; import java.nio.charset.StandardCharsets; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLSocket; +import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; +import org.eclipse.jetty.client.util.FutureResponseListener; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -34,19 +39,22 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class Socks4ProxyTest { - private ServerSocketChannel server; + private ServerSocketChannel proxy; private HttpClient client; @BeforeEach public void prepare() throws Exception { - server = ServerSocketChannel.open(); - server.bind(new InetSocketAddress("localhost", 0)); + proxy = ServerSocketChannel.open(); + proxy.bind(new InetSocketAddress("localhost", 0)); ClientConnector connector = new ClientConnector(); QueuedThreadPool clientThreads = new QueuedThreadPool(); @@ -62,13 +70,13 @@ public class Socks4ProxyTest public void dispose() throws Exception { client.stop(); - server.close(); + proxy.close(); } @Test public void testSocks4Proxy() throws Exception { - int proxyPort = server.socket().getLocalPort(); + int proxyPort = proxy.socket().getLocalPort(); client.getProxyConfiguration().getProxies().add(new Socks4Proxy("localhost", proxyPort)); CountDownLatch latch = new CountDownLatch(1); @@ -91,7 +99,7 @@ public class Socks4ProxyTest latch.countDown(); }); - try (SocketChannel channel = server.accept()) + try (SocketChannel channel = proxy.accept()) { int socks4MessageLength = 9; ByteBuffer buffer = ByteBuffer.allocate(socks4MessageLength); @@ -130,7 +138,7 @@ public class Socks4ProxyTest @Test public void testSocks4ProxyWithSplitResponse() throws Exception { - int proxyPort = server.socket().getLocalPort(); + int proxyPort = proxy.socket().getLocalPort(); client.getProxyConfiguration().getProxies().add(new Socks4Proxy("localhost", proxyPort)); CountDownLatch latch = new CountDownLatch(1); @@ -150,7 +158,7 @@ public class Socks4ProxyTest result.getFailure().printStackTrace(); }); - try (SocketChannel channel = server.accept()) + try (SocketChannel channel = proxy.accept()) { int socks4MessageLength = 9; ByteBuffer buffer = ByteBuffer.allocate(socks4MessageLength); @@ -189,7 +197,7 @@ public class Socks4ProxyTest public void testSocks4ProxyWithTLSServer() throws Exception { String proxyHost = "localhost"; - int proxyPort = server.socket().getLocalPort(); + int proxyPort = proxy.socket().getLocalPort(); String serverHost = "127.0.0.13"; // Server host different from proxy host. int serverPort = proxyPort + 1; // Any port will do. @@ -221,7 +229,7 @@ public class Socks4ProxyTest result.getFailure().printStackTrace(); }); - try (SocketChannel channel = server.accept()) + try (SocketChannel channel = proxy.accept()) { int socks4MessageLength = 9; ByteBuffer buffer = ByteBuffer.allocate(socks4MessageLength); @@ -269,4 +277,69 @@ public class Socks4ProxyTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } } + + @Test + public void testRequestTimeoutWhenSocksProxyDoesNotRespond() throws Exception + { + String proxyHost = "localhost"; + int proxyPort = proxy.socket().getLocalPort(); + client.getProxyConfiguration().getProxies().add(new Socks4Proxy(proxyHost, proxyPort)); + + long timeout = 1000; + Request request = client.newRequest("localhost", proxyPort + 1) + .timeout(timeout, TimeUnit.MILLISECONDS); + FutureResponseListener listener = new FutureResponseListener(request); + request.send(listener); + + try (SocketChannel ignored = proxy.accept()) + { + // Accept the connection, but do not reply and don't close. + + ExecutionException x = assertThrows(ExecutionException.class, () -> listener.get(2 * timeout, TimeUnit.MILLISECONDS)); + assertThat(x.getCause(), instanceOf(TimeoutException.class)); + } + } + + @Test + public void testIdleTimeoutWhenSocksProxyDoesNotRespond() throws Exception + { + String proxyHost = "localhost"; + int proxyPort = proxy.socket().getLocalPort(); + client.getProxyConfiguration().getProxies().add(new Socks4Proxy(proxyHost, proxyPort)); + long idleTimeout = 1000; + client.setIdleTimeout(idleTimeout); + + Request request = client.newRequest("localhost", proxyPort + 1); + FutureResponseListener listener = new FutureResponseListener(request); + request.send(listener); + + try (SocketChannel ignored = proxy.accept()) + { + // Accept the connection, but do not reply and don't close. + + ExecutionException x = assertThrows(ExecutionException.class, () -> listener.get(2 * idleTimeout, TimeUnit.MILLISECONDS)); + assertThat(x.getCause(), instanceOf(TimeoutException.class)); + } + } + + @Test + public void testSocksProxyClosesConnectionImmediately() throws Exception + { + String proxyHost = "localhost"; + int proxyPort = proxy.socket().getLocalPort(); + client.getProxyConfiguration().getProxies().add(new Socks4Proxy(proxyHost, proxyPort)); + + Request request = client.newRequest("localhost", proxyPort + 1); + FutureResponseListener listener = new FutureResponseListener(request); + request.send(listener); + + try (SocketChannel channel = proxy.accept()) + { + // Immediately close the connection. + channel.close(); + + ExecutionException x = assertThrows(ExecutionException.class, () -> listener.get(5, TimeUnit.SECONDS)); + assertThat(x.getCause(), instanceOf(IOException.class)); + } + } } From ecea0e6d0aeb9b6f6d9eabce8a0d788ac4fc1527 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 5 Oct 2022 11:46:00 +0200 Subject: [PATCH 4/4] =?UTF-8?q?Fixes=20#7993=20-=20HttpClient=20idleTimeou?= =?UTF-8?q?t=20configuration=20being=20ignored/over=E2=80=A6=20(#8672)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixes #7993 - HttpClient idleTimeout configuration being ignored/overridden The problem was that the timeout scheduling was not happening, because for TunnelRequest the timeouts were set in normalizeRequest(), which runs after the scheduling. Now a call to request.sent() is made also after normalizeRequest() so that the timeouts is scheduled (if it was not scheduled before). Signed-off-by: Simone Bordet --- .../eclipse/jetty/client/HttpConnection.java | 1 + .../eclipse/jetty/client/HttpDestination.java | 1 + .../org/eclipse/jetty/client/HttpProxy.java | 9 +- .../org/eclipse/jetty/client/HttpRequest.java | 10 +- .../client/http/HttpChannelOverHTTP.java | 5 +- .../client/http/HttpConnectionOverHTTP.java | 29 ++-- .../client/http/HttpReceiverOverHTTP.java | 1 + .../org/eclipse/jetty/io/IdleTimeout.java | 3 + .../org/eclipse/jetty/io/SelectorManager.java | 3 +- .../proxy/ForwardProxyTLSServerTest.java | 135 +++++++++++++++++- .../test/resources/jetty-logging.properties | 1 - 11 files changed, 176 insertions(+), 22 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java index 7120db1c159..adb16b46745 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java @@ -106,6 +106,7 @@ public abstract class HttpConnection implements IConnection, Attachable SendFailure result; if (channel.associate(exchange)) { + request.sent(); requestTimeouts.schedule(channel); channel.send(); result = null; diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java index 669346496a0..2fc7766e9f8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java @@ -311,6 +311,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest { if (enqueue(exchanges, exchange)) { + request.sent(); requestTimeouts.schedule(exchange); if (!client.isRunning() && exchanges.remove(exchange)) { 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 14ac751d08d..9d32b20ec82 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 @@ -19,6 +19,7 @@ import java.net.URI; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.TimeUnit; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Destination; @@ -195,10 +196,14 @@ 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 = new TunnelRequest(httpClient, proxyAddress) .method(HttpMethod.CONNECT) .path(target) - .headers(headers -> headers.put(HttpHeader.HOST, target)); + .headers(headers -> headers.put(HttpHeader.HOST, target)) + // Use the connect timeout as a total timeout, + // since this request is to "connect" to the server. + .timeout(connectTimeout, TimeUnit.MILLISECONDS); ProxyConfiguration.Proxy proxy = destination.getProxy(); if (proxy.isSecure()) connect.scheme(HttpScheme.HTTPS.asString()); @@ -234,7 +239,7 @@ public class HttpProxy extends ProxyConfiguration.Proxy private void tunnelFailed(EndPoint endPoint, Throwable failure) { - endPoint.close(); + endPoint.close(failure); promise.failed(failure); } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index b8f2291114d..28a259ab4d1 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -816,15 +816,17 @@ public class HttpRequest implements Request { if (listener != null) responseListeners.add(listener); - sent(); sender.accept(this, responseListeners); } void sent() { - long timeout = getTimeout(); - if (timeout > 0) - timeoutNanoTime = NanoTime.now() + TimeUnit.MILLISECONDS.toNanos(timeout); + if (timeoutNanoTime == Long.MAX_VALUE) + { + long timeout = getTimeout(); + if (timeout > 0) + timeoutNanoTime = NanoTime.now() + TimeUnit.MILLISECONDS.toNanos(timeout); + } } /** 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 2f8dd692cf6..9ee061ce2f6 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 @@ -131,7 +131,10 @@ public class HttpChannelOverHTTP extends HttpChannel { if (LOG.isDebugEnabled()) LOG.debug("Closing, reason: {} - {}", closeReason, connection); - connection.close(); + if (result.isFailed()) + connection.close(result.getFailure()); + else + connection.close(); } else { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java index c875f4989df..d6df85d30e2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java @@ -201,6 +201,16 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne return receiver.onUpgradeFrom(); } + void onResponseHeaders(HttpExchange exchange) + { + HttpRequest request = exchange.getRequest(); + if (request instanceof HttpProxy.TunnelRequest) + { + // Restore idle timeout + getEndPoint().setIdleTimeout(idleTimeout); + } + } + public void release() { // Restore idle timeout @@ -208,6 +218,11 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne getHttpDestination().release(this); } + public void remove() + { + getHttpDestination().remove(this); + } + @Override public void close() { @@ -241,14 +256,7 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne { if (!closed.get()) return false; - if (sweeps.incrementAndGet() < 4) - return false; - return true; - } - - public void remove() - { - getHttpDestination().remove(this); + return sweeps.incrementAndGet() > 3; } @Override @@ -300,9 +308,8 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne if (request instanceof HttpProxy.TunnelRequest) { - long connectTimeout = getHttpClient().getConnectTimeout(); - request.timeout(connectTimeout, TimeUnit.MILLISECONDS) - .idleTimeout(2 * connectTimeout, TimeUnit.MILLISECONDS); + // Override the idle timeout in case it is shorter than the connect timeout. + request.idleTimeout(2 * getHttpClient().getConnectTimeout(), TimeUnit.MILLISECONDS); } HttpConversation conversation = request.getConversation(); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java index 5440a0401ad..f380fed317e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java @@ -327,6 +327,7 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res // Store the EndPoint is case of upgrades, tunnels, etc. exchange.getRequest().getConversation().setAttribute(EndPoint.class.getName(), getHttpConnection().getEndPoint()); + getHttpConnection().onResponseHeaders(exchange); return !responseHeaders(exchange); } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java b/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java index bfb18080375..24040405717 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/IdleTimeout.java @@ -80,6 +80,9 @@ public abstract class IdleTimeout long old = _idleTimeout; _idleTimeout = idleTimeout; + if (LOG.isDebugEnabled()) + LOG.debug("Setting idle timeout {} -> {} on {}", old, idleTimeout, this); + // Do we have an old timeout if (old > 0) { diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 34f555e4965..351c3d43adb 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -166,7 +166,8 @@ public abstract class SelectorManager extends ContainerLifeCycle implements Dump public void connect(SelectableChannel channel, Object attachment) { ManagedSelector set = chooseSelector(); - set.submit(set.new Connect(channel, attachment)); + if (set != null) + set.submit(set.new Connect(channel, attachment)); } /** diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java index e1835d4884a..9c190a99db0 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ForwardProxyTLSServerTest.java @@ -24,6 +24,7 @@ import java.security.Principal; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; import javax.net.ssl.KeyManager; @@ -40,6 +41,7 @@ import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Destination; +import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.util.BasicAuthentication; import org.eclipse.jetty.client.util.FutureResponseListener; @@ -61,7 +63,6 @@ import org.eclipse.jetty.toolchain.test.Net; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Disabled; @@ -71,6 +72,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -413,7 +415,7 @@ public class ForwardProxyTLSServerTest .timeout(5, TimeUnit.SECONDS) .send(); }); - assertThat(x.getCause(), Matchers.instanceOf(ConnectException.class)); + assertThat(x.getCause(), instanceOf(ConnectException.class)); httpClient.stop(); } @@ -829,6 +831,135 @@ public class ForwardProxyTLSServerTest } } + @ParameterizedTest + @MethodSource("proxyTLS") + public void testServerLongProcessing(SslContextFactory.Server proxyTLS) throws Exception + { + long timeout = 500; + startTLSServer(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + { + sleep(3 * timeout); + baseRequest.setHandled(true); + } + }); + startProxy(proxyTLS); + HttpClient httpClient = newHttpClient(); + httpClient.getProxyConfiguration().getProxies().add(newHttpProxy()); + httpClient.setConnectTimeout(timeout); + httpClient.setIdleTimeout(4 * timeout); + httpClient.start(); + + try + { + // The idle timeout is larger than the server processing time, request should succeed. + ContentResponse response = httpClient.newRequest("localhost", serverConnector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + } + finally + { + httpClient.stop(); + } + } + + @ParameterizedTest + @MethodSource("proxyTLS") + public void testServerLongProcessingWithRequestIdleTimeout(SslContextFactory.Server proxyTLS) throws Exception + { + long timeout = 500; + startTLSServer(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + { + sleep(3 * timeout); + baseRequest.setHandled(true); + } + }); + startProxy(proxyTLS); + HttpClient httpClient = newHttpClient(); + httpClient.getProxyConfiguration().getProxies().add(newHttpProxy()); + httpClient.setConnectTimeout(timeout); + // Short idle timeout for HttpClient. + httpClient.setIdleTimeout(timeout); + httpClient.start(); + + try + { + // The idle timeout is larger than the server processing time, request should succeed. + ContentResponse response = httpClient.newRequest("localhost", serverConnector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + // Long idle timeout for the request, should override that of the client. + .idleTimeout(4 * timeout, TimeUnit.MILLISECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + } + finally + { + httpClient.stop(); + } + } + + @ParameterizedTest + @MethodSource("proxyTLS") + public void testProxyLongProcessing(SslContextFactory.Server proxyTLS) throws Exception + { + long timeout = 500; + startTLSServer(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + { + baseRequest.setHandled(true); + } + }); + startProxy(proxyTLS, new ConnectHandler() + { + @Override + protected void handleConnect(Request baseRequest, HttpServletRequest request, HttpServletResponse response, String serverAddress) + { + sleep(3 * timeout); + super.handleConnect(baseRequest, request, response, serverAddress); + } + }); + HttpClient httpClient = newHttpClient(); + httpClient.getProxyConfiguration().getProxies().add(newHttpProxy()); + httpClient.setConnectTimeout(timeout); + httpClient.setIdleTimeout(10 * timeout); + httpClient.start(); + + try + { + // Connecting to the server through the proxy involves a CONNECT + 200 + // so if the proxy delays the response, the client request interprets + // it as a "connect" timeout (rather than an idle timeout). + AtomicReference resultRef = new AtomicReference<>(); + CountDownLatch latch = new CountDownLatch(1); + httpClient.newRequest("localhost", serverConnector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .send(result -> + { + resultRef.set(result); + latch.countDown(); + }); + + assertTrue(latch.await(2 * timeout, TimeUnit.MILLISECONDS)); + Result result = resultRef.get(); + assertTrue(result.isFailed()); + assertThat(result.getFailure(), instanceOf(TimeoutException.class)); + } + finally + { + httpClient.stop(); + } + } + @Test @Tag("external") @Disabled diff --git a/jetty-proxy/src/test/resources/jetty-logging.properties b/jetty-proxy/src/test/resources/jetty-logging.properties index 7e7e5ce6384..d3a9e3c5f43 100644 --- a/jetty-proxy/src/test/resources/jetty-logging.properties +++ b/jetty-proxy/src/test/resources/jetty-logging.properties @@ -1,4 +1,3 @@ -# Jetty Logging using jetty-slf4j-impl #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.client.LEVEL=DEBUG #org.eclipse.jetty.proxy.LEVEL=DEBUG