From a179535db36de0404c8e4248d0e0d42fe3c4d196 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 8 Jul 2020 18:04:36 +1000 Subject: [PATCH 1/3] Issue #5018 - add WebSocketClient UpgradeRequest timeout to jetty 10 Signed-off-by: Lachlan Roberts --- .../client/ClientUpgradeRequest.java | 50 ++++++++++++------- .../websocket/client/WebSocketClient.java | 11 ++-- .../JettyWebSocketExtensionConfigTest.java | 3 +- .../tests/JettyWebSocketNegotiationTest.java | 3 +- .../tests/WebSocketServletExamplesTest.java | 3 +- .../tests/client/ClientTimeoutTest.java | 4 +- 6 files changed, 42 insertions(+), 32 deletions(-) diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java index edd4695ac765..fb41b6af2cd6 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java @@ -23,11 +23,11 @@ import java.security.Principal; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.TreeMap; +import java.util.concurrent.TimeUnit; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpScheme; @@ -39,19 +39,19 @@ */ public final class ClientUpgradeRequest implements UpgradeRequest { - private URI requestURI; - private List subProtocols = new ArrayList<>(1); - private List extensions = new ArrayList<>(1); - private List cookies = new ArrayList<>(1); - private Map> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - private Map> parameters = new HashMap<>(1); - private String httpVersion; - private String method; - private String host; + private final List subProtocols = new ArrayList<>(1); + private final List extensions = new ArrayList<>(1); + private final List cookies = new ArrayList<>(1); + private final Map> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + private final URI requestURI; + private final String host; + private long timeout; public ClientUpgradeRequest() { /* anonymous, no requestURI, upgrade request */ + this.requestURI = null; + this.host = null; } public ClientUpgradeRequest(URI uri) @@ -161,13 +161,13 @@ public String getHost() @Override public String getHttpVersion() { - return httpVersion; + throw new UnsupportedOperationException("HttpVersion not available on ClientUpgradeRequest"); } @Override public String getMethod() { - return method; + throw new UnsupportedOperationException("Method not available on ClientUpgradeRequest"); } @Override @@ -176,15 +176,10 @@ public String getOrigin() return getHeader(HttpHeader.ORIGIN.name()); } - /** - * Returns a map of the query parameters of the request. - * - * @return a unmodifiable map of query parameters of the request. - */ @Override public Map> getParameterMap() { - return Collections.unmodifiableMap(parameters); + return Collections.emptyMap(); } @Override @@ -297,6 +292,25 @@ public void setSession(Object session) throw new UnsupportedOperationException("HttpSession not available on Client request"); } + /** + * @param timeout the total timeout for the request/response conversation of the WebSocket handshake; + * use zero or a negative value to disable the timeout + * @param unit the timeout unit + */ + public void setTimeout(long timeout, TimeUnit unit) + { + this.timeout = unit.toMillis(timeout); + } + + /** + * @return the total timeout for this request, in milliseconds; + * zero or negative if the timeout is disabled + */ + public long getTimeout() + { + return timeout; + } + /** * ABNF from RFC 2616, RFC 822, and RFC 6455 specified characters requiring quoting. */ diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 56f74cc45299..e6e59ba41c9b 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -29,6 +29,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import org.eclipse.jetty.client.HttpClient; @@ -41,7 +42,6 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ShutdownThread; import org.eclipse.jetty.websocket.api.Session; -import org.eclipse.jetty.websocket.api.UpgradeRequest; import org.eclipse.jetty.websocket.api.WebSocketBehavior; import org.eclipse.jetty.websocket.api.WebSocketContainer; import org.eclipse.jetty.websocket.api.WebSocketPolicy; @@ -109,7 +109,7 @@ public CompletableFuture connect(Object websocket, URI toUri) throws IO * @return the future for the session, available on success of connect * @throws IOException if unable to connect */ - public CompletableFuture connect(Object websocket, URI toUri, UpgradeRequest request) throws IOException + public CompletableFuture connect(Object websocket, URI toUri, ClientUpgradeRequest request) throws IOException { return connect(websocket, toUri, request, null); } @@ -124,7 +124,7 @@ public CompletableFuture connect(Object websocket, URI toUri, UpgradeRe * @return the future for the session, available on success of connect * @throws IOException if unable to connect */ - public CompletableFuture connect(Object websocket, URI toUri, UpgradeRequest request, JettyUpgradeListener upgradeListener) throws IOException + public CompletableFuture connect(Object websocket, URI toUri, ClientUpgradeRequest request, JettyUpgradeListener upgradeListener) throws IOException { for (Connection.Listener listener : getBeans(Connection.Listener.class)) { @@ -132,6 +132,8 @@ public CompletableFuture connect(Object websocket, URI toUri, UpgradeRe } JettyClientUpgradeRequest upgradeRequest = new JettyClientUpgradeRequest(coreClient, request, toUri, frameHandlerFactory, websocket); + upgradeRequest.timeout(request.getTimeout(), TimeUnit.MILLISECONDS); + upgradeRequest.setConfiguration(configurationCustomizer); if (upgradeListener != null) { upgradeRequest.addListener(new UpgradeListener() @@ -149,9 +151,8 @@ public void onHandshakeResponse(HttpRequest request, HttpResponse response) } }); } - upgradeRequest.setConfiguration(configurationCustomizer); - CompletableFuture futureSession = new CompletableFuture<>(); + CompletableFuture futureSession = new CompletableFuture<>(); coreClient.connect(upgradeRequest).whenComplete((coreSession, error) -> { if (error != null) diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketExtensionConfigTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketExtensionConfigTest.java index 9b2a4cc74a01..b9c8f0ed65d2 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketExtensionConfigTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketExtensionConfigTest.java @@ -31,7 +31,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.websocket.api.Session; -import org.eclipse.jetty.websocket.api.UpgradeRequest; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; import org.eclipse.jetty.websocket.client.JettyUpgradeListener; @@ -104,7 +103,7 @@ public void testJettyExtensionConfig() throws Exception URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath"); EventSocket socket = new EventSocket(); - UpgradeRequest request = new ClientUpgradeRequest(); + ClientUpgradeRequest request = new ClientUpgradeRequest(); request.addExtensions(ExtensionConfig.parse("permessage-deflate")); CountDownLatch correctResponseExtensions = new CountDownLatch(1); diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java index 2c7efe059b0c..7e81b1a24af8 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java @@ -29,7 +29,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.websocket.api.Session; -import org.eclipse.jetty.websocket.api.UpgradeRequest; import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; @@ -81,7 +80,7 @@ public void testBadRequest() throws Exception URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath"); EventSocket socket = new EventSocket(); - UpgradeRequest upgradeRequest = new ClientUpgradeRequest(); + ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(); upgradeRequest.addExtensions("permessage-deflate;invalidParameter"); CompletableFuture connect = client.connect(socket, uri, upgradeRequest); diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketServletExamplesTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketServletExamplesTest.java index 547c5c396aec..802c069d358e 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketServletExamplesTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketServletExamplesTest.java @@ -36,7 +36,6 @@ import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Credential; import org.eclipse.jetty.websocket.api.Session; -import org.eclipse.jetty.websocket.api.UpgradeRequest; import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; @@ -139,7 +138,7 @@ public void testAdvancedEchoServlet() throws Exception URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/advancedEcho"); EventSocket socket = new EventSocket(); - UpgradeRequest upgradeRequest = new ClientUpgradeRequest(); + ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(); upgradeRequest.setSubProtocols("text"); CompletableFuture connect = client.connect(socket, uri, upgradeRequest); try (Session session = connect.get(5, TimeUnit.SECONDS)) diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientTimeoutTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientTimeoutTest.java index e691c8e12269..a0071ef6a930 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientTimeoutTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientTimeoutTest.java @@ -38,7 +38,6 @@ import org.eclipse.jetty.websocket.tests.EventSocket; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -109,14 +108,13 @@ public void testWebSocketClientTimeout() throws Exception assertThat(coreUpgradeException.getCause(), instanceOf(TimeoutException.class)); } - @Disabled("need the client timeout to be ported from 9.4 to 10") @Test public void testClientUpgradeRequestTimeout() throws Exception { EventSocket clientSocket = new EventSocket(); long timeout = 1000; ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(); - // TODO: upgradeRequest.setTimeout(timeout, TimeUnit.MILLISECONDS); + upgradeRequest.setTimeout(timeout, TimeUnit.MILLISECONDS); Future connect = client.connect(clientSocket, WSURI.toWebsocket(server.getURI()), upgradeRequest); ExecutionException executionException = assertThrows(ExecutionException.class, () -> connect.get(timeout * 2, TimeUnit.MILLISECONDS)); From 25d7b99ba1c00c75ad10140c0e9ee9c44e257de2 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 9 Jul 2020 08:11:05 +1000 Subject: [PATCH 2/3] Issue #5018 - add WebSocketClient UpgradeRequest timeout to jetty 10 Signed-off-by: Lachlan Roberts --- .../websocket/client/WebSocketClient.java | 2 -- .../impl/JettyClientUpgradeRequest.java | 30 +++++-------------- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index e6e59ba41c9b..ed36d8a09062 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -29,7 +29,6 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.ThreadLocalRandom; -import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import org.eclipse.jetty.client.HttpClient; @@ -132,7 +131,6 @@ public CompletableFuture connect(Object websocket, URI toUri, ClientUpg } JettyClientUpgradeRequest upgradeRequest = new JettyClientUpgradeRequest(coreClient, request, toUri, frameHandlerFactory, websocket); - upgradeRequest.timeout(request.getTimeout(), TimeUnit.MILLISECONDS); upgradeRequest.setConfiguration(configurationCustomizer); if (upgradeListener != null) { diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java index 0f6ddeeef99e..b3f0abd85079 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java @@ -18,30 +18,27 @@ package org.eclipse.jetty.websocket.client.impl; -import java.net.HttpCookie; import java.net.URI; -import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.eclipse.jetty.client.HttpResponse; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.EndPoint; -import org.eclipse.jetty.websocket.api.UpgradeRequest; +import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; import org.eclipse.jetty.websocket.common.JettyWebSocketFrameHandler; import org.eclipse.jetty.websocket.common.JettyWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.core.ExtensionConfig; import org.eclipse.jetty.websocket.core.FrameHandler; -import org.eclipse.jetty.websocket.core.client.ClientUpgradeRequest; import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient; -public class JettyClientUpgradeRequest extends ClientUpgradeRequest +public class JettyClientUpgradeRequest extends org.eclipse.jetty.websocket.core.client.ClientUpgradeRequest { private final DelegatedJettyClientUpgradeRequest handshakeRequest; private final JettyWebSocketFrameHandler frameHandler; - public JettyClientUpgradeRequest(WebSocketCoreClient coreClient, UpgradeRequest request, URI requestURI, JettyWebSocketFrameHandlerFactory frameHandlerFactory, + public JettyClientUpgradeRequest(WebSocketCoreClient coreClient, ClientUpgradeRequest request, URI requestURI, JettyWebSocketFrameHandlerFactory frameHandlerFactory, Object websocketPojo) { super(coreClient, requestURI); @@ -53,15 +50,7 @@ public JettyClientUpgradeRequest(WebSocketCoreClient coreClient, UpgradeRequest request.getHeaders().forEach(fields::put); // Copy manually created Cookies into place - List cookies = request.getCookies(); - if (cookies != null) - { - // TODO: remove existing Cookie header (if set)? - for (HttpCookie cookie : cookies) - { - fields.add(HttpHeader.COOKIE, cookie.toString()); - } - } + request.getCookies().forEach(c -> fields.add(HttpHeader.COOKIE, c.toString())); // Copy sub-protocols setSubProtocols(request.getSubProtocols()); @@ -71,13 +60,8 @@ public JettyClientUpgradeRequest(WebSocketCoreClient coreClient, UpgradeRequest .map(c -> new ExtensionConfig(c.getName(), c.getParameters())) .collect(Collectors.toList())); - // Copy method from upgradeRequest object - if (request.getMethod() != null) - method(request.getMethod()); - - // Copy version from upgradeRequest object - if (request.getHttpVersion() != null) - version(HttpVersion.fromString(request.getHttpVersion())); + // Copy timeout from upgradeRequest object + timeout(request.getTimeout(), TimeUnit.MILLISECONDS); } handshakeRequest = new DelegatedJettyClientUpgradeRequest(this); From 145dcf502ce20fb4231d78c95b53664590142422 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 14 Jul 2020 16:52:50 +1000 Subject: [PATCH 3/3] cleanups after merge Signed-off-by: Lachlan Roberts --- .../client/impl/JettyClientUpgradeRequest.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java index df255270ed93..f91f12214348 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/impl/JettyClientUpgradeRequest.java @@ -18,9 +18,7 @@ package org.eclipse.jetty.websocket.client.impl; -import java.net.HttpCookie; import java.net.URI; -import java.util.List; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -50,22 +48,13 @@ public JettyClientUpgradeRequest(WebSocketCoreClient coreClient, ClientUpgradeRe headers(fields -> request.getHeaders().forEach(fields::put)); // Copy manually created Cookies into place - List cookies = request.getCookies(); - if (cookies != null) - { - // TODO: remove existing Cookie header (if set)? - headers(fields -> cookies.forEach(cookie -> fields.add(HttpHeader.COOKIE, cookie.toString()))); - } + headers(fields -> request.getCookies().forEach(cookie -> fields.add(HttpHeader.COOKIE, cookie.toString()))); - // Copy sub-protocols setSubProtocols(request.getSubProtocols()); - - // Copy extensions setExtensions(request.getExtensions().stream() .map(c -> new ExtensionConfig(c.getName(), c.getParameters())) .collect(Collectors.toList())); - // Copy timeout from upgradeRequest object timeout(request.getTimeout(), TimeUnit.MILLISECONDS); }