From 544ad0a28a26d19f17030410a8ebfca115aee5d4 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 12 Nov 2019 10:14:51 -0600 Subject: [PATCH 1/3] Fixes #4248 - WebSocketClient UpgradeResponse.isSuccess never set true Signed-off-by: Joakim Erdfelt --- .../jetty/websocket/client/ClientUpgradeResponse.java | 7 +++++++ .../jetty/websocket/client/WebSocketUpgradeRequest.java | 6 ++++-- .../org/eclipse/jetty/websocket/client/CookieTest.java | 3 ++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java index 9ead02fa3ac..d15deb98577 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeResponse.java @@ -55,6 +55,13 @@ public class ClientUpgradeResponse extends UpgradeResponseAdapter setAcceptedSubProtocol(fields.get(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL)); } + @Override + public boolean isSuccess() + { + // If we reached this point, where we have a UpgradeResponse object, then the upgrade is a success. + return true; + } + @Override public List getExtensions() { diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java index 62867c658c1..841e5de6c2d 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketUpgradeRequest.java @@ -596,9 +596,11 @@ public class WebSocketUpgradeRequest extends HttpRequest implements CompleteList URI requestURI = this.getURI(); + ClientUpgradeResponse upgradeResponse = new ClientUpgradeResponse(response); + WebSocketSession session = getSessionFactory().createSession(requestURI, localEndpoint, connection); session.setUpgradeRequest(new ClientUpgradeRequest(this)); - session.setUpgradeResponse(new ClientUpgradeResponse(response)); + session.setUpgradeResponse(upgradeResponse); connection.addListener(session); List extensions = new ArrayList<>(); @@ -637,7 +639,7 @@ public class WebSocketUpgradeRequest extends HttpRequest implements CompleteList if (upgradeListener != null) { - upgradeListener.onHandshakeResponse(new ClientUpgradeResponse(response)); + upgradeListener.onHandshakeResponse(upgradeResponse); } // Now swap out the connection diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java index 6a651afcbc4..bbdb41e7fe8 100644 --- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java +++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/CookieTest.java @@ -196,7 +196,8 @@ public class CookieTest serverConn.write(serverCookieFrame); // Confirm client connect on future - clientConnectFuture.get(10, TimeUnit.SECONDS); + Session session = clientConnectFuture.get(10, TimeUnit.SECONDS); + assertTrue(session.getUpgradeResponse().isSuccess(), "UpgradeResponse.isSuccess()"); clientSocket.awaitOpen(2, TimeUnit.SECONDS); // Wait for client receipt of cookie frame via client websocket From 6a6206d8e4ff3f4abfa14c24b16267722b063c97 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 13 Nov 2019 15:32:23 -0600 Subject: [PATCH 2/3] Issue #4248 - Deprecating UpgradeResponse setSuccess / isSuccess Signed-off-by: Joakim Erdfelt --- .../java/org/eclipse/jetty/websocket/api/UpgradeResponse.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java index 7b21617134a..169ade3f256 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeResponse.java @@ -103,7 +103,9 @@ public interface UpgradeResponse * or was failed (resulting in no upgrade handshake) * * @return true if upgrade response was generated, false if no upgrade response was generated + * @deprecated this has no replacement, will be removed in Jetty 10 */ + @Deprecated boolean isSuccess(); /** @@ -174,6 +176,8 @@ public interface UpgradeResponse * * @param success true to indicate a response to the upgrade handshake was sent, false to indicate no upgrade * response was sent + * @deprecated this has no replacement, will be removed in Jetty 10 */ + @Deprecated void setSuccess(boolean success); } From 5479ed030f753d7f2e494da73c087970aca2f06b Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 13 Nov 2019 15:33:00 -0600 Subject: [PATCH 3/3] Issue #4248 - Using ServletUpgradeResponse.setSuccess(true) for servlet side Signed-off-by: Joakim Erdfelt --- .../eclipse/jetty/websocket/server/WebSocketServerFactory.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java index 2b5e116bd08..150db502c71 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java @@ -698,6 +698,8 @@ public class WebSocketServerFactory extends ContainerLifeCycle implements WebSoc // Process (version specific) handshake response handshaker.doHandshakeResponse(request, response); + response.setSuccess(true); + if (LOG.isDebugEnabled()) LOG.debug("Websocket upgrade {} {} {} {}", request.getRequestURI(), version, response.getAcceptedSubProtocol(), wsConnection);