From 0d570dd6b6895e71a780ccd76387477243abe70f Mon Sep 17 00:00:00 2001 From: lachan-roberts Date: Fri, 22 Mar 2019 15:42:39 +1100 Subject: [PATCH] Issue #3478 - changes from review ExtensionStack.negotiate now differentiates between incorrect extension config offered or incorrect config from negotiation adding more BadMessageException cases Signed-off-by: lachan-roberts --- .../client/JavaxWebSocketClientContainer.java | 8 -- .../test/resources/jetty-logging.properties | 2 +- .../core/client/ClientUpgradeRequest.java | 10 +- .../core/client/WebSocketCoreClient.java | 4 - .../core/internal/ExtensionStack.java | 29 ++++- .../websocket/core/server/Negotiation.java | 123 ++++++++++-------- .../server/internal/RFC6455Handshaker.java | 21 ++- .../core/GeneratorFrameFlagsTest.java | 10 +- .../jetty/websocket/core/GeneratorTest.java | 2 +- .../jetty/websocket/core/ParserCapture.java | 2 +- .../extensions/DeflateFrameExtensionTest.java | 2 +- .../core/extensions/ExtensionStackTest.java | 6 +- .../core/extensions/ExtensionTool.java | 2 +- .../servlet/ServletUpgradeRequest.java | 27 ++-- .../websocket/servlet/WebSocketMapping.java | 23 +--- 15 files changed, 139 insertions(+), 132 deletions(-) diff --git a/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java b/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java index 9d0b128e01e..c293ce34d53 100644 --- a/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java +++ b/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java @@ -156,18 +156,10 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple upgradeRequest.addListener(jsrUpgradeListener); for (Extension ext : clientEndpointConfig.getExtensions()) - { - if (!getExtensionRegistry().isAvailable(ext.getName())) - { - throw new IllegalArgumentException("Requested extension [" + ext.getName() + "] is not installed"); - } upgradeRequest.addExtensions(new JavaxWebSocketExtensionConfig(ext)); - } if (clientEndpointConfig.getPreferredSubprotocols().size() > 0) - { upgradeRequest.setSubProtocols(clientEndpointConfig.getPreferredSubprotocols()); - } } try diff --git a/jetty-websocket/jetty-websocket-tests/src/test/resources/jetty-logging.properties b/jetty-websocket/jetty-websocket-tests/src/test/resources/jetty-logging.properties index 90faa7b8c26..edd963caed4 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/jetty-websocket-tests/src/test/resources/jetty-logging.properties @@ -20,7 +20,7 @@ # org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog org.eclipse.jetty.LEVEL=WARN -org.eclipse.jetty.websocket.tests.LEVEL=DEBUG +# org.eclipse.jetty.websocket.tests.LEVEL=DEBUG # org.eclipse.jetty.util.log.stderr.LONG=true # org.eclipse.jetty.server.AbstractConnector.LEVEL=DEBUG # org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/ClientUpgradeRequest.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/ClientUpgradeRequest.java index 6e2e78e36fe..3482013657a 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/ClientUpgradeRequest.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/ClientUpgradeRequest.java @@ -260,7 +260,7 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon throw new HttpResponseException("Invalid Sec-WebSocket-Accept hash (was:" + respHash + ", expected:" + expectedHash + ")", response); // Parse the Negotiated Extensions - List extensions = new ArrayList<>(); + List negotiatedExtensions = new ArrayList<>(); HttpField extField = response.getHeaders().getField(HttpHeader.SEC_WEBSOCKET_EXTENSIONS); if (extField != null) { @@ -272,7 +272,7 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon QuotedStringTokenizer tok = new QuotedStringTokenizer(extVal, ","); while (tok.hasMoreTokens()) { - extensions.add(ExtensionConfig.parse(tok.nextToken())); + negotiatedExtensions.add(ExtensionConfig.parse(tok.nextToken())); } } } @@ -280,7 +280,7 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon // Verify the Negotiated Extensions List offeredExtensions = getExtensions(); - for (ExtensionConfig config : extensions) + for (ExtensionConfig config : negotiatedExtensions) { if (config.getName().startsWith("@")) continue; @@ -289,7 +289,7 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon if (numMatch < 1) throw new WebSocketException("Upgrade failed: Sec-WebSocket-Extensions contained extension not requested"); - numMatch = extensions.stream().filter(c -> config.getName().equalsIgnoreCase(c.getName())).count(); + numMatch = negotiatedExtensions.stream().filter(c -> config.getName().equalsIgnoreCase(c.getName())).count(); if (numMatch > 1) throw new WebSocketException("Upgrade failed: Sec-WebSocket-Extensions contained more than one extension of the same name"); } @@ -297,7 +297,7 @@ public abstract class ClientUpgradeRequest extends HttpRequest implements Respon // Negotiate the extension stack HttpClient httpClient = wsClient.getHttpClient(); ExtensionStack extensionStack = new ExtensionStack(wsClient.getExtensionRegistry()); - extensionStack.negotiate(wsClient.getObjectFactory(), httpClient.getByteBufferPool(), extensions); + extensionStack.negotiate(wsClient.getObjectFactory(), httpClient.getByteBufferPool(), offeredExtensions, negotiatedExtensions); // Get the negotiated subprotocol String negotiatedSubProtocol = null; diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java index 759232bf107..3a9a2c680fb 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java @@ -87,11 +87,7 @@ public class WebSocketCoreClient extends ContainerLifeCycle implements FrameHand public CompletableFuture connect(ClientUpgradeRequest request) throws IOException { if (!isStarted()) - { throw new IllegalStateException(WebSocketCoreClient.class.getSimpleName() + "@" + this.hashCode() + " is not started"); - } - - // TODO: add HttpClient delayed/on-demand start - See Issue #1516 // Validate Requested Extensions for (ExtensionConfig reqExt : request.getExtensions()) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/ExtensionStack.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/ExtensionStack.java index fa903261f14..e89cb46df3e 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/ExtensionStack.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/ExtensionStack.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.ListIterator; import java.util.stream.Collectors; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.DecoratedObjectFactory; @@ -38,6 +39,7 @@ import org.eclipse.jetty.websocket.core.ExtensionConfig; import org.eclipse.jetty.websocket.core.Frame; import org.eclipse.jetty.websocket.core.IncomingFrames; import org.eclipse.jetty.websocket.core.OutgoingFrames; +import org.eclipse.jetty.websocket.core.WebSocketException; import org.eclipse.jetty.websocket.core.WebSocketExtensionRegistry; /** @@ -107,20 +109,37 @@ public class ExtensionStack implements IncomingFrames, OutgoingFrames, Dumpable *

* For the list of negotiated extensions, use {@link #getNegotiatedExtensions()} * - * @param configs the configurations being requested + * @param offeredConfigs the configurations being requested by the client + * @param negotiatedConfigs the configurations accepted by the server */ - public void negotiate(DecoratedObjectFactory objectFactory, ByteBufferPool bufferPool, List configs) + public void negotiate(DecoratedObjectFactory objectFactory, ByteBufferPool bufferPool, List offeredConfigs, List negotiatedConfigs) { if (LOG.isDebugEnabled()) - LOG.debug("Extension Configs={}", configs); + LOG.debug("Extension Configs={}", negotiatedConfigs); this.extensions = new ArrayList<>(); String rsvClaims[] = new String[3]; - for (ExtensionConfig config : configs) + for (ExtensionConfig config : negotiatedConfigs) { - Extension ext = factory.newInstance(objectFactory, bufferPool, config); + Extension ext; + + try + { + ext = factory.newInstance(objectFactory, bufferPool, config); + } + catch (Throwable t) + { + for (ExtensionConfig offered : offeredConfigs) + { + if (offered.getParameterizedName().equals(config.getParameterizedName())) + throw new BadMessageException("offered extension had bad parameters: ", t); + } + + throw new WebSocketException("negotiated extension had bad parameters: ", t); + } + if (ext == null) { // Extension not present on this side diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/Negotiation.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/Negotiation.java index 7a70edd81e0..52df3e17baa 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/Negotiation.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/Negotiation.java @@ -27,6 +27,7 @@ import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.QuotedCSV; @@ -55,13 +56,16 @@ public class Negotiation private String subprotocol; private ExtensionStack extensionStack; + /** + * @throws BadMessageException if there is any errors parsing the upgrade request + */ public Negotiation( Request baseRequest, HttpServletRequest request, HttpServletResponse response, WebSocketExtensionRegistry registry, DecoratedObjectFactory objectFactory, - ByteBufferPool bufferPool) + ByteBufferPool bufferPool) throws BadMessageException { this.baseRequest = baseRequest; this.request = request; @@ -77,73 +81,80 @@ public class Negotiation QuotedCSV extensions = null; QuotedCSV subprotocols = null; - for (HttpField field : baseRequest.getHttpFields()) + try { - if (field.getHeader() != null) + for (HttpField field : baseRequest.getHttpFields()) { - switch (field.getHeader()) + if (field.getHeader() != null) { - case UPGRADE: - if (upgrade == null && "websocket".equalsIgnoreCase(field.getValue())) - upgrade = Boolean.TRUE; - break; + switch (field.getHeader()) + { + case UPGRADE: + if (upgrade == null && "websocket".equalsIgnoreCase(field.getValue())) + upgrade = Boolean.TRUE; + break; - case CONNECTION: - if (connectionCSVs == null) - connectionCSVs = new QuotedCSV(); - connectionCSVs.addValue(field.getValue()); - break; + case CONNECTION: + if (connectionCSVs == null) + connectionCSVs = new QuotedCSV(); + connectionCSVs.addValue(field.getValue()); + break; - case SEC_WEBSOCKET_KEY: - key = field.getValue(); - break; + case SEC_WEBSOCKET_KEY: + key = field.getValue(); + break; - case SEC_WEBSOCKET_VERSION: - version = field.getValue(); - break; + case SEC_WEBSOCKET_VERSION: + version = field.getValue(); + break; - case SEC_WEBSOCKET_EXTENSIONS: - if (extensions == null) - extensions = new QuotedCSV(field.getValue()); - else - extensions.addValue(field.getValue()); - break; + case SEC_WEBSOCKET_EXTENSIONS: + if (extensions == null) + extensions = new QuotedCSV(field.getValue()); + else + extensions.addValue(field.getValue()); + break; - case SEC_WEBSOCKET_SUBPROTOCOL: - if (subprotocols == null) - subprotocols = new QuotedCSV(field.getValue()); - else - subprotocols.addValue(field.getValue()); - break; + case SEC_WEBSOCKET_SUBPROTOCOL: + if (subprotocols == null) + subprotocols = new QuotedCSV(field.getValue()); + else + subprotocols.addValue(field.getValue()); + break; - default: + default: + } } } + + this.version = version; + this.key = key; + this.upgrade = upgrade != null && connectionCSVs != null && connectionCSVs.getValues().stream().anyMatch(s -> s.equalsIgnoreCase("Upgrade")); + + Set available = registry.getAvailableExtensionNames(); + offeredExtensions = extensions == null + ? Collections.emptyList() + : extensions.getValues().stream() + .map(ExtensionConfig::parse) + .filter(ec -> available.contains(ec.getName().toLowerCase()) && !ec.getName().startsWith("@")) + .collect(Collectors.toList()); + + offeredSubprotocols = subprotocols == null + ? Collections.emptyList() + : subprotocols.getValues(); + + negotiatedExtensions = new ArrayList<>(); + for (ExtensionConfig config : offeredExtensions) + { + long matches = negotiatedExtensions.stream() + .filter(negotiatedConfig -> negotiatedConfig.getName().equals(config.getName())).count(); + if (matches == 0) + negotiatedExtensions.add(config); + } } - - this.version = version; - this.key = key; - this.upgrade = upgrade != null && connectionCSVs != null && connectionCSVs.getValues().stream().anyMatch(s -> s.equalsIgnoreCase("Upgrade")); - - Set available = registry.getAvailableExtensionNames(); - offeredExtensions = extensions == null - ?Collections.emptyList() - :extensions.getValues().stream() - .map(ExtensionConfig::parse) - .filter(ec -> available.contains(ec.getName().toLowerCase()) && !ec.getName().startsWith("@")) - .collect(Collectors.toList()); - - offeredSubprotocols = subprotocols == null - ?Collections.emptyList() - :subprotocols.getValues(); - - negotiatedExtensions = new ArrayList<>(); - for (ExtensionConfig config : offeredExtensions) + catch (Throwable t) { - long matches = negotiatedExtensions.stream() - .filter(negotiatedConfig->negotiatedConfig.getName().equals(config.getName())).count(); - if (matches == 0) - negotiatedExtensions.add(config); + throw new BadMessageException("Invalid Handshake Request", t); } } @@ -217,7 +228,7 @@ public class Negotiation { // Extension stack can decide to drop any of these extensions or their parameters extensionStack = new ExtensionStack(registry); - extensionStack.negotiate(objectFactory, bufferPool, negotiatedExtensions); + extensionStack.negotiate(objectFactory, bufferPool, offeredExtensions, negotiatedExtensions); negotiatedExtensions = extensionStack.getNegotiatedExtensions(); if (extensionStack.hasNegotiatedExtensions()) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Handshaker.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Handshaker.java index 6135f3079c4..b5b8e39c2f4 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Handshaker.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Handshaker.java @@ -24,6 +24,7 @@ import java.util.concurrent.Executor; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; @@ -47,6 +48,7 @@ import org.eclipse.jetty.websocket.core.ExtensionConfig; import org.eclipse.jetty.websocket.core.FrameHandler; import org.eclipse.jetty.websocket.core.WebSocketConstants; import org.eclipse.jetty.websocket.core.WebSocketException; +import org.eclipse.jetty.websocket.core.internal.ExtensionStack; import org.eclipse.jetty.websocket.core.internal.Negotiated; import org.eclipse.jetty.websocket.core.internal.WebSocketChannel; import org.eclipse.jetty.websocket.core.internal.WebSocketConnection; @@ -119,11 +121,7 @@ public final class RFC6455Handshaker implements Handshaker } if (negotiation.getKey() == null) - { - if (LOG.isDebugEnabled()) - LOG.debug("not upgraded no key {}", baseRequest); - return false; - } + throw new BadMessageException("not upgraded no key"); // Negotiate the FrameHandler FrameHandler handler = negotiator.negotiate(negotiation); @@ -150,7 +148,8 @@ public final class RFC6455Handshaker implements Handshaker // Check for handler if (handler == null) { - LOG.warn("not upgraded: no frame handler provided {}", baseRequest); + if (LOG.isDebugEnabled()) + LOG.debug("not upgraded: no frame handler provided {}", baseRequest); return false; } @@ -182,11 +181,14 @@ public final class RFC6455Handshaker implements Handshaker throw new WebSocketException("Upgrade failed: multiple negotiated extensions of the same name"); } + // Create and Negotiate the ExtensionStack + ExtensionStack extensionStack = negotiation.getExtensionStack(); + Negotiated negotiated = new Negotiated( baseRequest.getHttpURI().toURI(), subprotocol, baseRequest.isSecure(), - negotiation.getExtensionStack(), + extensionStack, WebSocketConstants.SPEC_VERSION_STRING); // Create the Channel @@ -203,10 +205,7 @@ public final class RFC6455Handshaker implements Handshaker if (LOG.isDebugEnabled()) LOG.debug("connection {}", connection); if (connection == null) - { - LOG.warn("not upgraded: no connection {}", baseRequest); - return false; - } + throw new WebSocketException("not upgraded: no connection"); for (Connection.Listener listener : connector.getBeans(Connection.Listener.class)) connection.addListener(listener); diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorFrameFlagsTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorFrameFlagsTest.java index bcc57892434..4267b979995 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorFrameFlagsTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorFrameFlagsTest.java @@ -18,6 +18,10 @@ package org.eclipse.jetty.websocket.core; +import java.nio.ByteBuffer; +import java.util.LinkedList; +import java.util.stream.Stream; + import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.util.DecoratedObjectFactory; @@ -29,10 +33,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import java.nio.ByteBuffer; -import java.util.LinkedList; -import java.util.stream.Stream; - import static org.junit.jupiter.api.Assertions.assertThrows; /** @@ -64,7 +64,7 @@ public class GeneratorFrameFlagsTest public void setup(Frame invalidFrame) { ExtensionStack exStack = new ExtensionStack(new WebSocketExtensionRegistry()); - exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>()); + exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>(), new LinkedList<>()); this.channel = new WebSocketChannel(new AbstractTestFrameHandler(), Behavior.CLIENT, Negotiated.from(exStack)); } diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorTest.java index 052d650c02c..42990125bb4 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/GeneratorTest.java @@ -56,7 +56,7 @@ public class GeneratorTest { ByteBufferPool bufferPool = new MappedByteBufferPool(); ExtensionStack exStack = new ExtensionStack(new WebSocketExtensionRegistry()); - exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>()); + exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>(), new LinkedList<>()); return new WebSocketChannel(new AbstractTestFrameHandler(), behavior, Negotiated.from(exStack)); } diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/ParserCapture.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/ParserCapture.java index 9339bdb469f..75bea86cefc 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/ParserCapture.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/ParserCapture.java @@ -59,7 +59,7 @@ public class ParserCapture ByteBufferPool bufferPool = new MappedByteBufferPool(); ExtensionStack exStack = new ExtensionStack(new WebSocketExtensionRegistry()); - exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>()); + exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>(), new LinkedList<>()); this.channel = new WebSocketChannel(new AbstractTestFrameHandler(), behavior, Negotiated.from(exStack)); } diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/DeflateFrameExtensionTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/DeflateFrameExtensionTest.java index c62a6eac28b..46ad0bad67a 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/DeflateFrameExtensionTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/DeflateFrameExtensionTest.java @@ -420,7 +420,7 @@ public class DeflateFrameExtensionTest extends AbstractExtensionTest { ByteBufferPool bufferPool = new MappedByteBufferPool(); ExtensionStack exStack = new ExtensionStack(new WebSocketExtensionRegistry()); - exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>()); + exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>(), new LinkedList<>()); WebSocketChannel channel = new WebSocketChannel(new AbstractTestFrameHandler(), Behavior.SERVER, Negotiated.from(exStack)); channel.setMaxFrameSize(maxMessageSize); diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionStackTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionStackTest.java index d5276089157..89113f75590 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionStackTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionStackTest.java @@ -75,7 +75,7 @@ public class ExtensionStackTest // 1 extension List configs = new ArrayList<>(); configs.add(ExtensionConfig.parse("identity")); - stack.negotiate(objectFactory, bufferPool, configs); + stack.negotiate(objectFactory, bufferPool, configs, configs); // Setup Listeners IncomingFrames session = new IncomingFramesCapture(); @@ -99,7 +99,7 @@ public class ExtensionStackTest List configs = new ArrayList<>(); configs.add(ExtensionConfig.parse("identity; id=A")); configs.add(ExtensionConfig.parse("identity; id=B")); - stack.negotiate(objectFactory, bufferPool, configs); + stack.negotiate(objectFactory, bufferPool, configs, configs); // Setup Listeners IncomingFrames session = new IncomingFramesCapture(); @@ -130,7 +130,7 @@ public class ExtensionStackTest { String chromeRequest = "permessage-deflate; client_max_window_bits, x-webkit-deflate-frame"; List requestedConfigs = ExtensionConfig.parseList(chromeRequest); - stack.negotiate(objectFactory, bufferPool, requestedConfigs); + stack.negotiate(objectFactory, bufferPool, requestedConfigs, requestedConfigs); List negotiated = stack.getNegotiatedExtensions(); String response = ExtensionConfig.toHeaderValue(negotiated); diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionTool.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionTool.java index 519b4d8b437..53580963aad 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionTool.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/extensions/ExtensionTool.java @@ -154,7 +154,7 @@ public class ExtensionTool { ByteBufferPool bufferPool = new MappedByteBufferPool(); ExtensionStack exStack = new ExtensionStack(new WebSocketExtensionRegistry()); - exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>()); + exStack.negotiate(new DecoratedObjectFactory(), bufferPool, new LinkedList<>(), new LinkedList<>()); WebSocketChannel channel = new WebSocketChannel(new AbstractTestFrameHandler(), Behavior.SERVER, Negotiated.from(exStack)); return channel; } diff --git a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeRequest.java b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeRequest.java index b073df0ef18..409fc21b066 100644 --- a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeRequest.java +++ b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/ServletUpgradeRequest.java @@ -22,7 +22,6 @@ import java.net.HttpCookie; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.net.URI; -import java.net.URISyntaxException; import java.security.Principal; import java.security.cert.X509Certificate; import java.util.ArrayList; @@ -38,6 +37,7 @@ import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.websocket.core.ExtensionConfig; import org.eclipse.jetty.websocket.core.WebSocketConstants; @@ -57,21 +57,28 @@ public class ServletUpgradeRequest private List cookies; private Map> parameterMap; - public ServletUpgradeRequest(Negotiation negotiation) throws URISyntaxException + public ServletUpgradeRequest(Negotiation negotiation) throws BadMessageException { this.negotiation = negotiation; HttpServletRequest httpRequest = negotiation.getRequest(); this.queryString = httpRequest.getQueryString(); this.secure = httpRequest.isSecure(); - // TODO why is this URL and not URI? - StringBuffer uri = httpRequest.getRequestURL(); - // WHY? - if (this.queryString != null) - uri.append("?").append(this.queryString); - uri.replace(0, uri.indexOf(":"), secure?"wss":"ws"); - this.requestURI = new URI(uri.toString()); - this.request = new UpgradeHttpServletRequest(httpRequest); + try + { + // TODO why is this URL and not URI? + StringBuffer uri = httpRequest.getRequestURL(); + // WHY? + if (this.queryString != null) + uri.append("?").append(this.queryString); + uri.replace(0, uri.indexOf(":"), secure ? "wss" : "ws"); + this.requestURI = new URI(uri.toString()); + this.request = new UpgradeHttpServletRequest(httpRequest); + } + catch (Throwable t) + { + throw new BadMessageException("Bad WebSocket UpgradeRequest", t); + } } /** diff --git a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java index 5cec41207c8..b8ec1622915 100644 --- a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java +++ b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.websocket.servlet; import java.io.IOException; -import java.net.URISyntaxException; import java.util.function.Consumer; import javax.servlet.ServletContext; @@ -32,7 +31,6 @@ import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.http.pathmap.RegexPathSpec; import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.http.pathmap.UriTemplatePathSpec; -import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.LifeCycle; @@ -217,7 +215,7 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener return mapping.getResource(); } - public boolean upgrade(HttpServletRequest request, HttpServletResponse response, FrameHandler.Customizer defaultCustomizer) + public boolean upgrade(HttpServletRequest request, HttpServletResponse response, FrameHandler.Customizer defaultCustomizer) throws IOException { // Since this may be a filter, we need to be smart about determining the target path. // We should rely on the Container for stripping path parameters and its ilk before @@ -240,14 +238,7 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener LOG.debug("WebSocket Negotiated detected on {} for endpoint {}", target, negotiator); // We have an upgrade request - try - { - return handshaker.upgradeRequest(negotiator, request, response, defaultCustomizer); - } - catch (IOException e) - { - throw new WebSocketException(e); - } + return handshaker.upgradeRequest(negotiator, request, response, defaultCustomizer); } private class Negotiator extends WebSocketNegotiator.AbstractNegotiator @@ -269,7 +260,7 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener @Override - public FrameHandler negotiate(Negotiation negotiation) + public FrameHandler negotiate(Negotiation negotiation) throws IOException { ServletContext servletContext = negotiation.getRequest().getServletContext(); if (servletContext == null) @@ -304,14 +295,6 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener return null; } - catch (IOException e) - { - throw new RuntimeIOException(e); - } - catch (URISyntaxException e) - { - throw new RuntimeIOException("Unable to negotiate websocket due to mangled request URI", e); - } finally { Thread.currentThread().setContextClassLoader(old);