From efda64631919479ac8c1bab033cbeb47a4363bfc Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 13 Dec 2023 20:34:52 +0100 Subject: [PATCH] =?UTF-8?q?Fixes=20#11031=20-=20HttpClient=20should=20expo?= =?UTF-8?q?se=20Connection/EndPoint=20used=20by=20H=E2=80=A6=20(#11033)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduced method Request.getConnection() to expose the Connection after at the request begin event. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpChannel.java | 13 ++++++--- .../org/eclipse/jetty/client/HttpProxy.java | 13 +++++++++ .../org/eclipse/jetty/client/HttpRequest.java | 13 +++++++++ .../eclipse/jetty/client/api/Connection.java | 17 +++++++++++ .../org/eclipse/jetty/client/api/Request.java | 16 ++++++++++ .../client/http/HttpChannelOverHTTP.java | 7 +++++ .../client/http/HttpConnectionOverHTTP.java | 25 ++++++++++++++++ .../jetty/client/HttpClientFailureTest.java | 25 +++++----------- .../fcgi/client/http/HttpChannelOverFCGI.java | 9 +++++- .../client/http/HttpConnectionOverFCGI.java | 29 +++++++++++++++++-- .../client/http/HttpChannelOverHTTP2.java | 7 +++++ .../client/http/HttpConnectionOverHTTP2.java | 13 +++++++++ .../http/internal/HttpChannelOverHTTP3.java | 7 +++++ .../internal/HttpConnectionOverHTTP3.java | 13 +++++++++ .../jetty/http/client/HttpClientTest.java | 19 ++++++++++++ 15 files changed, 202 insertions(+), 24 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java index 2d2181b77c79..8f1255d7975f 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.client; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.io.CyclicTimeouts; import org.eclipse.jetty.util.thread.AutoLock; @@ -53,7 +54,7 @@ public boolean associate(HttpExchange exchange) { boolean result = false; boolean abort = true; - try (AutoLock l = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { if (_exchange == null) { @@ -64,12 +65,14 @@ public boolean associate(HttpExchange exchange) } } + HttpRequest request = exchange.getRequest(); if (abort) { - exchange.getRequest().abort(new UnsupportedOperationException("Pipelined requests not supported")); + request.abort(new UnsupportedOperationException("Pipelined requests not supported")); } else { + request.setConnection(getConnection()); if (LOG.isDebugEnabled()) LOG.debug("{} associated {} to {}", exchange, result, this); } @@ -80,7 +83,7 @@ public boolean associate(HttpExchange exchange) public boolean disassociate(HttpExchange exchange) { boolean result = false; - try (AutoLock l = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { HttpExchange existing = _exchange; _exchange = null; @@ -98,12 +101,14 @@ public boolean disassociate(HttpExchange exchange) public HttpExchange getHttpExchange() { - try (AutoLock l = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { return _exchange; } } + protected abstract Connection getConnection(); + @Override public long getExpireNanoTime() { 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 5ee566e2c8f4..580766ab32a7 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 @@ -15,6 +15,7 @@ import java.io.IOException; import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.net.URI; import java.util.List; import java.util.Map; @@ -277,6 +278,18 @@ private ProxyConnection(Destination destination, Connection connection, Promise< this.promise = promise; } + @Override + public SocketAddress getLocalSocketAddress() + { + return connection.getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return connection.getRemoteSocketAddress(); + } + @Override public void send(Request request, 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 1179a3bd8c35..b9bdcb9acd2b 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 @@ -41,6 +41,7 @@ import java.util.function.LongConsumer; import java.util.function.Supplier; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; @@ -69,6 +70,7 @@ public class HttpRequest implements Request private final AtomicReference aborted = new AtomicReference<>(); private final HttpClient client; private final HttpConversation conversation; + private Connection connection; private String scheme; private String host; private int port; @@ -162,6 +164,17 @@ public HttpConversation getConversation() return conversation; } + @Override + public Connection getConnection() + { + return connection; + } + + void setConnection(Connection connection) + { + this.connection = connection; + } + @Override public String getScheme() { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Connection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Connection.java index a35dab8a0b0d..d557066f49b1 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Connection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Connection.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.client.api; import java.io.Closeable; +import java.net.SocketAddress; import org.eclipse.jetty.util.Promise; @@ -46,4 +47,20 @@ public interface Connection extends Closeable * @see #close() */ boolean isClosed(); + + /** + * @return the local socket address associated with the connection + */ + default SocketAddress getLocalSocketAddress() + { + return null; + } + + /** + * @return the remote socket address associated with the connection + */ + default SocketAddress getRemoteSocketAddress() + { + return null; + } } 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 1766b0a35510..bc339616912e 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 @@ -50,6 +50,22 @@ */ public interface Request { + /** + *

Returns the connection associated with this request.

+ *

The connection is available only starting from the + * {@link #onRequestBegin(BeginListener) request begin} event, + * when a connection is associated with the request to be sent, + * otherwise {@code null} is returned.

+ * + * @return the connection associated with this request, + * or {@code null} if there is no connection associated + * with this request + */ + default Connection getConnection() + { + return null; + } + /** * @return the URI scheme of this request, such as "http" or "https" */ diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java index c4cffd0bb1a6..245557d7dd4a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpChannelOverHTTP.java @@ -17,6 +17,7 @@ import org.eclipse.jetty.client.HttpChannel; import org.eclipse.jetty.client.HttpExchange; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http.HttpFields; @@ -55,6 +56,12 @@ protected HttpReceiverOverHTTP newHttpReceiver() return new HttpReceiverOverHTTP(this); } + @Override + protected Connection getConnection() + { + return connection; + } + @Override protected HttpSenderOverHTTP getHttpSender() { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java index e0698be8d4f1..980b6d1c4d05 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.client.http; +import java.net.SocketAddress; import java.nio.ByteBuffer; import java.nio.channels.AsynchronousCloseException; import java.util.Collections; @@ -100,6 +101,18 @@ public HttpDestination getHttpDestination() return delegate.getHttpDestination(); } + @Override + public SocketAddress getLocalSocketAddress() + { + return delegate.getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return delegate.getRemoteSocketAddress(); + } + @Override public long getBytesIn() { @@ -285,6 +298,18 @@ protected Iterator getHttpChannels() return Collections.singleton(channel).iterator(); } + @Override + public SocketAddress getLocalSocketAddress() + { + return getEndPoint().getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return getEndPoint().getRemoteSocketAddress(); + } + @Override public SendFailure send(HttpExchange exchange) { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientFailureTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientFailureTest.java index ecfac7319ae9..77ef8603e549 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientFailureTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientFailureTest.java @@ -21,6 +21,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.http.HttpConnectionOverHTTP; import org.eclipse.jetty.client.util.AsyncRequestContent; @@ -68,26 +69,16 @@ public void testFailureBeforeRequestCommit() throws Exception { startServer(new EmptyServerHandler()); - final AtomicReference connectionRef = new AtomicReference<>(); - client = new HttpClient(new HttpClientTransportOverHTTP(1) - { - @Override - public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map context) throws IOException - { - HttpConnectionOverHTTP connection = (HttpConnectionOverHTTP)super.newConnection(endPoint, context); - connectionRef.set(connection); - return connection; - } - }); + client = new HttpClient(new HttpClientTransportOverHTTP(1)); client.start(); - assertThrows(ExecutionException.class, () -> - client.newRequest("localhost", connector.getLocalPort()) - .onRequestHeaders(request -> connectionRef.get().getEndPoint().close()) - .timeout(5, TimeUnit.SECONDS) - .send()); + Request request = client.newRequest("localhost", connector.getLocalPort()) + .onRequestHeaders(r -> r.getConnection().close()) + .timeout(5, TimeUnit.SECONDS); + assertThrows(ExecutionException.class, request::send); - DuplexConnectionPool connectionPool = (DuplexConnectionPool)connectionRef.get().getHttpDestination().getConnectionPool(); + HttpDestination destination = (HttpDestination)client.resolveDestination(request); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); assertEquals(0, connectionPool.getConnectionCount()); assertEquals(0, connectionPool.getActiveConnections().size()); assertEquals(0, connectionPool.getIdleConnections().size()); diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java index 344891b06ed3..f7b97fec24f0 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java @@ -20,6 +20,7 @@ import org.eclipse.jetty.client.HttpExchange; import org.eclipse.jetty.client.HttpReceiver; import org.eclipse.jetty.client.HttpSender; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.fcgi.generator.Flusher; import org.eclipse.jetty.fcgi.generator.Generator; @@ -43,7 +44,7 @@ public class HttpChannelOverFCGI extends HttpChannel private int request; private HttpVersion version; - public HttpChannelOverFCGI(final HttpConnectionOverFCGI connection, Flusher flusher, long idleTimeout) + public HttpChannelOverFCGI(HttpConnectionOverFCGI connection, Flusher flusher, long idleTimeout) { super(connection.getHttpDestination()); this.connection = connection; @@ -63,6 +64,12 @@ void setRequest(int request) this.request = request; } + @Override + protected Connection getConnection() + { + return connection; + } + @Override protected HttpSender getHttpSender() { diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java index 7fbbb063339b..d6ad5326b1fb 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.fcgi.client.http; import java.io.EOFException; +import java.net.SocketAddress; import java.nio.ByteBuffer; import java.nio.channels.AsynchronousCloseException; import java.util.Collections; @@ -87,6 +88,18 @@ public HttpDestination getHttpDestination() return destination; } + @Override + public SocketAddress getLocalSocketAddress() + { + return delegate.getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return delegate.getRemoteSocketAddress(); + } + protected Flusher getFlusher() { return flusher; @@ -319,7 +332,7 @@ private void failAndClose(Throwable failure) private int acquireRequest() { - try (AutoLock l = lock.lock()) + try (AutoLock ignored = lock.lock()) { int last = requests.getLast(); int request = last + 1; @@ -330,7 +343,7 @@ private int acquireRequest() private void releaseRequest(int request) { - try (AutoLock l = lock.lock()) + try (AutoLock ignored = lock.lock()) { requests.removeFirstOccurrence(request); } @@ -373,6 +386,18 @@ protected Iterator getHttpChannels() return channel == null ? Collections.emptyIterator() : Collections.singleton(channel).iterator(); } + @Override + public SocketAddress getLocalSocketAddress() + { + return getEndPoint().getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return getEndPoint().getRemoteSocketAddress(); + } + @Override public SendFailure send(HttpExchange exchange) { diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java index 9a6dda2ed64b..6960ff9f2c35 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java @@ -18,6 +18,7 @@ import org.eclipse.jetty.client.HttpExchange; import org.eclipse.jetty.client.HttpReceiver; import org.eclipse.jetty.client.HttpSender; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.HTTP2Channel; @@ -67,6 +68,12 @@ public Stream.Listener getStreamListener() return listener; } + @Override + protected Connection getConnection() + { + return connection; + } + @Override protected HttpSender getHttpSender() { diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java index 2aa5324731f5..097ba74b886b 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http2.client.http; +import java.net.SocketAddress; import java.nio.channels.AsynchronousCloseException; import java.util.Iterator; import java.util.List; @@ -68,6 +69,18 @@ public Session getSession() return session; } + @Override + public SocketAddress getLocalSocketAddress() + { + return session.getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return session.getRemoteSocketAddress(); + } + public boolean isRecycleHttpChannels() { return recycleHttpChannels; diff --git a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpChannelOverHTTP3.java b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpChannelOverHTTP3.java index 1aaa2c784642..34225c459580 100644 --- a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpChannelOverHTTP3.java +++ b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpChannelOverHTTP3.java @@ -18,6 +18,7 @@ import org.eclipse.jetty.client.HttpExchange; import org.eclipse.jetty.client.HttpReceiver; import org.eclipse.jetty.client.HttpSender; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http3.api.Stream; import org.eclipse.jetty.http3.client.internal.HTTP3SessionClient; @@ -50,6 +51,12 @@ public Stream.Client.Listener getStreamListener() return receiver; } + @Override + protected Connection getConnection() + { + return connection; + } + @Override protected HttpSender getHttpSender() { diff --git a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpConnectionOverHTTP3.java b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpConnectionOverHTTP3.java index ed2b58b8e6ab..0a79f95c037f 100644 --- a/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpConnectionOverHTTP3.java +++ b/jetty-http3/http3-http-client-transport/src/main/java/org/eclipse/jetty/http3/client/http/internal/HttpConnectionOverHTTP3.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http3.client.http.internal; +import java.net.SocketAddress; import java.nio.channels.AsynchronousCloseException; import java.util.Iterator; import java.util.Set; @@ -50,6 +51,18 @@ public HTTP3SessionClient getSession() return session; } + @Override + public SocketAddress getLocalSocketAddress() + { + return session.getLocalSocketAddress(); + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return session.getRemoteSocketAddress(); + } + @Override public int getMaxMultiplex() { 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 377fd54812de..a3a84657be82 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 @@ -835,6 +835,25 @@ else if (target.equals("/2")) assertEquals(200, response.getStatus()); } + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testRequestConnection(Transport transport) throws Exception + { + init(transport); + + scenario.start(new EmptyServerHandler()); + + ContentResponse response = scenario.client.newRequest(scenario.newURI()) + .onRequestBegin(r -> + { + if (r.getConnection() == null) + r.abort(new IllegalStateException()); + }) + .send(); + + assertEquals(200, response.getStatus()); + } + private void sleep(long time) throws IOException { try