From 902603fc9e164929981dac882e0458e2caa1aa4e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 29 Nov 2019 12:05:30 +0100 Subject: [PATCH 01/16] Fixes #4374 - Jetty client: Response.AsyncContentListener.onContent is not called. Now the various content listeners inherit from each other, like it should have been from the beginning. This also allowed to remove code duplication due to the default implementation of the methods in various places. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpRequest.java | 25 ++------ .../eclipse/jetty/client/api/Response.java | 60 +++++++++---------- .../eclipse/jetty/client/HttpClientTest.java | 53 ++++++++++++++++ 3 files changed, 86 insertions(+), 52 deletions(-) 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 77027ce83060..bb0c22b5fe3d 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 @@ -502,21 +502,12 @@ public void onHeaders(Response response) @Override public Request onResponseContent(final Response.ContentListener listener) { - this.responseListeners.add(new Response.DemandedContentListener() + this.responseListeners.add(new Response.ContentListener() { @Override - public void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) + public void onContent(Response response, ByteBuffer content) { - try - { - listener.onContent(response, content); - callback.succeeded(); - demand.accept(1); - } - catch (Throwable x) - { - callback.failed(x); - } + listener.onContent(response, content); } }); return this; @@ -525,16 +516,12 @@ public void onContent(Response response, LongConsumer demand, ByteBuffer content @Override public Request onResponseContentAsync(final Response.AsyncContentListener listener) { - this.responseListeners.add(new Response.DemandedContentListener() + this.responseListeners.add(new Response.AsyncContentListener() { @Override - public void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) + public void onContent(Response response, ByteBuffer content, Callback callback) { - listener.onContent(response, content, Callback.from(() -> - { - callback.succeeded(); - demand.accept(1); - }, callback::failed)); + listener.onContent(response, content, callback); } }); return this; diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java index b16cd7786aff..74a7cbebed34 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java @@ -138,7 +138,7 @@ interface HeadersListener extends ResponseListener * * @see AsyncContentListener */ - interface ContentListener extends ResponseListener + interface ContentListener extends AsyncContentListener { /** * Callback method invoked when the response content has been received, parsed and there is demand. @@ -149,6 +149,20 @@ interface ContentListener extends ResponseListener * @param content the content bytes received */ void onContent(Response response, ByteBuffer content); + + @Override + default void onContent(Response response, ByteBuffer content, Callback callback) + { + try + { + onContent(response, content); + callback.succeeded(); + } + catch (Throwable x) + { + callback.failed(x); + } + } } /** @@ -156,7 +170,7 @@ interface ContentListener extends ResponseListener * * @see DemandedContentListener */ - interface AsyncContentListener extends ResponseListener + interface AsyncContentListener extends DemandedContentListener { /** * Callback method invoked when the response content has been received, parsed and there is demand. @@ -168,6 +182,16 @@ interface AsyncContentListener extends ResponseListener * @param callback the callback to call when the content is consumed and to demand more content */ void onContent(Response response, ByteBuffer content, Callback callback); + + @Override + default void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) + { + onContent(response, content, Callback.from(() -> + { + callback.succeeded(); + demand.accept(1); + }, callback::failed)); + } } /** @@ -257,7 +281,7 @@ interface CompleteListener extends ResponseListener /** * Listener for all response events. */ - interface Listener extends BeginListener, HeaderListener, HeadersListener, ContentListener, AsyncContentListener, DemandedContentListener, SuccessListener, FailureListener, CompleteListener + interface Listener extends BeginListener, HeaderListener, HeadersListener, ContentListener, SuccessListener, FailureListener, CompleteListener { @Override public default void onBegin(Response response) @@ -275,41 +299,11 @@ public default void onHeaders(Response response) { } - @Override - default void onBeforeContent(Response response, LongConsumer demand) - { - demand.accept(1); - } - @Override public default void onContent(Response response, ByteBuffer content) { } - @Override - public default void onContent(Response response, ByteBuffer content, Callback callback) - { - try - { - onContent(response, content); - callback.succeeded(); - } - catch (Throwable x) - { - callback.failed(x); - } - } - - @Override - public default void onContent(Response response, LongConsumer demand, ByteBuffer content, Callback callback) - { - onContent(response, content, Callback.from(() -> - { - callback.succeeded(); - demand.accept(1); - }, callback::failed)); - } - @Override public default void onSuccess(Response response) { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index a938eeb7ab81..0de9ac2722c2 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.client; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -84,6 +85,7 @@ import org.eclipse.jetty.toolchain.test.Net; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.IO; @@ -1788,6 +1790,57 @@ public void test204WithContent(Scenario scenario) throws Exception } } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testContentListenerAsCompleteListener(Scenario scenario) throws Exception + { + byte[] bytes = new byte[1024]; + new Random().nextBytes(bytes); + start(scenario, new AbstractHandler() + { + @Override + public void handle(String target, org.eclipse.jetty.server.Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + ServletOutputStream output = response.getOutputStream(); + output.write(bytes); + } + }); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + CountDownLatch latch = new CountDownLatch(1); + class L implements Response.ContentListener, Response.CompleteListener + { + @Override + public void onContent(Response response, ByteBuffer content) + { + try + { + BufferUtil.writeTo(content, baos); + } + catch (IOException x) + { + baos.reset(); + x.printStackTrace(); + } + } + + @Override + public void onComplete(Result result) + { + if (result.isSucceeded()) + latch.countDown(); + } + } + + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .send(new L()); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + assertArrayEquals(bytes, baos.toByteArray()); + } + private void assertCopyRequest(Request original) { Request copy = client.copyRequest((HttpRequest)original, original.getURI()); From 9628ea3bc158784bb2d9b040e264d1a45fc4b7df Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Dec 2019 11:36:53 +0100 Subject: [PATCH 02/16] Fixes #3512 - File descriptor is not released after zip file uploaded via jetty-client. In case of multiple parts only the last iterator was closed. Now, every part's iterator is closed. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpContent.java | 12 +-- .../client/util/MultiPartContentProvider.java | 17 ++-- .../util/MultiPartContentProviderTest.java | 88 +++++++++++++++++++ 3 files changed, 103 insertions(+), 14 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java index 17c8d3ab1bd9..ce1e0a82c25c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpContent.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -218,15 +219,8 @@ public void failed(Throwable x) @Override public void close() { - try - { - if (iterator instanceof Closeable) - ((Closeable)iterator).close(); - } - catch (Throwable x) - { - LOG.ignore(x); - } + if (iterator instanceof Closeable) + IO.close((Closeable)iterator); } @Override diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java index e6441371d578..30d817dfc811 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/MultiPartContentProvider.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -345,10 +346,16 @@ public ByteBuffer next() if (iterator.hasNext()) return iterator.next(); ++index; - if (index == parts.size()) - state = State.LAST_BOUNDARY; - else + if (index < parts.size()) + { state = State.MIDDLE_BOUNDARY; + if (iterator instanceof Closeable) + IO.close((Closeable)iterator); + } + else + { + state = State.LAST_BOUNDARY; + } break; } case MIDDLE_BOUNDARY: @@ -380,14 +387,14 @@ public Object getLock() @Override public void succeeded() { - if (iterator instanceof Callback) + if (state == State.CONTENT && iterator instanceof Callback) ((Callback)iterator).succeeded(); } @Override public void failed(Throwable x) { - if (iterator instanceof Callback) + if (state == State.CONTENT && iterator instanceof Callback) ((Callback)iterator).failed(x); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java index e1d855e3e2bf..ec90c842c317 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java @@ -20,6 +20,7 @@ import java.io.BufferedWriter; import java.io.ByteArrayInputStream; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.OutputStream; @@ -31,10 +32,12 @@ import java.nio.file.StandardOpenOption; import java.util.ArrayList; import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.MultipartConfigElement; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -435,6 +438,46 @@ protected void handle(HttpServletRequest request, HttpServletResponse response) assertTrue(responseLatch.await(5, TimeUnit.SECONDS)); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testEachPartIsClosed(Scenario scenario) throws Exception + { + String name1 = "field1"; + String value1 = "value1"; + String name2 = "field2"; + String value2 = "value2"; + start(scenario, new AbstractMultiPartHandler() + { + @Override + protected void handle(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + Collection parts = request.getParts(); + assertEquals(2, parts.size()); + Iterator iterator = parts.iterator(); + Part part1 = iterator.next(); + assertEquals(name1, part1.getName()); + assertEquals(value1, IO.toString(part1.getInputStream())); + Part part2 = iterator.next(); + assertEquals(name2, part2.getName()); + assertEquals(value2, IO.toString(part2.getInputStream())); + } + }); + + AtomicInteger closeCount = new AtomicInteger(); + MultiPartContentProvider multiPart = new MultiPartContentProvider(); + multiPart.addFieldPart(name1, new CloseableStringContentProvider(value1, closeCount::incrementAndGet), null); + multiPart.addFieldPart(name2, new CloseableStringContentProvider(value2, closeCount::incrementAndGet), null); + multiPart.close(); + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .method(HttpMethod.POST) + .content(multiPart) + .send(); + + assertEquals(200, response.getStatus()); + assertEquals(2, closeCount.get()); + } + private abstract static class AbstractMultiPartHandler extends AbstractHandler { @Override @@ -448,4 +491,49 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques protected abstract void handle(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException; } + + private static class CloseableStringContentProvider extends StringContentProvider + { + private final Runnable closeFn; + + private CloseableStringContentProvider(String content, Runnable closeFn) + { + super(content); + this.closeFn = closeFn; + } + + @Override + public Iterator iterator() + { + return new CloseableIterator<>(super.iterator()); + } + + private class CloseableIterator implements Iterator, Closeable + { + private final Iterator iterator; + + public CloseableIterator(Iterator iterator) + { + this.iterator = iterator; + } + + @Override + public boolean hasNext() + { + return iterator.hasNext(); + } + + @Override + public T next() + { + return iterator.next(); + } + + @Override + public void close() + { + closeFn.run(); + } + } + } } From 839846d9a6e56b3557bb2aa3593d8e4591760ffd Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Dec 2019 13:01:28 +0100 Subject: [PATCH 03/16] Fixes #4392 - Suppress logging of QuietException in HttpChannelState.asyncError(). Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/server/HttpChannelState.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index bf3027591200..74e72098f64f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.QuietException; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandler.Context; import org.eclipse.jetty.server.handler.ErrorHandler; @@ -406,8 +407,6 @@ public Action handling() */ protected Action unhandle() { - boolean readInterested = false; - synchronized (this) { if (LOG.isDebugEnabled()) @@ -736,8 +735,10 @@ public void asyncError(Throwable failure) } else { - LOG.warn(failure.toString()); - LOG.debug(failure); + if (!(failure instanceof QuietException)) + LOG.warn(failure.toString()); + if (LOG.isDebugEnabled()) + LOG.debug(failure); } } @@ -1340,7 +1341,7 @@ public boolean onReadReady() * but that a handling thread may need to produce (fill/parse) * it. Typically called by the async read success callback. * - * @return true if more content may be available + * @return {@code true} if more content may be available */ public boolean onReadPossible() { @@ -1372,7 +1373,7 @@ public boolean onReadPossible() * Called to signal that a read has read -1. * Will wake if the read was called while in ASYNC_WAIT state * - * @return true if woken + * @return {@code true} if woken */ public boolean onReadEof() { From 2ef02da1bd020c420beb6418b79e08d169e6e74c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 3 Dec 2019 21:40:55 +0100 Subject: [PATCH 04/16] Fixes #4366 - HTTP client uses SOCKS4 proxy hostname for SSL hostname verification. Now setting correctly the host and port to the server destination _after_ the SOCKS tunnel is established, similarly to what is done for the HTTP CONNECT tunnel. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/Socks4Proxy.java | 3 + .../eclipse/jetty/client/Socks4ProxyTest.java | 105 +++++++++++++++++- 2 files changed, 103 insertions(+), 5 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 7150247e8c91..4700591ceab9 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 @@ -30,6 +30,7 @@ import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.io.ssl.SslClientConnectionFactory; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; @@ -195,6 +196,8 @@ private void tunnel() try { HttpDestination destination = (HttpDestination)context.get(HttpClientTransport.HTTP_DESTINATION_CONTEXT_KEY); + context.put(SslClientConnectionFactory.SSL_PEER_HOST_CONTEXT_KEY, destination.getHost()); + context.put(SslClientConnectionFactory.SSL_PEER_PORT_CONTEXT_KEY, destination.getPort()); ClientConnectionFactory connectionFactory = this.connectionFactory; if (destination.isSecure()) connectionFactory = destination.newSslClientConnectionFactory(null, connectionFactory); 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 df56547514cf..5b7eea37db6a 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 @@ -18,6 +18,8 @@ package org.eclipse.jetty.client; +import java.io.InputStream; +import java.io.OutputStream; import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.nio.channels.ServerSocketChannel; @@ -25,7 +27,12 @@ import java.nio.charset.StandardCharsets; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; +import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -44,7 +51,10 @@ public void prepare() throws Exception server = ServerSocketChannel.open(); server.bind(new InetSocketAddress("localhost", 0)); - client = new HttpClient(); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + client = new HttpClient(new SslContextFactory.Client()); + client.setExecutor(clientThreads); client.start(); } @@ -61,7 +71,7 @@ public void testSocks4Proxy() throws Exception int proxyPort = server.socket().getLocalPort(); client.getProxyConfiguration().getProxies().add(new Socks4Proxy("localhost", proxyPort)); - final CountDownLatch latch = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(1); byte ip1 = 127; byte ip2 = 0; @@ -111,7 +121,7 @@ public void testSocks4Proxy() throws Exception "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n"; - channel.write(ByteBuffer.wrap(response.getBytes("UTF-8"))); + channel.write(ByteBuffer.wrap(response.getBytes(StandardCharsets.UTF_8))); assertTrue(latch.await(5, TimeUnit.SECONDS)); } @@ -123,7 +133,7 @@ public void testSocks4ProxyWithSplitResponse() throws Exception int proxyPort = server.socket().getLocalPort(); client.getProxyConfiguration().getProxies().add(new Socks4Proxy("localhost", proxyPort)); - final CountDownLatch latch = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(1); String serverHost = "127.0.0.13"; // Test expects an IP address. int serverPort = proxyPort + 1; // Any port will do @@ -169,7 +179,92 @@ public void testSocks4ProxyWithSplitResponse() throws Exception "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n"; - channel.write(ByteBuffer.wrap(response.getBytes("UTF-8"))); + channel.write(ByteBuffer.wrap(response.getBytes(StandardCharsets.UTF_8))); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + } + + @Test + public void testSocks4ProxyWithTLSServer() throws Exception + { + String proxyHost = "localhost"; + int proxyPort = server.socket().getLocalPort(); + + String serverHost = "127.0.0.13"; // Server host different from proxy host. + int serverPort = proxyPort + 1; // Any port will do. + + SslContextFactory clientTLS = client.getSslContextFactory(); + clientTLS.reload(ssl -> + { + // The client keystore contains the trustedCertEntry for the + // self-signed server certificate, so it acts as a truststore. + ssl.setTrustStorePath("src/test/resources/client_keystore.jks"); + ssl.setTrustStorePassword("storepwd"); + // Disable TLS hostname verification, but + // enable application hostname verification. + ssl.setEndpointIdentificationAlgorithm(null); + // The hostname must be that of the server, not of the proxy. + ssl.setHostnameVerifier((hostname, session) -> serverHost.equals(hostname)); + }); + client.getProxyConfiguration().getProxies().add(new Socks4Proxy(proxyHost, proxyPort)); + + CountDownLatch latch = new CountDownLatch(1); + client.newRequest(serverHost, serverPort) + .scheme(HttpScheme.HTTPS.asString()) + .path("/path") + .send(result -> + { + if (result.isSucceeded()) + latch.countDown(); + else + result.getFailure().printStackTrace(); + }); + + try (SocketChannel channel = server.accept()) + { + int socks4MessageLength = 9; + ByteBuffer buffer = ByteBuffer.allocate(socks4MessageLength); + int read = channel.read(buffer); + assertEquals(socks4MessageLength, read); + + // Socks4 response. + channel.write(ByteBuffer.wrap(new byte[]{0, 0x5A, 0, 0, 0, 0, 0, 0})); + + // Wrap the socket with TLS. + SslContextFactory.Server serverTLS = new SslContextFactory.Server(); + serverTLS.setKeyStorePath("src/test/resources/keystore.jks"); + serverTLS.setKeyStorePassword("storepwd"); + serverTLS.start(); + SSLContext sslContext = serverTLS.getSslContext(); + SSLSocket sslSocket = (SSLSocket)sslContext.getSocketFactory().createSocket(channel.socket(), serverHost, serverPort, false); + sslSocket.setUseClientMode(false); + + // Read the request. + int crlfs = 0; + InputStream input = sslSocket.getInputStream(); + while (true) + { + read = input.read(); + if (read < 0) + break; + if (read == '\r' || read == '\n') + ++crlfs; + else + crlfs = 0; + if (crlfs == 4) + break; + } + + // Send the response. + String response = + "HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + "Connection: close\r\n" + + "\r\n"; + OutputStream output = sslSocket.getOutputStream(); + output.write(response.getBytes(StandardCharsets.UTF_8)); + output.flush(); assertTrue(latch.await(5, TimeUnit.SECONDS)); } From 53073ca25704ebd49c7133226053418a5124900f Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 6 Dec 2019 13:20:24 -0600 Subject: [PATCH 05/16] Issue #4385 - Reverting WARN log in favor of IllegalStateException + Plus fleshing out the testcases more for Base / Client / Server with and without certificates that will trigger SNI requirement and ISE. Signed-off-by: Joakim Erdfelt --- .../jetty/util/ssl/SslContextFactory.java | 24 +++-- .../org/eclipse/jetty/util/ssl/X509Test.java | 91 ++++++++++--------- 2 files changed, 61 insertions(+), 54 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index f141b846bec8..c1f356064306 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -1249,18 +1249,13 @@ protected KeyManager[] getKeyManagers(KeyStore keyStore) throws Exception // Is SNI needed to select a certificate? if (!_certWilds.isEmpty() || _certHosts.size() > 1 || (_certHosts.size() == 1 && _aliasX509.size() > 1)) { - if (this instanceof SslContextFactory.Server) + for (int idx = 0; idx < managers.length; idx++) { - for (int idx = 0; idx < managers.length; idx++) + if (managers[idx] instanceof X509ExtendedKeyManager) { - if (managers[idx] instanceof X509ExtendedKeyManager) - managers[idx] = newSniX509ExtendedKeyManager((X509ExtendedKeyManager)managers[idx]); + managers[idx] = newSniX509ExtendedKeyManager((X509ExtendedKeyManager)managers[idx]); } } - else - { - LOG.warn("Unable to support SNI on {} (expecting {})", this.getClass().getName(), SslContextFactory.Server.class.getName()); - } } } } @@ -1277,7 +1272,11 @@ protected KeyManager[] getKeyManagers(KeyStore keyStore) throws Exception @Deprecated protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager) { - throw new UnsupportedOperationException("X509ExtendedKeyManager only supported on " + SslContextFactory.Server.class.getName()); + throw new IllegalStateException(String.format( + "KeyStores with multiple certificates are not supported on the base class %s. (Use %s or %s instead)", + SslContextFactory.class.getName(), + Server.class.getName(), + Client.class.getName())); } protected TrustManager[] getTrustManagers(KeyStore trustStore, Collection crls) throws Exception @@ -2185,6 +2184,13 @@ protected void checkConfiguration() checkEndPointIdentificationAlgorithm(); super.checkConfiguration(); } + + @Override + protected X509ExtendedKeyManager newSniX509ExtendedKeyManager(X509ExtendedKeyManager keyManager) + { + // Client has no SNI functionality. + return keyManager; + } } @ManagedObject diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java index a893be2d9ebe..85ca35233a9f 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/ssl/X509Test.java @@ -20,8 +20,6 @@ import java.nio.file.Path; import java.security.cert.X509Certificate; -import javax.net.ssl.KeyManager; -import javax.net.ssl.X509ExtendedKeyManager; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.resource.PathResource; @@ -31,7 +29,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertThrows; public class X509Test @@ -133,66 +130,70 @@ public boolean[] getKeyUsage() assertThat("Normal X509", X509.isCertSign(bogusX509), is(false)); } - private X509ExtendedKeyManager getX509ExtendedKeyManager(SslContextFactory sslContextFactory) throws Exception + @Test + public void testBaseClass_WithSni() { - Resource keystoreResource = Resource.newSystemResource("keystore"); - Resource truststoreResource = Resource.newSystemResource("keystore"); - sslContextFactory.setKeyStoreResource(keystoreResource); - sslContextFactory.setTrustStoreResource(truststoreResource); - sslContextFactory.setKeyStorePassword("storepwd"); - sslContextFactory.setKeyManagerPassword("keypwd"); - sslContextFactory.setTrustStorePassword("storepwd"); - sslContextFactory.start(); - - KeyManager[] keyManagers = sslContextFactory.getKeyManagers(sslContextFactory.getKeyStore()); - X509ExtendedKeyManager x509ExtendedKeyManager = null; - - for (KeyManager keyManager : keyManagers) - { - if (keyManager instanceof X509ExtendedKeyManager) - { - x509ExtendedKeyManager = (X509ExtendedKeyManager)keyManager; - break; - } - } - assertThat("Found X509ExtendedKeyManager", x509ExtendedKeyManager, is(notNullValue())); - return x509ExtendedKeyManager; + SslContextFactory baseSsl = new SslContextFactory(); + Path keystorePath = MavenTestingUtils.getTestResourcePathFile("keystore_sni.p12"); + baseSsl.setKeyStoreResource(new PathResource(keystorePath)); + baseSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); + baseSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); + IllegalStateException ex = assertThrows(IllegalStateException.class, baseSsl::start); + assertThat("IllegalStateException.message", ex.getMessage(), containsString("KeyStores with multiple certificates are not supported on the base class")); } @Test - public void testSniX509ExtendedKeyManager_BaseClass() throws Exception + public void testServerClass_WithSni() throws Exception { - SslContextFactory baseSsl = new SslContextFactory(); - X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(baseSsl); - UnsupportedOperationException ex = assertThrows(UnsupportedOperationException.class, () -> baseSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager)); - assertThat("UnsupportedOperationException.message", ex.getMessage(), containsString("X509ExtendedKeyManager only supported on " + SslContextFactory.Server.class.getName())); + SslContextFactory serverSsl = new SslContextFactory.Server(); + Path keystorePath = MavenTestingUtils.getTestResourcePathFile("keystore_sni.p12"); + serverSsl.setKeyStoreResource(new PathResource(keystorePath)); + serverSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); + serverSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); + serverSsl.start(); } @Test - public void testSniX509ExtendedKeyManager_BaseClass_Start() throws Exception + public void testClientClass_WithSni() throws Exception { - SslContextFactory baseSsl = new SslContextFactory(); + SslContextFactory clientSsl = new SslContextFactory.Client(); Path keystorePath = MavenTestingUtils.getTestResourcePathFile("keystore_sni.p12"); - baseSsl.setKeyStoreResource(new PathResource(keystorePath)); - baseSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); - baseSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); - baseSsl.start(); // should not throw an exception + clientSsl.setKeyStoreResource(new PathResource(keystorePath)); + clientSsl.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4"); + clientSsl.setKeyManagerPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g"); + clientSsl.start(); } @Test - public void testSniX509ExtendedKeyManager_ClientClass() throws Exception + public void testBaseClass_WithoutSni() throws Exception { - SslContextFactory clientSsl = new SslContextFactory.Client(); - X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(clientSsl); - UnsupportedOperationException ex = assertThrows(UnsupportedOperationException.class, () -> clientSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager)); - assertThat("SNI X509 ExtendedKeyManager is unsupported in Client mode", ex.getMessage(), containsString("X509ExtendedKeyManager only supported on " + SslContextFactory.Server.class.getName())); + SslContextFactory baseSsl = new SslContextFactory(); + Resource keystoreResource = Resource.newSystemResource("keystore"); + baseSsl.setKeyStoreResource(keystoreResource); + baseSsl.setKeyStorePassword("storepwd"); + baseSsl.setKeyManagerPassword("keypwd"); + baseSsl.start(); } @Test - public void testSniX509ExtendedKeyManager_ServerClass() throws Exception + public void testServerClass_WithoutSni() throws Exception { SslContextFactory serverSsl = new SslContextFactory.Server(); - X509ExtendedKeyManager x509ExtendedKeyManager = getX509ExtendedKeyManager(serverSsl); - serverSsl.newSniX509ExtendedKeyManager(x509ExtendedKeyManager); + Resource keystoreResource = Resource.newSystemResource("keystore"); + serverSsl.setKeyStoreResource(keystoreResource); + serverSsl.setKeyStorePassword("storepwd"); + serverSsl.setKeyManagerPassword("keypwd"); + serverSsl.start(); + } + + @Test + public void testClientClass_WithoutSni() throws Exception + { + SslContextFactory clientSsl = new SslContextFactory.Client(); + Resource keystoreResource = Resource.newSystemResource("keystore"); + clientSsl.setKeyStoreResource(keystoreResource); + clientSsl.setKeyStorePassword("storepwd"); + clientSsl.setKeyManagerPassword("keypwd"); + clientSsl.start(); } } From 53eda03203706e5f29ba3a4881262a7cd63f7ebb Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 6 Dec 2019 13:24:30 -0600 Subject: [PATCH 06/16] Making exception message more clear Signed-off-by: Joakim Erdfelt --- .../src/main/java/org/eclipse/jetty/server/Request.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index f02ad81dbdab..0c0f5734e941 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -2306,7 +2306,7 @@ public Collection getParts() throws IOException, ServletException { String contentType = getContentType(); if (contentType == null || !MimeTypes.Type.MULTIPART_FORM_DATA.is(HttpFields.valueParameters(contentType, null))) - throw new ServletException("Content-Type != multipart/form-data"); + throw new ServletException("Unsupported Content-Type [" + contentType + "], expected [multipart/form-data]"); return getParts(null); } From 0e6a1ce76b1b8427bd05a210a10ed15796c150b4 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 10 Dec 2019 12:29:40 +1100 Subject: [PATCH 07/16] Issue #4402 Fix NPE in JettyRunWarExplodedMojo (#4406) Signed-off-by: Jan Bartel --- .../org/eclipse/jetty/maven/plugin/JettyRunWarExplodedMojo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarExplodedMojo.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarExplodedMojo.java index 65de9d95f5b1..fd124a9457f9 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarExplodedMojo.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunWarExplodedMojo.java @@ -88,7 +88,7 @@ public void configureScanner() throws MojoExecutionException { try { - scanner.addDirectory(webApp.getClasses().toPath()); + scanner.addDirectory(classes.toPath()); } catch (IOException e) { From 6bbec7f3b04278a384d3b55e05ab71cadad0d86e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 11 Dec 2019 00:11:29 +0100 Subject: [PATCH 08/16] Issue #4411 - Jetty server spins on incomplete request. Fixed HttpInput to be in READY state if an error is detected before the call to setWriteListener(). Signed-off-by: Simone Bordet --- .../jetty/server/HttpConfiguration.java | 2 +- .../org/eclipse/jetty/server/HttpInput.java | 29 ++++--- .../http/client/HttpClientStreamTest.java | 80 +++++++++++++++++++ 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java index 42aa19d9de9a..8545600a538f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java @@ -343,7 +343,7 @@ public boolean getSendDateHeader() } /** - * @param delay if true, delay the application dispatch until content is available (default false) + * @param delay if true, delays the application dispatch until content is available (defaults to true) */ public void setDelayDispatchUntilContent(boolean delay) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java index 15f04a2c8647..6b12818296b6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java @@ -756,22 +756,29 @@ public void setReadListener(ReadListener readListener) _listener = Objects.requireNonNull(readListener); - Content content = produceNextContext(); - if (content != null) + if (isError()) { - _state = ASYNC; woken = _channelState.onReadReady(); } - else if (_state == EOF) - { - _state = AEOF; - woken = _channelState.onReadEof(); - } else { - _state = ASYNC; - _channelState.onReadUnready(); - _waitingForContent = true; + Content content = produceNextContext(); + if (content != null) + { + _state = ASYNC; + woken = _channelState.onReadReady(); + } + else if (_state == EOF) + { + _state = AEOF; + woken = _channelState.onReadEof(); + } + else + { + _state = ASYNC; + _channelState.onReadUnready(); + _waitingForContent = true; + } } } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java index f56e260a2557..ac83e55357ab 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java @@ -43,7 +43,9 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import javax.servlet.AsyncContext; +import javax.servlet.ReadListener; import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -56,6 +58,7 @@ import org.eclipse.jetty.client.util.InputStreamContentProvider; import org.eclipse.jetty.client.util.InputStreamResponseListener; import org.eclipse.jetty.client.util.OutputStreamContentProvider; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; @@ -63,6 +66,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -1264,4 +1268,80 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques Result result = listener.await(5, TimeUnit.SECONDS); assertTrue(result.isSucceeded()); } + + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testClientDefersContentServerIdleTimeout(Transport transport) throws Exception + { + // TODO: fix FCGI that is failing this test. + Assumptions.assumeTrue(transport != Transport.FCGI); + + init(transport); + CountDownLatch dataLatch = new CountDownLatch(1); + CountDownLatch errorLatch = new CountDownLatch(1); + scenario.start(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException + { + AsyncContext asyncContext = request.startAsync(); + asyncContext.setTimeout(0); + request.getInputStream().setReadListener(new ReadListener() + { + @Override + public void onDataAvailable() + { + dataLatch.countDown(); + } + + @Override + public void onAllDataRead() + { + dataLatch.countDown(); + } + + @Override + public void onError(Throwable t) + { + errorLatch.countDown(); + response.setStatus(HttpStatus.REQUEST_TIMEOUT_408); + asyncContext.complete(); + } + }); + } + }); + long idleTimeout = 1000; + scenario.setServerIdleTimeout(idleTimeout); + + CountDownLatch latch = new CountDownLatch(1); + byte[] bytes = "[{\"key\":\"value\"}]".getBytes(StandardCharsets.UTF_8); + OutputStreamContentProvider content = new OutputStreamContentProvider() + { + @Override + public long getLength() + { + return bytes.length; + } + }; + scenario.client.newRequest(scenario.newURI()) + .method(HttpMethod.POST) + .path(scenario.servletPath) + .content(content, "application/json;charset=UTF-8") + .onResponseSuccess(response -> + { + assertEquals(HttpStatus.REQUEST_TIMEOUT_408, response.getStatus()); + latch.countDown(); + }) + .send(null); + + // Wait for the server to idle timeout. + Thread.sleep(2 * idleTimeout); + + assertTrue(errorLatch.await(5, TimeUnit.SECONDS)); + + // Do not send the content to the server. + + assertFalse(dataLatch.await(1, TimeUnit.SECONDS)); + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } } From 8e875ab7a4fbee318e1e7fe3fe59cbeda0b53e9e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 11 Dec 2019 13:02:23 +0100 Subject: [PATCH 09/16] Issue #4411 - Jetty server spins on incomplete request. Updated FastCGI code to pass the test. Signed-off-by: Simone Bordet --- .../eclipse/jetty/fcgi/server/HttpChannelOverFCGI.java | 8 ++++++++ .../eclipse/jetty/fcgi/server/ServerFCGIConnection.java | 8 ++++++++ .../eclipse/jetty/http/client/HttpClientStreamTest.java | 4 ---- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/HttpChannelOverFCGI.java b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/HttpChannelOverFCGI.java index 1ba90e21ce22..bc7542ed3546 100644 --- a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/HttpChannelOverFCGI.java +++ b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/HttpChannelOverFCGI.java @@ -125,6 +125,14 @@ protected void dispatch() dispatcher.dispatch(); } + public boolean onIdleTimeout(Throwable timeout) + { + boolean handle = getRequest().getHttpInput().onIdleTimeout(timeout); + if (handle) + execute(this); + return !handle; + } + private static class Dispatcher implements Runnable { private final AtomicReference state = new AtomicReference<>(State.IDLE); diff --git a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java index 73b45bda8c8f..97cfbbd3ff93 100644 --- a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java +++ b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/ServerFCGIConnection.java @@ -105,6 +105,14 @@ else if (read == 0) } } + @Override + protected boolean onReadTimeout(Throwable timeout) + { + return channels.values().stream() + .mapToInt(channel -> channel.onIdleTimeout(timeout) ? 0 : 1) + .sum() == 0; + } + private void parse(ByteBuffer buffer) { while (buffer.hasRemaining()) diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java index ac83e55357ab..a8ac3c23578d 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java @@ -66,7 +66,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; -import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -1273,9 +1272,6 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques @ArgumentsSource(TransportProvider.class) public void testClientDefersContentServerIdleTimeout(Transport transport) throws Exception { - // TODO: fix FCGI that is failing this test. - Assumptions.assumeTrue(transport != Transport.FCGI); - init(transport); CountDownLatch dataLatch = new CountDownLatch(1); CountDownLatch errorLatch = new CountDownLatch(1); From 2b58379f88899867484bdb94d82e7ace47edf8dc Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 Dec 2019 14:38:42 -0600 Subject: [PATCH 10/16] Issue #4415 - Addressing Gzip Decoding of large files + Now applying proper RFC 1952 ISIZE check. + Bit shifting is done with Longs against Long value. Signed-off-by: Joakim Erdfelt --- .../jetty/http/GZIPContentDecoder.java | 10 +- .../jetty/http/GZIPContentDecoderTest.java | 100 ++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index 6ca334aba29b..c8d7af6f2e03 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -37,13 +37,16 @@ */ public class GZIPContentDecoder implements Destroyable { + // Unsigned Integer Max == 2^32 + private static final long UINT_MAX = 0xffffffffL; + private final List _inflateds = new ArrayList<>(); private final Inflater _inflater = new Inflater(true); private final ByteBufferPool _pool; private final int _bufferSize; private State _state; private int _size; - private int _value; + private long _value; private byte _flags; private ByteBuffer _inflated; @@ -375,11 +378,12 @@ else if (_inflater.finished()) } case ISIZE: { - _value += (currByte & 0xFF) << 8 * _size; + _value = _value | ((currByte & 0xFFL) << (8 * _size)); ++_size; if (_size == 4) { - if (_value != _inflater.getBytesWritten()) + // RFC 1952: Section 2.3.1; ISIZE is the input size modulo 2^32 + if (_value != (_inflater.getBytesWritten() & UINT_MAX)) throw new ZipException("Invalid input size"); // TODO ByteBuffer result = output == null ? BufferUtil.EMPTY_BUFFER : ByteBuffer.wrap(output); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java index 93c7c4c3f373..64e113db32d9 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java @@ -20,6 +20,8 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.concurrent.atomic.AtomicInteger; @@ -27,10 +29,15 @@ import java.util.zip.GZIPOutputStream; import org.eclipse.jetty.io.ArrayByteBufferPool; +import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -351,4 +358,97 @@ public void testBigBlockWithExtraBytes() throws Exception assertTrue(buffer.hasRemaining()); assertEquals(data2, StandardCharsets.UTF_8.decode(buffer).toString()); } + + // Signed Integer Max + final long INT_MAX = Integer.MAX_VALUE; + + // Unsigned Integer Max == 2^32 + final long UINT_MAX = 0xffffffffL; + + @ParameterizedTest + @ValueSource(longs = {INT_MAX, INT_MAX + 1, UINT_MAX, UINT_MAX + 1}) + public void testLargeGzipStream(long origSize) throws IOException + { + final int BUFSIZE = (1 * 1024 * 1024); // 1MB + + // Create a buffer to use over and over again to produce the uncompressed input + byte[] cbuf = "0123456789ABCDEFGHIJKLMOPQRSTUVWXYZ".getBytes(StandardCharsets.UTF_8); + byte[] buf = new byte[BUFSIZE]; + for (int off = 0; off < buf.length; ) + { + int len = Math.min(cbuf.length, buf.length - off); + System.arraycopy(cbuf, 0, buf, off, len); + off += len; + } + + GZIPContentDecoder decoder = new GZIPContentDecoder(BUFSIZE); + + try (GZIPDecoderOutputStream out = new GZIPDecoderOutputStream(decoder); + GZIPOutputStream outputStream = new GZIPOutputStream(out, BUFSIZE)) + { + long writtenBytes = 0L; + for (long bytesLeft = origSize; bytesLeft > 0; ) + { + int len = buf.length; + if (bytesLeft < buf.length) + { + len = (int)bytesLeft; + } + outputStream.write(buf, 0, len); + bytesLeft -= len; + writtenBytes += len; + } + outputStream.close(); + + assertThat("Written byte count", writtenBytes, is(origSize)); + assertThat("Decoded byte count", out.decodedByteCount, is(origSize)); + } + } + + public static class GZIPDecoderOutputStream extends OutputStream + { + private final GZIPContentDecoder decoder; + public long decodedByteCount = 0L; + + public GZIPDecoderOutputStream(GZIPContentDecoder decoder) + { + this.decoder = decoder; + } + + @Override + public void write(byte[] b, int off, int len) throws IOException + { + ByteBuffer buf = ByteBuffer.wrap(b, off, len); + decode(buf); + } + + @Override + public void write(byte[] b) throws IOException + { + ByteBuffer buf = ByteBuffer.wrap(b); + decode(buf); + } + + @Override + public void write(int b) throws IOException + { + ByteBuffer buf = BufferUtil.allocate(32); + buf.put((byte)(b & 0xFF)); + buf.flip(); + decode(buf); + } + + private void decode(ByteBuffer buffer) + { + while (buffer.hasRemaining()) + { + ByteBuffer decoded = decoder.decode(buffer); + if (decoded.hasRemaining()) + { + decodedByteCount += decoded.remaining(); + } + decoder.release(decoded); + } + } + } } From 1e810d370f0e2a2c7751d1cf7bcf4081d8a45ce7 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 12 Dec 2019 11:58:44 -0600 Subject: [PATCH 11/16] Issue #4415 - Addressing Gzip Decoding of large files + Addressing PR review Signed-off-by: Joakim Erdfelt --- .../jetty/http/GZIPContentDecoder.java | 2 +- .../jetty/http/GZIPContentDecoderTest.java | 66 +++++++------------ 2 files changed, 26 insertions(+), 42 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index c8d7af6f2e03..bd73544e44f1 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -38,7 +38,7 @@ public class GZIPContentDecoder implements Destroyable { // Unsigned Integer Max == 2^32 - private static final long UINT_MAX = 0xffffffffL; + private static final long UINT_MAX = 0xFFFFFFFFL; private final List _inflateds = new ArrayList<>(); private final Inflater _inflater = new Inflater(true); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java index 64e113db32d9..4261a12651d7 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java @@ -29,7 +29,6 @@ import java.util.zip.GZIPOutputStream; import org.eclipse.jetty.io.ArrayByteBufferPool; -import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -363,7 +362,7 @@ public void testBigBlockWithExtraBytes() throws Exception final long INT_MAX = Integer.MAX_VALUE; // Unsigned Integer Max == 2^32 - final long UINT_MAX = 0xffffffffL; + final long UINT_MAX = 0xFFFFFFFFL; @ParameterizedTest @ValueSource(longs = {INT_MAX, INT_MAX + 1, UINT_MAX, UINT_MAX + 1}) @@ -383,26 +382,26 @@ public void testLargeGzipStream(long origSize) throws IOException GZIPContentDecoder decoder = new GZIPContentDecoder(BUFSIZE); - try (GZIPDecoderOutputStream out = new GZIPDecoderOutputStream(decoder); - GZIPOutputStream outputStream = new GZIPOutputStream(out, BUFSIZE)) + GZIPDecoderOutputStream out = new GZIPDecoderOutputStream(decoder); + GZIPOutputStream outputStream = new GZIPOutputStream(out, BUFSIZE); + + for (long bytesLeft = origSize; bytesLeft > 0; ) { - long writtenBytes = 0L; - for (long bytesLeft = origSize; bytesLeft > 0; ) + int len = buf.length; + if (bytesLeft < buf.length) { - int len = buf.length; - if (bytesLeft < buf.length) - { - len = (int)bytesLeft; - } - outputStream.write(buf, 0, len); - bytesLeft -= len; - writtenBytes += len; + len = (int)bytesLeft; } - outputStream.close(); - - assertThat("Written byte count", writtenBytes, is(origSize)); - assertThat("Decoded byte count", out.decodedByteCount, is(origSize)); + outputStream.write(buf, 0, len); + bytesLeft -= len; } + + // Close GZIPOutputStream to have it generate gzip trailer. + // This can cause more writes of unflushed gzip buffers + outputStream.close(); + + // out.decodedByteCount is only valid after close + assertThat("Decoded byte count", out.decodedByteCount, is(origSize)); } public static class GZIPDecoderOutputStream extends OutputStream @@ -419,30 +418,9 @@ public GZIPDecoderOutputStream(GZIPContentDecoder decoder) public void write(byte[] b, int off, int len) throws IOException { ByteBuffer buf = ByteBuffer.wrap(b, off, len); - decode(buf); - } - - @Override - public void write(byte[] b) throws IOException - { - ByteBuffer buf = ByteBuffer.wrap(b); - decode(buf); - } - - @Override - public void write(int b) throws IOException - { - ByteBuffer buf = BufferUtil.allocate(32); - buf.put((byte)(b & 0xFF)); - buf.flip(); - decode(buf); - } - - private void decode(ByteBuffer buffer) - { - while (buffer.hasRemaining()) + while (buf.hasRemaining()) { - ByteBuffer decoded = decoder.decode(buffer); + ByteBuffer decoded = decoder.decode(buf); if (decoded.hasRemaining()) { decodedByteCount += decoded.remaining(); @@ -450,5 +428,11 @@ private void decode(ByteBuffer buffer) decoder.release(decoded); } } + + @Override + public void write(int b) throws IOException + { + write(new byte[]{(byte)(b & 0xFF)}, 0, 1); + } } } From 36d06d016d966169122ea4858fc4a6c98db60bea Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 12 Dec 2019 13:23:44 -0600 Subject: [PATCH 12/16] Issue #4415 - Addressing Gzip Decoding of large files + Addressing PR review Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/http/GZIPContentDecoderTest.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java index 4261a12651d7..47f0165d616f 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java @@ -368,7 +368,8 @@ public void testBigBlockWithExtraBytes() throws Exception @ValueSource(longs = {INT_MAX, INT_MAX + 1, UINT_MAX, UINT_MAX + 1}) public void testLargeGzipStream(long origSize) throws IOException { - final int BUFSIZE = (1 * 1024 * 1024); // 1MB + // Size chosen for trade off between speed of I/O vs speed of Gzip + final int BUFSIZE = 1024 * 1024; // Create a buffer to use over and over again to produce the uncompressed input byte[] cbuf = "0123456789ABCDEFGHIJKLMOPQRSTUVWXYZ".getBytes(StandardCharsets.UTF_8); @@ -380,9 +381,7 @@ public void testLargeGzipStream(long origSize) throws IOException off += len; } - GZIPContentDecoder decoder = new GZIPContentDecoder(BUFSIZE); - - GZIPDecoderOutputStream out = new GZIPDecoderOutputStream(decoder); + GZIPDecoderOutputStream out = new GZIPDecoderOutputStream(new GZIPContentDecoder(BUFSIZE)); GZIPOutputStream outputStream = new GZIPOutputStream(out, BUFSIZE); for (long bytesLeft = origSize; bytesLeft > 0; ) @@ -432,7 +431,7 @@ public void write(byte[] b, int off, int len) throws IOException @Override public void write(int b) throws IOException { - write(new byte[]{(byte)(b & 0xFF)}, 0, 1); + write(new byte[]{(byte)b}, 0, 1); } } } From 9f93577054de1ffb7854e5a70c04588a85975463 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 17 Dec 2019 07:40:19 +1100 Subject: [PATCH 13/16] Update Notice (#4395) Signed-off-by: Greg Wilkins --- NOTICE.txt | 130 +++++++++++++++++++++++------------------------------ 1 file changed, 57 insertions(+), 73 deletions(-) diff --git a/NOTICE.txt b/NOTICE.txt index 28ee14ee3e19..3c7d3124ab4c 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -1,41 +1,42 @@ -============================================================== - Jetty Web Container - Copyright 1995-2019 Mort Bay Consulting Pty Ltd. -============================================================== +Notices for Eclipse Jetty +========================= +This content is produced and maintained by the Eclipse Jetty project. -The Jetty Web Container is Copyright Mort Bay Consulting Pty Ltd -unless otherwise noted. +Project home: https://www.eclipse.org/jetty/ -Jetty is dual licensed under both +Trademarks +---------- +Eclipse Jetty, and Jetty are trademarks of the Eclipse Foundation. - * The Apache 2.0 License - http://www.apache.org/licenses/LICENSE-2.0.html +Copyright +--------- +All contributions are the property of the respective authors or of +entities to which copyright has been assigned by the authors (eg. employer). - and +Declared Project Licenses +------------------------- +This artifacts of this project are made available under the terms of: - * The Eclipse Public 1.0 License + * the Eclipse Public License v. 1.0 http://www.eclipse.org/legal/epl-v10.html + SPDX-License-Identifier: EPL-1.0 -Jetty may be distributed under either license. + or ------- -Eclipse + * the Apache License, Version 2.0 + https://www.apache.org/licenses/LICENSE-2.0. + SPDX-License-Identifier: Apache-2.0 -The following artifacts are EPL. +The following dependencies are EPL. * org.eclipse.jetty.orbit:org.eclipse.jdt.core -The following artifacts are EPL and ASL2. +The following dependencies are EPL and ASL2. * org.eclipse.jetty.orbit:javax.security.auth.message - -The following artifacts are EPL and CDDL 1.0. +The following dependencies are EPL and CDDL 1.0. * org.eclipse.jetty.orbit:javax.mail.glassfish - ------- -Oracle - -The following artifacts are CDDL + GPLv2 with classpath exception. +The following dependencies are CDDL + GPLv2 with classpath exception. https://glassfish.dev.java.net/nonav/public/CDDL+GPL.html * javax.servlet:javax.servlet-api @@ -43,72 +44,55 @@ https://glassfish.dev.java.net/nonav/public/CDDL+GPL.html * javax.transaction:javax.transaction-api * javax.websocket:javax.websocket-api ------- -Oracle OpenJDK - If ALPN is used to negotiate HTTP/2 connections, then the following -artifacts may be included in the distribution or downloaded when ALPN -module is selected. - - * java.sun.security.ssl - -These artifacts replace/modify OpenJDK classes. The modififications -are hosted at github and both modified and original are under GPL v2 with -classpath exceptions. +distribution may be included in the distribution or downloaded when ALPN +module is selected. These artifacts replace/modify OpenJDK classes. +The modifications are hosted at github and both modified and original +are under GPL v2 with classpath exceptions. http://openjdk.java.net/legal/gplv2+ce.html + * java.sun.security.ssl ------- -OW2 - -The following artifacts are licensed by the OW2 Foundation according to the +The following dependencies are licensed by the OW2 Foundation according to the terms of http://asm.ow2.org/license.html -org.ow2.asm:asm-commons -org.ow2.asm:asm - - ------- -Apache - -The following artifacts are ASL2 licensed. - -org.apache.taglibs:taglibs-standard-spec -org.apache.taglibs:taglibs-standard-impl + * org.ow2.asm:asm-commons + * org.ow2.asm:asm +The following dependencies are ASL2 licensed. ------- -MortBay + * org.apache.taglibs:taglibs-standard-spec + * org.apache.taglibs:taglibs-standard-impl -The following artifacts are ASL2 licensed. Based on selected classes from +The following dependencies are ASL2 licensed. Based on selected classes from following Apache Tomcat jars, all ASL2 licensed. -org.mortbay.jasper:apache-jsp - org.apache.tomcat:tomcat-jasper - org.apache.tomcat:tomcat-juli - org.apache.tomcat:tomcat-jsp-api - org.apache.tomcat:tomcat-el-api - org.apache.tomcat:tomcat-jasper-el - org.apache.tomcat:tomcat-api - org.apache.tomcat:tomcat-util-scan - org.apache.tomcat:tomcat-util - -org.mortbay.jasper:apache-el - org.apache.tomcat:tomcat-jasper-el - org.apache.tomcat:tomcat-el-api - - ------- -Mortbay + * org.mortbay.jasper:apache-jsp + * org.apache.tomcat:tomcat-jasper + * org.apache.tomcat:tomcat-juli + * org.apache.tomcat:tomcat-jsp-api + * org.apache.tomcat:tomcat-el-api + * org.apache.tomcat:tomcat-jasper-el + * org.apache.tomcat:tomcat-api + * org.apache.tomcat:tomcat-util-scan + * org.apache.tomcat:tomcat-util + * org.mortbay.jasper:apache-el + * org.apache.tomcat:tomcat-jasper-el + * org.apache.tomcat:tomcat-el-api The following artifacts are CDDL + GPLv2 with classpath exception. - https://glassfish.dev.java.net/nonav/public/CDDL+GPL.html -org.eclipse.jetty.toolchain:jetty-schemas + * org.eclipse.jetty.toolchain:jetty-schemas ------- -Assorted +Cryptography +------------ +Content may contain encryption software. The country in which you are currently +may have restrictions on the import, possession, and use, and/or re-export to +another country, of encryption software. BEFORE using any encryption software, +please check the country's laws, regulations and policies concerning the import, +possession, or use, and re-export of encryption software, to see if this is +permitted. The UnixCrypt.java code implements the one way cryptography used by Unix systems for simple password protection. Copyright 1996 Aki Yoshida, From 584e264b0b198092ca39da36618dd85a0db50ec4 Mon Sep 17 00:00:00 2001 From: Lachlan Date: Tue, 17 Dec 2019 11:28:39 +1100 Subject: [PATCH 14/16] Clean up CustomRequestLog and fix the handling of the %u code. (#4397) * Clean up CustomRequestLog and fix the handling of the %u code. * Add test for logging of remote user with %u and %{d}u * update javadoc to clarify that %u is only for servlet auth * remove the prepended '?' when deferred authentication is checked --- .../jetty/server/CustomRequestLog.java | 67 +++++++-------- .../jetty/test}/CustomRequestLogTest.java | 81 ++++++++++++++++--- 2 files changed, 101 insertions(+), 47 deletions(-) rename {jetty-server/src/test/java/org/eclipse/jetty/server/handler => tests/test-integration/src/test/java/org/eclipse/jetty/test}/CustomRequestLogTest.java (87%) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java index 8135dfbc5b8f..5a58abc84cf2 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java @@ -31,12 +31,14 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.http.pathmap.PathMappings; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.DateCache; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.ContainerLifeCycle; @@ -233,8 +235,9 @@ * * %{d}u * - * Remote user if the request was authenticated. May be bogus if return status (%s) is 401 (unauthorized). - * Optional parameter d, with this parameter deferred authentication will also be checked. + * Remote user if the request was authenticated with servlet authentication. May be bogus if return status (%s) is 401 (unauthorized). + * Optional parameter d, with this parameter deferred authentication will also be checked, + * this is equivalent to {@link HttpServletRequest#getRemoteUser()}. * * * @@ -294,11 +297,7 @@ public CustomRequestLog(RequestLog.Writer writer, String formatString) { _logHandle = getLogHandle(formatString); } - catch (NoSuchMethodException e) - { - throw new IllegalStateException(e); - } - catch (IllegalAccessException e) + catch (NoSuchMethodException | IllegalAccessException e) { throw new IllegalStateException(e); } @@ -357,20 +356,14 @@ public void log(Request request, Response response) protected static String getAuthentication(Request request, boolean checkDeferred) { Authentication authentication = request.getAuthentication(); - - String name = null; - - boolean deferred = false; if (checkDeferred && authentication instanceof Authentication.Deferred) - { authentication = ((Authentication.Deferred)authentication).authenticate(request); - deferred = true; - } + String name = null; if (authentication instanceof Authentication.User) name = ((Authentication.User)authentication).getUserIdentity().getUserPrincipal().getName(); - return (name == null) ? null : (deferred ? ("?" + name) : name); + return name; } /** @@ -415,9 +408,9 @@ protected synchronized void doStart() throws Exception if (_ignorePaths != null && _ignorePaths.length > 0) { _ignorePathMap = new PathMappings<>(); - for (int i = 0; i < _ignorePaths.length; i++) + for (String ignorePath : _ignorePaths) { - _ignorePathMap.put(_ignorePaths[i], _ignorePaths[i]); + _ignorePathMap.put(ignorePath, ignorePath); } } else @@ -442,8 +435,8 @@ private static void append(String s, StringBuilder buf) private MethodHandle getLogHandle(String formatString) throws NoSuchMethodException, IllegalAccessException { MethodHandles.Lookup lookup = MethodHandles.lookup(); - MethodHandle append = lookup.findStatic(CustomRequestLog.class, "append", methodType(Void.TYPE, String.class, StringBuilder.class)); - MethodHandle logHandle = lookup.findStatic(CustomRequestLog.class, "logNothing", methodType(Void.TYPE, StringBuilder.class, Request.class, Response.class)); + MethodHandle append = lookup.findStatic(CustomRequestLog.class, "append", methodType(void.class, String.class, StringBuilder.class)); + MethodHandle logHandle = lookup.findStatic(CustomRequestLog.class, "logNothing", methodType(void.class, StringBuilder.class, Request.class, Response.class)); List tokens = getTokens(formatString); Collections.reverse(tokens); @@ -486,7 +479,7 @@ private static List getTokens(String formatString) String arg = m.group("ARG"); String modifierString = m.group("MOD"); - Boolean negated = false; + boolean negated = false; if (modifierString != null) { if (modifierString.startsWith("!")) @@ -581,8 +574,8 @@ private static boolean modify(List modifiers, Boolean negated, StringBui private MethodHandle updateLogHandle(MethodHandle logHandle, MethodHandle append, MethodHandles.Lookup lookup, String code, String arg, List modifiers, boolean negated) throws NoSuchMethodException, IllegalAccessException { - MethodType logType = methodType(Void.TYPE, StringBuilder.class, Request.class, Response.class); - MethodType logTypeArg = methodType(Void.TYPE, String.class, StringBuilder.class, Request.class, Response.class); + MethodType logType = methodType(void.class, StringBuilder.class, Request.class, Response.class); + MethodType logTypeArg = methodType(void.class, String.class, StringBuilder.class, Request.class, Response.class); //TODO should we throw IllegalArgumentExceptions when given arguments for codes which do not take them MethodHandle specificHandle; @@ -596,7 +589,7 @@ private MethodHandle updateLogHandle(MethodHandle logHandle, MethodHandle append case "a": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) arg = "server"; String method; @@ -628,7 +621,7 @@ private MethodHandle updateLogHandle(MethodHandle logHandle, MethodHandle append case "p": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) arg = "server"; String method; @@ -662,7 +655,7 @@ private MethodHandle updateLogHandle(MethodHandle logHandle, MethodHandle append case "I": { String method; - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) method = "logBytesReceived"; else if (arg.equalsIgnoreCase("clf")) method = "logBytesReceivedCLF"; @@ -676,7 +669,7 @@ else if (arg.equalsIgnoreCase("clf")) case "O": { String method; - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) method = "logBytesSent"; else if (arg.equalsIgnoreCase("clf")) method = "logBytesSentCLF"; @@ -690,7 +683,7 @@ else if (arg.equalsIgnoreCase("clf")) case "S": { String method; - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) method = "logBytesTransferred"; else if (arg.equalsIgnoreCase("clf")) method = "logBytesTransferredCLF"; @@ -703,7 +696,7 @@ else if (arg.equalsIgnoreCase("clf")) case "C": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) { specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestCookies", logType); } @@ -723,7 +716,7 @@ else if (arg.equalsIgnoreCase("clf")) case "e": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) throw new IllegalArgumentException("No arg for %e"); specificHandle = lookup.findStatic(CustomRequestLog.class, "logEnvironmentVar", logTypeArg); @@ -745,7 +738,7 @@ else if (arg.equalsIgnoreCase("clf")) case "i": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) throw new IllegalArgumentException("No arg for %i"); specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestHeader", logTypeArg); @@ -767,7 +760,7 @@ else if (arg.equalsIgnoreCase("clf")) case "o": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) throw new IllegalArgumentException("No arg for %o"); specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseHeader", logTypeArg); @@ -832,7 +825,7 @@ else if (arg.equalsIgnoreCase("clf")) DateCache logDateCache = new DateCache(format, locale, timeZone); - MethodType logTypeDateCache = methodType(Void.TYPE, DateCache.class, StringBuilder.class, Request.class, Response.class); + MethodType logTypeDateCache = methodType(void.class, DateCache.class, StringBuilder.class, Request.class, Response.class); specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestTime", logTypeDateCache); specificHandle = specificHandle.bindTo(logDateCache); break; @@ -866,10 +859,12 @@ else if (arg.equalsIgnoreCase("clf")) case "u": { String method; - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) + method = "logRequestAuthentication"; + else if ("d".equals(arg)) method = "logRequestAuthenticationWithDeferred"; else - method = "logRequestAuthentication"; + throw new IllegalArgumentException("Invalid arg for %u: " + arg); specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; @@ -889,7 +884,7 @@ else if (arg.equalsIgnoreCase("clf")) case "ti": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) throw new IllegalArgumentException("No arg for %ti"); specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestTrailer", logTypeArg); @@ -899,7 +894,7 @@ else if (arg.equalsIgnoreCase("clf")) case "to": { - if (arg == null || arg.isEmpty()) + if (StringUtil.isEmpty(arg)) throw new IllegalArgumentException("No arg for %to"); specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseTrailer", logTypeArg); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/CustomRequestLogTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java similarity index 87% rename from jetty-server/src/test/java/org/eclipse/jetty/server/handler/CustomRequestLogTest.java rename to tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java index b2a816e887d2..32769fc0fd9a 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/CustomRequestLogTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java @@ -16,7 +16,7 @@ // ======================================================================== // -package org.eclipse.jetty.server.handler; +package org.eclipse.jetty.test; import java.io.IOException; import java.io.InputStream; @@ -26,15 +26,25 @@ import java.net.Socket; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.util.Base64; import java.util.Enumeration; import java.util.Locale; +import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.security.ConstraintMapping; +import org.eclipse.jetty.security.ConstraintSecurityHandler; +import org.eclipse.jetty.security.HashLoginService; +import org.eclipse.jetty.security.SecurityHandler; +import org.eclipse.jetty.security.UserStore; +import org.eclipse.jetty.security.authentication.BasicAuthenticator; import org.eclipse.jetty.server.CustomRequestLog; import org.eclipse.jetty.server.ForwardedRequestCustomizer; import org.eclipse.jetty.server.HttpConnectionFactory; @@ -44,8 +54,12 @@ import org.eclipse.jetty.server.RequestLog; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.DateCache; +import org.eclipse.jetty.util.security.Constraint; +import org.eclipse.jetty.util.security.Credential; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; @@ -88,24 +102,70 @@ void testHandlerServerStart(String formatString) throws Exception TestRequestLogWriter writer = new TestRequestLogWriter(); _log = new CustomRequestLog(writer, formatString); _server.setRequestLog(_log); - _server.setHandler(new TestHandler()); + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setSecurityHandler(getSecurityHandler("username", "password", "testRealm")); + contextHandler.addServlet(new ServletHolder(new TestServlet()), "/"); + _server.setHandler(contextHandler); _server.start(); String host = _serverConnector.getHost(); if (host == null) - { host = "localhost"; - } + int localPort = _serverConnector.getLocalPort(); _serverURI = new URI(String.format("http://%s:%d/", host, localPort)); } + private static SecurityHandler getSecurityHandler(String username, String password, String realm) + { + HashLoginService loginService = new HashLoginService(); + UserStore userStore = new UserStore(); + userStore.addUser(username, Credential.getCredential(password), new String[]{"user"}); + loginService.setUserStore(userStore); + loginService.setName(realm); + + Constraint constraint = new Constraint(); + constraint.setName("auth"); + constraint.setAuthenticate(true); + constraint.setRoles(new String[]{"**"}); + + ConstraintMapping mapping = new ConstraintMapping(); + mapping.setPathSpec("/secure/*"); + mapping.setConstraint(constraint); + + ConstraintSecurityHandler security = new ConstraintSecurityHandler(); + security.addConstraintMapping(mapping); + security.setAuthenticator(new BasicAuthenticator()); + security.setLoginService(loginService); + + return security; + } + @AfterEach public void after() throws Exception { _server.stop(); } + @Test + public void testLogRemoteUser() throws Exception + { + String authHeader = HttpHeader.AUTHORIZATION + ": Basic " + Base64.getEncoder().encodeToString("username:password".getBytes()); + testHandlerServerStart("%u %{d}u"); + + _connector.getResponse("GET / HTTP/1.0\n\n\n"); + String log = _entries.poll(5, TimeUnit.SECONDS); + assertThat(log, is("- -")); + + _connector.getResponse("GET / HTTP/1.0\n" + authHeader + "\n\n\n"); + log = _entries.poll(5, TimeUnit.SECONDS); + assertThat(log, is("- username")); + + _connector.getResponse("GET /secure HTTP/1.0\n" + authHeader + "\n\n\n"); + log = _entries.poll(5, TimeUnit.SECONDS); + assertThat(log, is("username username")); + } + @Test public void testModifier() throws Exception { @@ -374,7 +434,7 @@ public void testLogRequestTime() throws Exception _connector.getResponse("GET / HTTP/1.0\n\n"); String log = _entries.poll(5, TimeUnit.SECONDS); long requestTime = requestTimes.poll(5, TimeUnit.SECONDS); - DateCache dateCache = new DateCache(_log.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT"); + DateCache dateCache = new DateCache(CustomRequestLog.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT"); assertThat(log, is("RequestTime: [" + dateCache.format(requestTime) + "]")); } @@ -549,11 +609,13 @@ public void write(String requestEntry) } } - private class TestHandler extends AbstractHandler + private class TestServlet extends HttpServlet { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + Request baseRequest = Objects.requireNonNull(Request.getBaseRequest(request)); + if (request.getRequestURI().contains("error404")) { response.setStatus(404); @@ -596,10 +658,7 @@ else if (request.getRequestURI().contains("delay")) if (request.getContentLength() > 0) { InputStream in = request.getInputStream(); - while (in.read() > 0) - { - ; - } + while (in.read() > 0); } } } From 129a51c7a2d4da1de4b4a4787072f62130fa76b9 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 17 Dec 2019 10:36:16 +0100 Subject: [PATCH 15/16] Fixes #4421 - HttpClient support for PROXY protocol. (#4424) * Fixes #4421 - HttpClient support for PROXY protocol. Implemented support for the PROXY protocol in HttpClient. Introduced Request.tag(Object) to tag requests that belong to the same group (e.g. a client address) so that they can generate a different destination. The tag object may implement ClientConnectionFactory.Decorator so that it can decorate the HttpDestination ClientConnectionFactory and therefore work both with and without forward proxy configuration. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpClient.java | 19 +- .../eclipse/jetty/client/HttpDestination.java | 3 + .../org/eclipse/jetty/client/HttpProxy.java | 8 +- .../org/eclipse/jetty/client/HttpRequest.java | 14 + .../java/org/eclipse/jetty/client/Origin.java | 34 +- .../jetty/client/ProxyConfiguration.java | 2 +- .../ProxyProtocolClientConnectionFactory.java | 621 ++++++++++++++++++ .../org/eclipse/jetty/client/api/Request.java | 22 + .../client/HttpClientProxyProtocolTest.java | 264 ++++++++ .../jetty/io/ClientConnectionFactory.java | 17 + .../jetty/server/ProxyConnectionFactory.java | 48 +- .../jetty/server/ProxyConnectionTest.java | 5 +- .../jetty/http/client/HttpClientTest.java | 26 + 13 files changed, 1052 insertions(+), 31 deletions(-) create mode 100644 jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java create mode 100644 jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index a295fe8735bd..a009cd89d7b2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -530,16 +530,29 @@ public Destination getDestination(String scheme, String host, int port) } protected HttpDestination destinationFor(String scheme, String host, int port) + { + return resolveDestination(scheme, host, port, null); + } + + protected HttpDestination resolveDestination(String scheme, String host, int port, Object tag) + { + Origin origin = createOrigin(scheme, host, port, tag); + return resolveDestination(origin); + } + + protected Origin createOrigin(String scheme, String host, int port, Object tag) { if (!HttpScheme.HTTP.is(scheme) && !HttpScheme.HTTPS.is(scheme) && !HttpScheme.WS.is(scheme) && !HttpScheme.WSS.is(scheme)) throw new IllegalArgumentException("Invalid protocol " + scheme); - scheme = scheme.toLowerCase(Locale.ENGLISH); host = host.toLowerCase(Locale.ENGLISH); port = normalizePort(scheme, port); + return new Origin(scheme, host, port, tag); + } - Origin origin = new Origin(scheme, host, port); + protected HttpDestination resolveDestination(Origin origin) + { return destinations.computeIfAbsent(origin, o -> { HttpDestination newDestination = getTransport().newHttpDestination(o); @@ -566,7 +579,7 @@ public List getDestinations() protected void send(final HttpRequest request, List listeners) { - HttpDestination destination = destinationFor(request.getScheme(), request.getHost(), request.getPort()); + HttpDestination destination = resolveDestination(request.getScheme(), request.getHost(), request.getPort(), request.getTag()); destination.send(request, listeners); } 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 f7b40536b65d..640dfc24d037 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 @@ -94,6 +94,9 @@ public HttpDestination(HttpClient client, Origin origin) if (isSecure()) connectionFactory = newSslClientConnectionFactory(null, connectionFactory); } + Object tag = origin.getTag(); + if (tag instanceof ClientConnectionFactory.Decorator) + connectionFactory = ((ClientConnectionFactory.Decorator)tag).apply(connectionFactory); this.connectionFactory = connectionFactory; String host = HostPort.normalizeHost(getHost()); 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 3591d549cd97..ee9fecb9d149 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 @@ -72,7 +72,7 @@ public URI getURI() return URI.create(new Origin(scheme, getAddress()).asString()); } - private class HttpProxyClientConnectionFactory implements ClientConnectionFactory + private static class HttpProxyClientConnectionFactory implements ClientConnectionFactory { private final ClientConnectionFactory connectionFactory; @@ -127,7 +127,7 @@ public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map */ - private class CreateTunnelPromise implements Promise + private static class CreateTunnelPromise implements Promise { private final ClientConnectionFactory connectionFactory; private final EndPoint endPoint; @@ -233,7 +233,7 @@ private void tunnelFailed(EndPoint endPoint, Throwable failure) } } - private class ProxyConnection implements Connection + private static class ProxyConnection implements Connection { private final Destination destination; private final Connection connection; @@ -272,7 +272,7 @@ public boolean isClosed() } } - private class TunnelPromise implements Promise + private static class TunnelPromise implements Promise { private final Request request; private final Response.CompleteListener listener; 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 bb0c22b5fe3d..6a820a241192 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 @@ -87,6 +87,7 @@ public class HttpRequest implements Request private List requestListeners; private BiFunction pushListener; private Supplier trailers; + private Object tag; protected HttpRequest(HttpClient client, HttpConversation conversation, URI uri) { @@ -313,6 +314,19 @@ public Request cookie(HttpCookie cookie) return this; } + @Override + public Request tag(Object tag) + { + this.tag = tag; + return this; + } + + @Override + public Object getTag() + { + return tag; + } + @Override public Request attribute(String name, Object value) { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java b/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java index 0587b8348e05..9080a10e86d8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/Origin.java @@ -26,16 +26,28 @@ public class Origin { private final String scheme; private final Address address; + private final Object tag; public Origin(String scheme, String host, int port) { - this(scheme, new Address(host, port)); + this(scheme, host, port, null); + } + + public Origin(String scheme, String host, int port, Object tag) + { + this(scheme, new Address(host, port), tag); } public Origin(String scheme, Address address) + { + this(scheme, address, null); + } + + public Origin(String scheme, Address address, Object tag) { this.scheme = Objects.requireNonNull(scheme); this.address = address; + this.tag = tag; } public String getScheme() @@ -48,6 +60,11 @@ public Address getAddress() return address; } + public Object getTag() + { + return tag; + } + public String asString() { StringBuilder result = new StringBuilder(); @@ -63,14 +80,23 @@ public boolean equals(Object obj) if (obj == null || getClass() != obj.getClass()) return false; Origin that = (Origin)obj; - return scheme.equals(that.scheme) && address.equals(that.address); + return scheme.equals(that.scheme) && + address.equals(that.address) && + Objects.equals(tag, that.tag); } @Override public int hashCode() { - int result = scheme.hashCode(); - result = 31 * result + address.hashCode(); + return Objects.hash(scheme, address, tag); + } + + @Override + public String toString() + { + String result = asString(); + if (tag != null) + result += "[tag=" + tag + "]"; return result; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyConfiguration.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyConfiguration.java index 682078dbc9a5..1fc5733360c9 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyConfiguration.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyConfiguration.java @@ -61,7 +61,7 @@ public Proxy match(Origin origin) public abstract static class Proxy { - // TO use IPAddress Map + // TODO use InetAddressSet? Or IncludeExcludeSet? private final Set included = new HashSet<>(); private final Set excluded = new HashSet<>(); private final Origin.Address address; diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java new file mode 100644 index 000000000000..b371e5a5a2d7 --- /dev/null +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java @@ -0,0 +1,621 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.client; + +import java.net.Inet4Address; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.Executor; + +import org.eclipse.jetty.io.AbstractConnection; +import org.eclipse.jetty.io.ClientConnectionFactory; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + *

ClientConnectionFactory for the + * PROXY protocol.

+ *

Use the {@link V1} or {@link V2} versions of this class to specify what version of the + * PROXY protocol you want to use.

+ */ +public abstract class ProxyProtocolClientConnectionFactory implements ClientConnectionFactory +{ + /** + * A ClientConnectionFactory for the PROXY protocol version 1. + */ + public static class V1 extends ProxyProtocolClientConnectionFactory + { + public V1(ClientConnectionFactory factory) + { + super(factory); + } + + @Override + protected ProxyProtocolConnection newProxyProtocolConnection(EndPoint endPoint, Map context) + { + HttpDestination destination = (HttpDestination)context.get(HttpClientTransport.HTTP_DESTINATION_CONTEXT_KEY); + Executor executor = destination.getHttpClient().getExecutor(); + Tag tag = (Tag)destination.getOrigin().getTag(); + if (tag == null) + { + InetSocketAddress local = endPoint.getLocalAddress(); + InetSocketAddress remote = endPoint.getRemoteAddress(); + boolean ipv4 = local.getAddress() instanceof Inet4Address; + tag = new Tag(ipv4 ? "TCP4" : "TCP6", local.getAddress().getHostAddress(), local.getPort(), remote.getAddress().getHostAddress(), remote.getPort()); + } + return new ProxyProtocolConnectionV1(endPoint, executor, getClientConnectionFactory(), context, tag); + } + + /** + *

PROXY protocol version 1 metadata holder to be used in conjunction + * with {@link org.eclipse.jetty.client.api.Request#tag(Object)}.

+ *

Instances of this class are associated to a destination so that + * all connections of that destination will initiate the communication + * with the PROXY protocol version 1 bytes specified by this metadata.

+ */ + public static class Tag implements ClientConnectionFactory.Decorator + { + /** + * The PROXY V1 Tag typically used to "ping" the server. + */ + public static final Tag UNKNOWN = new Tag("UNKNOWN", null, 0, null, 0); + + private final String family; + private final String srcIP; + private final int srcPort; + private final String dstIP; + private final int dstPort; + + /** + *

Creates a Tag whose metadata will be derived from the underlying EndPoint.

+ */ + public Tag() + { + this(null, 0); + } + + /** + *

Creates a Tag with the given source metadata.

+ *

The destination metadata will be derived from the underlying EndPoint.

+ * + * @param srcIP the source IP address + * @param srcPort the source port + */ + public Tag(String srcIP, int srcPort) + { + this(null, srcIP, srcPort, null, 0); + } + + /** + *

Creates a Tag with the given metadata.

+ * + * @param family the protocol family + * @param srcIP the source IP address + * @param srcPort the source port + * @param dstIP the destination IP address + * @param dstPort the destination port + */ + public Tag(String family, String srcIP, int srcPort, String dstIP, int dstPort) + { + this.family = family; + this.srcIP = srcIP; + this.srcPort = srcPort; + this.dstIP = dstIP; + this.dstPort = dstPort; + } + + public String getFamily() + { + return family; + } + + public String getSourceAddress() + { + return srcIP; + } + + public int getSourcePort() + { + return srcPort; + } + + public String getDestinationAddress() + { + return dstIP; + } + + public int getDestinationPort() + { + return dstPort; + } + + @Override + public ClientConnectionFactory apply(ClientConnectionFactory factory) + { + return new V1(factory); + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) + return true; + if (obj == null || getClass() != obj.getClass()) + return false; + Tag that = (Tag)obj; + return Objects.equals(family, that.family) && + Objects.equals(srcIP, that.srcIP) && + srcPort == that.srcPort && + Objects.equals(dstIP, that.dstIP) && + dstPort == that.dstPort; + } + + @Override + public int hashCode() + { + return Objects.hash(family, srcIP, srcPort, dstIP, dstPort); + } + } + } + + /** + * A ClientConnectionFactory for the PROXY protocol version 2. + */ + public static class V2 extends ProxyProtocolClientConnectionFactory + { + public V2(ClientConnectionFactory factory) + { + super(factory); + } + + @Override + protected ProxyProtocolConnection newProxyProtocolConnection(EndPoint endPoint, Map context) + { + HttpDestination destination = (HttpDestination)context.get(HttpClientTransport.HTTP_DESTINATION_CONTEXT_KEY); + Executor executor = destination.getHttpClient().getExecutor(); + Tag tag = (Tag)destination.getOrigin().getTag(); + if (tag == null) + { + InetSocketAddress local = endPoint.getLocalAddress(); + InetSocketAddress remote = endPoint.getRemoteAddress(); + boolean ipv4 = local.getAddress() instanceof Inet4Address; + tag = new Tag(Tag.Command.PROXY, ipv4 ? Tag.Family.INET4 : Tag.Family.INET6, Tag.Protocol.STREAM, local.getAddress().getHostAddress(), local.getPort(), remote.getAddress().getHostAddress(), remote.getPort()); + } + return new ProxyProtocolConnectionV2(endPoint, executor, getClientConnectionFactory(), context, tag); + } + + /** + *

PROXY protocol version 2 metadata holder to be used in conjunction + * with {@link org.eclipse.jetty.client.api.Request#tag(Object)}.

+ *

Instances of this class are associated to a destination so that + * all connections of that destination will initiate the communication + * with the PROXY protocol version 2 bytes specified by this metadata.

+ */ + public static class Tag implements ClientConnectionFactory.Decorator + { + /** + * The PROXY V2 Tag typically used to "ping" the server. + */ + public static final Tag LOCAL = new Tag(Command.LOCAL, Family.UNSPEC, Protocol.UNSPEC, null, 0, null, 0); + + private Command command; + private Family family; + private Protocol protocol; + private String srcIP; + private int srcPort; + private String dstIP; + private int dstPort; + private Map vectors; + + /** + *

Creates a Tag whose metadata will be derived from the underlying EndPoint.

+ */ + public Tag() + { + this(null, 0); + } + + /** + *

Creates a Tag with the given source metadata.

+ *

The destination metadata will be derived from the underlying EndPoint.

+ * + * @param srcIP the source IP address + * @param srcPort the source port + */ + public Tag(String srcIP, int srcPort) + { + this(Command.PROXY, null, Protocol.STREAM, srcIP, srcPort, null, 0); + } + + /** + *

Creates a Tag with the given metadata.

+ * + * @param command the LOCAL or PROXY command + * @param family the protocol family + * @param protocol the protocol type + * @param srcIP the source IP address + * @param srcPort the source port + * @param dstIP the destination IP address + * @param dstPort the destination port + */ + public Tag(Command command, Family family, Protocol protocol, String srcIP, int srcPort, String dstIP, int dstPort) + { + this.command = command; + this.family = family; + this.protocol = protocol; + this.srcIP = srcIP; + this.srcPort = srcPort; + this.dstIP = dstIP; + this.dstPort = dstPort; + } + + public void put(int type, byte[] data) + { + if (type < 0 || type > 255) + throw new IllegalArgumentException("Invalid type: " + type); + if (data != null && data.length > 65535) + throw new IllegalArgumentException("Invalid data length: " + data.length); + if (vectors == null) + vectors = new HashMap<>(); + vectors.put(type, data); + } + + public Command getCommand() + { + return command; + } + + public Family getFamily() + { + return family; + } + + public Protocol getProtocol() + { + return protocol; + } + + public String getSourceAddress() + { + return srcIP; + } + + public int getSourcePort() + { + return srcPort; + } + + public String getDestinationAddress() + { + return dstIP; + } + + public int getDestinationPort() + { + return dstPort; + } + + public Map getVectors() + { + return vectors != null ? vectors : Collections.emptyMap(); + } + + @Override + public ClientConnectionFactory apply(ClientConnectionFactory factory) + { + return new V2(factory); + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) + return true; + if (obj == null || getClass() != obj.getClass()) + return false; + Tag that = (Tag)obj; + return command == that.command && + family == that.family && + protocol == that.protocol && + Objects.equals(srcIP, that.srcIP) && + srcPort == that.srcPort && + Objects.equals(dstIP, that.dstIP) && + dstPort == that.dstPort; + } + + @Override + public int hashCode() + { + return Objects.hash(command, family, protocol, srcIP, srcPort, dstIP, dstPort); + } + + public enum Command + { + LOCAL, PROXY + } + + public enum Family + { + UNSPEC, INET4, INET6, UNIX + } + + public enum Protocol + { + UNSPEC, STREAM, DGRAM + } + } + } + + private final ClientConnectionFactory factory; + + private ProxyProtocolClientConnectionFactory(ClientConnectionFactory factory) + { + this.factory = factory; + } + + public ClientConnectionFactory getClientConnectionFactory() + { + return factory; + } + + @Override + public Connection newConnection(EndPoint endPoint, Map context) + { + ProxyProtocolConnection connection = newProxyProtocolConnection(endPoint, context); + return customize(connection, context); + } + + protected abstract ProxyProtocolConnection newProxyProtocolConnection(EndPoint endPoint, Map context); + + private abstract static class ProxyProtocolConnection extends AbstractConnection implements Callback + { + protected static final Logger LOG = Log.getLogger(ProxyProtocolConnection.class); + + private final ClientConnectionFactory factory; + private final Map context; + + private ProxyProtocolConnection(EndPoint endPoint, Executor executor, ClientConnectionFactory factory, Map context) + { + super(endPoint, executor); + this.factory = factory; + this.context = context; + } + + @Override + public void onOpen() + { + super.onOpen(); + writePROXYBytes(getEndPoint(), this); + } + + protected abstract void writePROXYBytes(EndPoint endPoint, Callback callback); + + @Override + public void succeeded() + { + try + { + EndPoint endPoint = getEndPoint(); + Connection connection = factory.newConnection(endPoint, context); + if (LOG.isDebugEnabled()) + LOG.debug("Written PROXY line, upgrading to {}", connection); + endPoint.upgrade(connection); + } + catch (Throwable x) + { + failed(x); + } + } + + @Override + public void failed(Throwable x) + { + close(); + Promise promise = (Promise)context.get(HttpClientTransport.HTTP_CONNECTION_PROMISE_CONTEXT_KEY); + promise.failed(x); + } + + @Override + public InvocationType getInvocationType() + { + return InvocationType.NON_BLOCKING; + } + + @Override + public void onFillable() + { + } + } + + private static class ProxyProtocolConnectionV1 extends ProxyProtocolConnection + { + private final V1.Tag tag; + + public ProxyProtocolConnectionV1(EndPoint endPoint, Executor executor, ClientConnectionFactory factory, Map context, V1.Tag tag) + { + super(endPoint, executor, factory, context); + this.tag = tag; + } + + @Override + protected void writePROXYBytes(EndPoint endPoint, Callback callback) + { + try + { + InetSocketAddress localAddress = endPoint.getLocalAddress(); + InetSocketAddress remoteAddress = endPoint.getRemoteAddress(); + String family = tag.getFamily(); + String srcIP = tag.getSourceAddress(); + int srcPort = tag.getSourcePort(); + String dstIP = tag.getDestinationAddress(); + int dstPort = tag.getDestinationPort(); + if (family == null) + family = localAddress.getAddress() instanceof Inet4Address ? "TCP4" : "TCP6"; + family = family.toUpperCase(Locale.ENGLISH); + boolean unknown = family.equals("UNKNOWN"); + StringBuilder builder = new StringBuilder(64); + builder.append("PROXY ").append(family); + if (!unknown) + { + if (srcIP == null) + srcIP = localAddress.getAddress().getHostAddress(); + builder.append(" ").append(srcIP); + if (dstIP == null) + dstIP = remoteAddress.getAddress().getHostAddress(); + builder.append(" ").append(dstIP); + if (srcPort <= 0) + srcPort = localAddress.getPort(); + builder.append(" ").append(srcPort); + if (dstPort <= 0) + dstPort = remoteAddress.getPort(); + builder.append(" ").append(dstPort); + } + builder.append("\r\n"); + String line = builder.toString(); + if (LOG.isDebugEnabled()) + LOG.debug("Writing PROXY bytes: {}", line.trim()); + ByteBuffer buffer = ByteBuffer.wrap(line.getBytes(StandardCharsets.US_ASCII)); + endPoint.write(callback, buffer); + } + catch (Throwable x) + { + callback.failed(x); + } + } + } + + private static class ProxyProtocolConnectionV2 extends ProxyProtocolConnection + { + private static final byte[] MAGIC = {0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, 0x55, 0x49, 0x54, 0x0A}; + + private final V2.Tag tag; + + public ProxyProtocolConnectionV2(EndPoint endPoint, Executor executor, ClientConnectionFactory factory, Map context, V2.Tag tag) + { + super(endPoint, executor, factory, context); + this.tag = tag; + } + + @Override + protected void writePROXYBytes(EndPoint endPoint, Callback callback) + { + try + { + int capacity = MAGIC.length; + capacity += 1; // version and command + capacity += 1; // family and protocol + capacity += 2; // length + capacity += 216; // max address length + Map vectors = tag.getVectors(); + int vectorsLength = vectors.values().stream() + .mapToInt(data -> 1 + 2 + data.length) + .sum(); + capacity += vectorsLength; + ByteBuffer buffer = ByteBuffer.allocateDirect(capacity); + buffer.put(MAGIC); + V2.Tag.Command command = tag.getCommand(); + int versionAndCommand = (2 << 4) | (command.ordinal() & 0x0F); + buffer.put((byte)versionAndCommand); + V2.Tag.Family family = tag.getFamily(); + String srcAddr = tag.getSourceAddress(); + if (srcAddr == null) + srcAddr = endPoint.getLocalAddress().getAddress().getHostAddress(); + int srcPort = tag.getSourcePort(); + if (srcPort <= 0) + srcPort = endPoint.getLocalAddress().getPort(); + if (family == null) + family = InetAddress.getByName(srcAddr) instanceof Inet4Address ? V2.Tag.Family.INET4 : V2.Tag.Family.INET6; + V2.Tag.Protocol protocol = tag.getProtocol(); + if (protocol == null) + protocol = V2.Tag.Protocol.STREAM; + int familyAndProtocol = (family.ordinal() << 4) | protocol.ordinal(); + buffer.put((byte)familyAndProtocol); + int length = 0; + switch (family) + { + case UNSPEC: + break; + case INET4: + length = 12; + break; + case INET6: + length = 36; + break; + case UNIX: + length = 216; + break; + default: + throw new IllegalStateException(); + } + length += vectorsLength; + buffer.putShort((short)length); + String dstAddr = tag.getDestinationAddress(); + if (dstAddr == null) + dstAddr = endPoint.getRemoteAddress().getAddress().getHostAddress(); + int dstPort = tag.getDestinationPort(); + if (dstPort <= 0) + dstPort = endPoint.getRemoteAddress().getPort(); + switch (family) + { + case UNSPEC: + break; + case INET4: + case INET6: + buffer.put(InetAddress.getByName(srcAddr).getAddress()); + buffer.put(InetAddress.getByName(dstAddr).getAddress()); + buffer.putShort((short)srcPort); + buffer.putShort((short)dstPort); + break; + case UNIX: + int position = buffer.position(); + buffer.put(srcAddr.getBytes(StandardCharsets.US_ASCII)); + buffer.position(position + 108); + buffer.put(dstAddr.getBytes(StandardCharsets.US_ASCII)); + break; + default: + throw new IllegalStateException(); + } + for (Map.Entry entry : vectors.entrySet()) + { + buffer.put(entry.getKey().byteValue()); + byte[] data = entry.getValue(); + buffer.putShort((short)data.length); + buffer.put(data); + } + buffer.flip(); + endPoint.write(callback, buffer); + } + catch (Throwable x) + { + callback.failed(x); + } + } + } +} diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java index 83774e328205..a6a47d102dc9 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java @@ -180,6 +180,28 @@ public interface Request */ Request cookie(HttpCookie cookie); + /** + *

Tags this request with the given metadata tag.

+ *

Each different tag will create a different destination, + * even if the destination origin is the same.

+ *

This is particularly useful in proxies, where requests + * for the same origin but from different clients may be tagged + * with client's metadata (e.g. the client remote address).

+ *

The tag metadata class must correctly implement + * {@link Object#hashCode()} and {@link Object#equals(Object)} + * so that it can be used, along with the origin, to identify + * a destination.

+ * + * @param tag the metadata to tag the request with + * @return this request object + */ + Request tag(Object tag); + + /** + * @return the metadata this request has been tagged with + */ + Object getTag(); + /** * @param name the name of the attribute * @param value the value of the attribute diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java new file mode 100644 index 000000000000..306c04cfd5e6 --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java @@ -0,0 +1,264 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.client; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.concurrent.ThreadLocalRandom; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.api.Destination; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.ProxyConnectionFactory; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V1; +import static org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V2; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class HttpClientProxyProtocolTest +{ + private Server server; + private ServerConnector connector; + private HttpClient client; + + private void startServer(Handler handler) throws Exception + { + QueuedThreadPool serverThreads = new QueuedThreadPool(); + serverThreads.setName("server"); + server = new Server(serverThreads); + HttpConnectionFactory http = new HttpConnectionFactory(); + ProxyConnectionFactory proxy = new ProxyConnectionFactory(http.getProtocol()); + connector = new ServerConnector(server, 1, 1, proxy, http); + server.addConnector(connector); + server.setHandler(handler); + server.start(); + } + + private void startClient() throws Exception + { + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + client = new HttpClient(); + client.setExecutor(clientThreads); + client.setRemoveIdleDestinations(false); + client.start(); + } + + @AfterEach + public void dispose() throws Exception + { + if (server != null) + server.stop(); + if (client != null) + client.stop(); + } + + @Test + public void testClientProxyProtocolV1() throws Exception + { + startServer(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); + response.getOutputStream().print(request.getRemotePort()); + } + }); + startClient(); + + int serverPort = connector.getLocalPort(); + + int clientPort = ThreadLocalRandom.current().nextInt(1024, 65536); + V1.Tag tag = new V1.Tag("127.0.0.1", clientPort); + + ContentResponse response = client.newRequest("localhost", serverPort) + .tag(tag) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort), response.getContentAsString()); + } + + @Test + public void testClientProxyProtocolV1Unknown() throws Exception + { + startServer(new EmptyServerHandler()); + startClient(); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .tag(V1.Tag.UNKNOWN) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + } + + @Test + public void testClientProxyProtocolV2() throws Exception + { + startServer(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); + response.getOutputStream().print(request.getRemotePort()); + } + }); + startClient(); + + int serverPort = connector.getLocalPort(); + + int clientPort = ThreadLocalRandom.current().nextInt(1024, 65536); + V2.Tag tag = new V2.Tag("127.0.0.1", clientPort); + + ContentResponse response = client.newRequest("localhost", serverPort) + .tag(tag) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort), response.getContentAsString()); + } + + @Test + public void testClientProxyProtocolV2Local() throws Exception + { + startServer(new EmptyServerHandler()); + startClient(); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .tag(V2.Tag.LOCAL) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + } + + @Test + public void testClientProxyProtocolV2WithVectors() throws Exception + { + String tlsVersion = "TLSv1.3"; + byte[] tlsVersionBytes = tlsVersion.getBytes(StandardCharsets.US_ASCII); + startServer(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + EndPoint endPoint = jettyRequest.getHttpChannel().getEndPoint(); + assertTrue(endPoint instanceof ProxyConnectionFactory.ProxyEndPoint); + ProxyConnectionFactory.ProxyEndPoint proxyEndPoint = (ProxyConnectionFactory.ProxyEndPoint)endPoint; + assertEquals(tlsVersion, proxyEndPoint.getAttribute(ProxyConnectionFactory.TLS_VERSION)); + response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); + response.getOutputStream().print(request.getRemotePort()); + } + }); + startClient(); + + int serverPort = connector.getLocalPort(); + + int clientPort = ThreadLocalRandom.current().nextInt(1024, 65536); + V2.Tag tag = new V2.Tag("127.0.0.1", clientPort); + int typeTLS = 0x20; + byte[] dataTLS = new byte[1 + 4 + (1 + 2 + tlsVersionBytes.length)]; + dataTLS[0] = 0x01; // CLIENT_SSL + dataTLS[5] = 0x21; // SUBTYPE_SSL_VERSION + dataTLS[6] = 0x00; // Length, hi byte + dataTLS[7] = (byte)tlsVersionBytes.length; // Length, lo byte + System.arraycopy(tlsVersionBytes, 0, dataTLS, 8, tlsVersionBytes.length); + tag.put(typeTLS, dataTLS); + + ContentResponse response = client.newRequest("localhost", serverPort) + .tag(tag) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort), response.getContentAsString()); + } + + @Test + public void testProxyProtocolWrappingHTTPProxy() throws Exception + { + startServer(new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); + response.getOutputStream().print(request.getRemotePort()); + } + }); + startClient(); + + int proxyPort = connector.getLocalPort(); + int serverPort = proxyPort + 1; // Any port will do. + client.getProxyConfiguration().getProxies().add(new HttpProxy("localhost", proxyPort)); + + // We are simulating to be a HttpClient inside a proxy. + // The server is configured with the PROXY protocol to know the socket address of clients. + + // The proxy receives a request from the client, and it extracts the client address. + int clientPort = ThreadLocalRandom.current().nextInt(1024, 65536); + V1.Tag tag = new V1.Tag("127.0.0.1", clientPort); + + // The proxy maps the client address, then sends the request. + ContentResponse response = client.newRequest("localhost", serverPort) + .tag(tag) + .header(HttpHeader.CONNECTION, "close") + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort), response.getContentAsString()); + List destinations = client.getDestinations(); + assertEquals(1, destinations.size()); + HttpDestination destination = (HttpDestination)destinations.get(0); + assertTrue(destination.getConnectionPool().isEmpty()); + + // The previous connection has been closed. + // Make another request from the same client address. + response = client.newRequest("localhost", serverPort) + .tag(tag) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort), response.getContentAsString()); + destinations = client.getDestinations(); + assertEquals(1, destinations.size()); + assertSame(destination, destinations.get(0)); + + // Make another request from a different client address. + int clientPort2 = clientPort + 1; + V1.Tag tag2 = new V1.Tag("127.0.0.1", clientPort2); + response = client.newRequest("localhost", serverPort) + .tag(tag2) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort2), response.getContentAsString()); + destinations = client.getDestinations(); + assertEquals(2, destinations.size()); + } +} diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ClientConnectionFactory.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ClientConnectionFactory.java index 9cddb8e6b92c..cb626322c56d 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ClientConnectionFactory.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ClientConnectionFactory.java @@ -44,4 +44,21 @@ default Connection customize(Connection connection, Map context) connector.getBeans(Connection.Listener.class).forEach(connection::addListener); return connection; } + + /** + *

Wraps another ClientConnectionFactory.

+ *

This is typically done by protocols that send "preface" bytes with some metadata + * before other protocols. The metadata could be, for example, proxying information + * or authentication information.

+ */ + interface Decorator + { + /** + *

Wraps the given {@code factory}.

+ * + * @param factory the ClientConnectionFactory to wrap + * @return the wrapping ClientConnectionFactory + */ + ClientConnectionFactory apply(ClientConnectionFactory factory); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java index b52f77949ea8..e38732f354c3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java @@ -102,9 +102,10 @@ public Connection newConnection(Connector connector, EndPoint endp) public class ProxyProtocolV1orV2Connection extends AbstractConnection { + // Only do a tiny read to figure out what PROXY version it is. + private final ByteBuffer _buffer = BufferUtil.allocate(16); private final Connector _connector; private final String _next; - private ByteBuffer _buffer = BufferUtil.allocate(16); protected ProxyProtocolV1orV2Connection(EndPoint endp, Connector connector, String next) { @@ -157,8 +158,11 @@ public void onFillable() return; } default: + { LOG.warn("Not PROXY protocol for {}", getEndPoint()); close(); + break; + } } } catch (Throwable x) @@ -179,8 +183,8 @@ public static class ProxyProtocolV1Connection extends AbstractConnection private final Connector _connector; private final String _next; private final StringBuilder _builder = new StringBuilder(); - private final String[] _field = new String[6]; - private int _fields; + private final String[] _fields = new String[6]; + private int _index; private int _length; protected ProxyProtocolV1Connection(EndPoint endp, Connector connector, String next, ByteBuffer buffer) @@ -201,16 +205,18 @@ public void onOpen() private boolean parse(ByteBuffer buffer) { - // parse fields + // Parse fields while (buffer.hasRemaining()) { byte b = buffer.get(); - if (_fields < 6) + if (_index < 6) { - if (b == ' ' || b == '\r' && _fields == 5) + if (b == ' ' || b == '\r') { - _field[_fields++] = _builder.toString(); + _fields[_index++] = _builder.toString(); _builder.setLength(0); + if (b == '\r') + _index = 6; } else if (b < ' ') { @@ -227,7 +233,7 @@ else if (b < ' ') { if (b == '\n') { - _fields = 7; + _index = 7; return true; } @@ -245,12 +251,12 @@ public void onFillable() try { ByteBuffer buffer = null; - while (_fields < 7) + while (_index < 7) { // Create a buffer that will not read too much data // since once read it is impossible to push back for the // real connection to read it. - int size = Math.max(1, SIZE[_fields] - _builder.length()); + int size = Math.max(1, SIZE[_index] - _builder.length()); if (buffer == null || buffer.capacity() != size) buffer = BufferUtil.allocate(size); else @@ -282,22 +288,34 @@ public void onFillable() } // Check proxy - if (!"PROXY".equals(_field[0])) + if (!"PROXY".equals(_fields[0])) { LOG.warn("Not PROXY protocol for {}", getEndPoint()); close(); return; } - // Extract Addresses - InetSocketAddress remote = new InetSocketAddress(_field[2], Integer.parseInt(_field[4])); - InetSocketAddress local = new InetSocketAddress(_field[3], Integer.parseInt(_field[5])); + String srcIP = _fields[2]; + String srcPort = _fields[4]; + String dstIP = _fields[3]; + String dstPort = _fields[5]; + // If UNKNOWN, we must ignore the information sent, so use the EndPoint's. + boolean unknown = "UNKNOWN".equalsIgnoreCase(_fields[1]); + if (unknown) + { + srcIP = getEndPoint().getRemoteAddress().getAddress().getHostAddress(); + srcPort = String.valueOf(getEndPoint().getRemoteAddress().getPort()); + dstIP = getEndPoint().getLocalAddress().getAddress().getHostAddress(); + dstPort = String.valueOf(getEndPoint().getLocalAddress().getPort()); + } + InetSocketAddress remote = new InetSocketAddress(srcIP, Integer.parseInt(srcPort)); + InetSocketAddress local = new InetSocketAddress(dstIP, Integer.parseInt(dstPort)); // Create the next protocol ConnectionFactory connectionFactory = _connector.getConnectionFactory(_next); if (connectionFactory == null) { - LOG.warn("No Next protocol '{}' for {}", _next, getEndPoint()); + LOG.warn("No next protocol '{}' for {}", _next, getEndPoint()); close(); return; } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java index ce2f7493f2be..97c35d352215 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java @@ -30,9 +30,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertNull; -/** - * - */ public class ProxyConnectionTest { private Server _server; @@ -85,7 +82,7 @@ public void testSimple() throws Exception public void testIPv6() throws Exception { Assumptions.assumeTrue(Net.isIpv6InterfaceAvailable()); - String response = _connector.getResponse("PROXY UNKNOWN eeee:eeee:eeee:eeee:eeee:eeee:eeee:eeee ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff 65535 65535\r\n" + + String response = _connector.getResponse("PROXY TCP6 eeee:eeee:eeee:eeee:eeee:eeee:eeee:eeee ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff 65535 65535\r\n" + "GET /path HTTP/1.1\n" + "Host: server:80\n" + "Connection: close\n" + diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java index 256cc1969022..7dd918554f1a 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.InterruptedIOException; +import java.util.List; import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -35,6 +36,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.api.Destination; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.BytesContentProvider; import org.eclipse.jetty.client.util.FutureResponseListener; @@ -650,6 +652,30 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t assertEquals(0, response.getContent().length); } + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testOneDestinationPerUser(Transport transport) throws Exception + { + init(transport); + scenario.start(new EmptyServerHandler()); + + int runs = 4; + int users = 16; + for (int i = 0; i < runs; ++i) + { + for (int j = 0; j < users; ++j) + { + ContentResponse response = scenario.client.newRequest(scenario.newURI()) + .tag(j) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + } + } + + List destinations = scenario.client.getDestinations(); + assertEquals(users, destinations.size()); + } + private void sleep(long time) throws IOException { try From bea7f1a5cfbf45a391879f1d283bd21df09fc4a4 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 17 Dec 2019 23:26:28 +0100 Subject: [PATCH 16/16] Fixes #4421 - HttpClient support for PROXY protocol. Improved support for Type-Length-Value (TLV) objects. Signed-off-by: Simone Bordet --- .../ProxyProtocolClientConnectionFactory.java | 113 +++++++++++++----- .../client/HttpClientProxyProtocolTest.java | 21 +++- 2 files changed, 101 insertions(+), 33 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java index b371e5a5a2d7..fac784d5bcf4 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java @@ -23,8 +23,8 @@ import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import java.util.Collections; -import java.util.HashMap; +import java.util.Arrays; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -206,7 +206,7 @@ protected ProxyProtocolConnection newProxyProtocolConnection(EndPoint endPoint, InetSocketAddress local = endPoint.getLocalAddress(); InetSocketAddress remote = endPoint.getRemoteAddress(); boolean ipv4 = local.getAddress() instanceof Inet4Address; - tag = new Tag(Tag.Command.PROXY, ipv4 ? Tag.Family.INET4 : Tag.Family.INET6, Tag.Protocol.STREAM, local.getAddress().getHostAddress(), local.getPort(), remote.getAddress().getHostAddress(), remote.getPort()); + tag = new Tag(Tag.Command.PROXY, ipv4 ? Tag.Family.INET4 : Tag.Family.INET6, Tag.Protocol.STREAM, local.getAddress().getHostAddress(), local.getPort(), remote.getAddress().getHostAddress(), remote.getPort(), null); } return new ProxyProtocolConnectionV2(endPoint, executor, getClientConnectionFactory(), context, tag); } @@ -223,7 +223,7 @@ public static class Tag implements ClientConnectionFactory.Decorator /** * The PROXY V2 Tag typically used to "ping" the server. */ - public static final Tag LOCAL = new Tag(Command.LOCAL, Family.UNSPEC, Protocol.UNSPEC, null, 0, null, 0); + public static final Tag LOCAL = new Tag(Command.LOCAL, Family.UNSPEC, Protocol.UNSPEC, null, 0, null, 0, null); private Command command; private Family family; @@ -232,7 +232,7 @@ public static class Tag implements ClientConnectionFactory.Decorator private int srcPort; private String dstIP; private int dstPort; - private Map vectors; + private List tlvs; /** *

Creates a Tag whose metadata will be derived from the underlying EndPoint.

@@ -251,7 +251,20 @@ public Tag() */ public Tag(String srcIP, int srcPort) { - this(Command.PROXY, null, Protocol.STREAM, srcIP, srcPort, null, 0); + this(Command.PROXY, null, Protocol.STREAM, srcIP, srcPort, null, 0, null); + } + + /** + *

Creates a Tag with the given source metadata and Type-Length-Value (TLV) objects.

+ *

The destination metadata will be derived from the underlying EndPoint.

+ * + * @param srcIP the source IP address + * @param srcPort the source port + * @param tlvs the TLV objects + */ + public Tag(String srcIP, int srcPort, List tlvs) + { + this(Command.PROXY, null, Protocol.STREAM, srcIP, srcPort, null, 0, tlvs); } /** @@ -264,8 +277,9 @@ public Tag(String srcIP, int srcPort) * @param srcPort the source port * @param dstIP the destination IP address * @param dstPort the destination port + * @param tlvs the TLV objects */ - public Tag(Command command, Family family, Protocol protocol, String srcIP, int srcPort, String dstIP, int dstPort) + public Tag(Command command, Family family, Protocol protocol, String srcIP, int srcPort, String dstIP, int dstPort, List tlvs) { this.command = command; this.family = family; @@ -274,17 +288,7 @@ public Tag(Command command, Family family, Protocol protocol, String srcIP, int this.srcPort = srcPort; this.dstIP = dstIP; this.dstPort = dstPort; - } - - public void put(int type, byte[] data) - { - if (type < 0 || type > 255) - throw new IllegalArgumentException("Invalid type: " + type); - if (data != null && data.length > 65535) - throw new IllegalArgumentException("Invalid data length: " + data.length); - if (vectors == null) - vectors = new HashMap<>(); - vectors.put(type, data); + this.tlvs = tlvs; } public Command getCommand() @@ -322,9 +326,9 @@ public int getDestinationPort() return dstPort; } - public Map getVectors() + public List getTLVs() { - return vectors != null ? vectors : Collections.emptyMap(); + return tlvs; } @Override @@ -347,13 +351,14 @@ public boolean equals(Object obj) Objects.equals(srcIP, that.srcIP) && srcPort == that.srcPort && Objects.equals(dstIP, that.dstIP) && - dstPort == that.dstPort; + dstPort == that.dstPort && + Objects.equals(tlvs, that.tlvs); } @Override public int hashCode() { - return Objects.hash(command, family, protocol, srcIP, srcPort, dstIP, dstPort); + return Objects.hash(command, family, protocol, srcIP, srcPort, dstIP, dstPort, tlvs); } public enum Command @@ -370,6 +375,51 @@ public enum Protocol { UNSPEC, STREAM, DGRAM } + + public static class TLV + { + private final int type; + private final byte[] value; + + public TLV(int type, byte[] value) + { + if (type < 0 || type > 255) + throw new IllegalArgumentException("Invalid type: " + type); + if (value != null && value.length > 65535) + throw new IllegalArgumentException("Invalid value length: " + value.length); + this.type = type; + this.value = Objects.requireNonNull(value); + } + + public int getType() + { + return type; + } + + public byte[] getValue() + { + return value; + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) + return true; + if (obj == null || getClass() != obj.getClass()) + return false; + TLV that = (TLV)obj; + return type == that.type && Arrays.equals(value, that.value); + } + + @Override + public int hashCode() + { + int result = Objects.hash(type); + result = 31 * result + Arrays.hashCode(value); + return result; + } + } } } @@ -533,9 +583,9 @@ protected void writePROXYBytes(EndPoint endPoint, Callback callback) capacity += 1; // family and protocol capacity += 2; // length capacity += 216; // max address length - Map vectors = tag.getVectors(); - int vectorsLength = vectors.values().stream() - .mapToInt(data -> 1 + 2 + data.length) + List tlvs = tag.getTLVs(); + int vectorsLength = tlvs == null ? 0 : tlvs.stream() + .mapToInt(tlv -> 1 + 2 + tlv.getValue().length) .sum(); capacity += vectorsLength; ByteBuffer buffer = ByteBuffer.allocateDirect(capacity); @@ -602,12 +652,15 @@ protected void writePROXYBytes(EndPoint endPoint, Callback callback) default: throw new IllegalStateException(); } - for (Map.Entry entry : vectors.entrySet()) + if (tlvs != null) { - buffer.put(entry.getKey().byteValue()); - byte[] data = entry.getValue(); - buffer.putShort((short)data.length); - buffer.put(data); + for (V2.Tag.TLV tlv : tlvs) + { + buffer.put((byte)tlv.getType()); + byte[] data = tlv.getValue(); + buffer.putShort((short)data.length); + buffer.put(data); + } } buffer.flip(); endPoint.write(callback, buffer); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java index 306c04cfd5e6..6b5e254b8f14 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.List; import java.util.concurrent.ThreadLocalRandom; import javax.servlet.http.HttpServletRequest; @@ -174,7 +175,8 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r EndPoint endPoint = jettyRequest.getHttpChannel().getEndPoint(); assertTrue(endPoint instanceof ProxyConnectionFactory.ProxyEndPoint); ProxyConnectionFactory.ProxyEndPoint proxyEndPoint = (ProxyConnectionFactory.ProxyEndPoint)endPoint; - assertEquals(tlsVersion, proxyEndPoint.getAttribute(ProxyConnectionFactory.TLS_VERSION)); + if (target.equals("/tls_version")) + assertEquals(tlsVersion, proxyEndPoint.getAttribute(ProxyConnectionFactory.TLS_VERSION)); response.setContentType(MimeTypes.Type.TEXT_PLAIN.asString()); response.getOutputStream().print(request.getRemotePort()); } @@ -184,7 +186,6 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r int serverPort = connector.getLocalPort(); int clientPort = ThreadLocalRandom.current().nextInt(1024, 65536); - V2.Tag tag = new V2.Tag("127.0.0.1", clientPort); int typeTLS = 0x20; byte[] dataTLS = new byte[1 + 4 + (1 + 2 + tlsVersionBytes.length)]; dataTLS[0] = 0x01; // CLIENT_SSL @@ -192,13 +193,27 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r dataTLS[6] = 0x00; // Length, hi byte dataTLS[7] = (byte)tlsVersionBytes.length; // Length, lo byte System.arraycopy(tlsVersionBytes, 0, dataTLS, 8, tlsVersionBytes.length); - tag.put(typeTLS, dataTLS); + V2.Tag.TLV tlv = new V2.Tag.TLV(typeTLS, dataTLS); + V2.Tag tag = new V2.Tag("127.0.0.1", clientPort, Collections.singletonList(tlv)); ContentResponse response = client.newRequest("localhost", serverPort) + .path("/tls_version") .tag(tag) .send(); assertEquals(HttpStatus.OK_200, response.getStatus()); assertEquals(String.valueOf(clientPort), response.getContentAsString()); + + // Make another request with the same address information, but different TLV. + V2.Tag.TLV tlv2 = new V2.Tag.TLV(0x01, "http/1.1".getBytes(StandardCharsets.UTF_8)); + V2.Tag tag2 = new V2.Tag("127.0.0.1", clientPort, Collections.singletonList(tlv2)); + response = client.newRequest("localhost", serverPort) + .tag(tag2) + .send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals(String.valueOf(clientPort), response.getContentAsString()); + + // Make sure the two TLVs created two destinations. + assertEquals(2, client.getDestinations().size()); } @Test