From 148cf38c5d0f9b213f30e67051155657befcac08 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 15 Jun 2021 18:47:20 +1000 Subject: [PATCH 1/3] Issue #6407 - fix logic in ClientUpgradeRequest(URI) constructor and deprecate it Signed-off-by: Lachlan Roberts --- .../core/client/CoreClientUpgradeRequest.java | 10 +------- .../websocket/core/internal/Negotiated.java | 23 ++++++------------- .../jetty/websocket/api/util/WSURI.java | 15 +++--------- .../client/ClientUpgradeRequest.java | 3 ++- 4 files changed, 13 insertions(+), 38 deletions(-) diff --git a/jetty-websocket/websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java b/jetty-websocket/websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java index c665c7240dc..cc45b3f001a 100644 --- a/jetty-websocket/websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java +++ b/jetty-websocket/websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java @@ -88,25 +88,17 @@ public abstract class CoreClientUpgradeRequest extends HttpRequest implements Re // Validate websocket URI if (!requestURI.isAbsolute()) - { throw new IllegalArgumentException("WebSocket URI must be absolute"); - } if (StringUtil.isBlank(requestURI.getScheme())) - { throw new IllegalArgumentException("WebSocket URI must include a scheme"); - } String scheme = requestURI.getScheme(); if (!HttpScheme.WS.is(scheme) && !HttpScheme.WSS.is(scheme)) - { throw new IllegalArgumentException("WebSocket URI scheme only supports [ws] and [wss], not [" + scheme + "]"); - } if (requestURI.getHost() == null) - { throw new IllegalArgumentException("Invalid WebSocket URI: host not present"); - } this.wsClient = webSocketClient; this.futureCoreSession = new CompletableFuture<>(); @@ -437,7 +429,7 @@ public abstract class CoreClientUpgradeRequest extends HttpRequest implements Re Negotiated negotiated = new Negotiated( request.getURI(), negotiatedSubProtocol, - HttpScheme.HTTPS.is(request.getScheme()), // TODO better than this? + HttpClient.isSchemeSecure(request.getScheme()), extensionStack, WebSocketConstants.SPEC_VERSION_STRING); diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Negotiated.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Negotiated.java index b0f52febbb2..af345cce7db 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Negotiated.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Negotiated.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; +import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.UrlEncoded; @@ -134,24 +135,14 @@ public class Negotiated String httpScheme = uri.getScheme(); if (httpScheme == null) return uri; - - if ("ws".equalsIgnoreCase(httpScheme) || "wss".equalsIgnoreCase(httpScheme)) - { - // keep as-is + if (HttpScheme.WS.is(httpScheme) || HttpScheme.WSS.is(httpScheme)) return uri; - } - if ("http".equalsIgnoreCase(httpScheme)) - { - // convert to ws - return new URI("ws" + uri.toString().substring(httpScheme.length())); - } - - if ("https".equalsIgnoreCase(httpScheme)) - { - // convert to wss - return new URI("wss" + uri.toString().substring(httpScheme.length())); - } + String afterScheme = uri.toString().substring(httpScheme.length()); + if (HttpScheme.HTTP.is(httpScheme)) + return new URI("ws" + afterScheme); + if (HttpScheme.HTTPS.is(httpScheme)) + return new URI("wss" + afterScheme); throw new URISyntaxException(uri.toString(), "Unrecognized HTTP scheme"); } diff --git a/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java b/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java index f8c534f76ba..dfa646d77cb 100644 --- a/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java +++ b/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java @@ -104,22 +104,13 @@ public final class WSURI Objects.requireNonNull(inputUri, "Input URI must not be null"); String httpScheme = inputUri.getScheme(); if ("ws".equalsIgnoreCase(httpScheme) || "wss".equalsIgnoreCase(httpScheme)) - { - // keep as-is return inputUri; - } + String afterScheme = inputUri.toString().substring(httpScheme.length()); if ("http".equalsIgnoreCase(httpScheme)) - { - // convert to ws - return new URI("ws" + inputUri.toString().substring(httpScheme.length())); - } - + return new URI("ws" + afterScheme); if ("https".equalsIgnoreCase(httpScheme)) - { - // convert to wss - return new URI("wss" + inputUri.toString().substring(httpScheme.length())); - } + return new URI("wss" + afterScheme); throw new URISyntaxException(inputUri.toString(), "Unrecognized HTTP scheme"); } 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 f17acf3dfc4..2cc9681c6e0 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 @@ -50,11 +50,12 @@ public final class ClientUpgradeRequest implements UpgradeRequest this.host = null; } + @Deprecated public ClientUpgradeRequest(URI uri) { this.requestURI = uri; String scheme = uri.getScheme(); - if (!HttpScheme.WS.is(scheme) || !HttpScheme.WSS.is(scheme)) + if (!HttpScheme.WS.is(scheme) && !HttpScheme.WSS.is(scheme)) throw new IllegalArgumentException("URI scheme must be 'ws' or 'wss'"); this.host = this.requestURI.getHost(); } From 10ed23f22d46e8a631b46609df03af0741cf014c Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 16 Jun 2021 00:40:35 +1000 Subject: [PATCH 2/3] add javadoc and null check for HttpScheme Signed-off-by: Lachlan Roberts --- .../main/java/org/eclipse/jetty/websocket/api/util/WSURI.java | 3 +++ .../eclipse/jetty/websocket/client/ClientUpgradeRequest.java | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java b/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java index dfa646d77cb..c6cea7c62d9 100644 --- a/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java +++ b/jetty-websocket/websocket-jetty-api/src/main/java/org/eclipse/jetty/websocket/api/util/WSURI.java @@ -103,6 +103,9 @@ public final class WSURI { Objects.requireNonNull(inputUri, "Input URI must not be null"); String httpScheme = inputUri.getScheme(); + if (httpScheme == null) + throw new URISyntaxException(inputUri.toString(), "Undefined HTTP scheme"); + if ("ws".equalsIgnoreCase(httpScheme) || "wss".equalsIgnoreCase(httpScheme)) return inputUri; 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 2cc9681c6e0..fd5be96ab26 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 @@ -50,6 +50,10 @@ public final class ClientUpgradeRequest implements UpgradeRequest this.host = null; } + /** + * @deprecated use the no-args constructor instead. + * @see ClientUpgradeRequest#ClientUpgradeRequest() + */ @Deprecated public ClientUpgradeRequest(URI uri) { From 402d79f59a3eda8642c94f491bc7d44d4d0bb0b4 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 16 Jun 2021 01:00:10 +1000 Subject: [PATCH 3/3] fix javadoc for ClientUpgradeRequest Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/websocket/client/ClientUpgradeRequest.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 fd5be96ab26..d8023551651 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 @@ -51,8 +51,7 @@ public final class ClientUpgradeRequest implements UpgradeRequest } /** - * @deprecated use the no-args constructor instead. - * @see ClientUpgradeRequest#ClientUpgradeRequest() + * @deprecated use {@link #ClientUpgradeRequest()} instead. */ @Deprecated public ClientUpgradeRequest(URI uri)