diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketNegotiationTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketNegotiationTest.java deleted file mode 100644 index fdfab72fd62..00000000000 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketNegotiationTest.java +++ /dev/null @@ -1,171 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.websocket.tests; - -import java.io.EOFException; -import java.io.IOException; -import java.io.InputStream; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.nio.charset.StandardCharsets; - -import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.util.B64Code; -import org.eclipse.jetty.websocket.client.WebSocketClient; -import org.eclipse.jetty.websocket.server.JettyWebSocketServlet; -import org.eclipse.jetty.websocket.server.JettyWebSocketServletFactory; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.startsWith; - -public class WebSocketNegotiationTest -{ - public static class EchoServlet extends JettyWebSocketServlet - { - @Override - public void configure(JettyWebSocketServletFactory factory) - { - factory.register(EchoSocket.class); - } - } - - private Server server; - private ServerConnector connector; - private WebSocketClient client; - - @BeforeEach - public void start() throws Exception - { - server = new Server(); - connector = new ServerConnector(server); - connector.setPort(0); - server.addConnector(connector); - - ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS); - contextHandler.setContextPath("/"); - contextHandler.addServlet(EchoServlet.class, "/"); - server.setHandler(contextHandler); - - client = new WebSocketClient(); - - server.start(); - client.start(); - } - - @AfterEach - public void stop() throws Exception - { - client.stop(); - server.stop(); - } - - @Test - public void testValidUpgradeRequest() throws Exception - { - Socket client = new Socket(); - client.connect(new InetSocketAddress("127.0.0.1", connector.getLocalPort())); - - HttpFields httpFields = newUpgradeRequest(null); - httpFields.remove(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL); - httpFields.add(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL, "testInvalidUpgradeRequest"); - String upgradeRequest = "GET / HTTP/1.1\r\n" + httpFields; - client.getOutputStream().write(upgradeRequest.getBytes(StandardCharsets.ISO_8859_1)); - String response = getUpgradeResponse(client.getInputStream()); - - assertThat(response, startsWith("HTTP/1.1 101 Switching Protocols")); - assertThat(response, containsString("Sec-WebSocket-Accept: +WahVcVmeMLKQUMm0fvPrjSjwzI=")); - } - - @Test - public void testInvalidUpgradeRequestNoKey() throws Exception - { - Socket client = new Socket(); - client.connect(new InetSocketAddress("127.0.0.1", connector.getLocalPort())); - - HttpFields httpFields = newUpgradeRequest(null); - httpFields.remove(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL); - httpFields.add(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL, "testInvalidUpgradeRequest"); - httpFields.remove(HttpHeader.SEC_WEBSOCKET_KEY); - - String upgradeRequest = "GET / HTTP/1.1\r\n" + httpFields; - client.getOutputStream().write(upgradeRequest.getBytes(StandardCharsets.ISO_8859_1)); - String response = getUpgradeResponse(client.getInputStream()); - - assertThat(response, containsString("400 Missing request header 'Sec-WebSocket-Key'")); - } - - - protected static HttpFields newUpgradeRequest(String extensions) - { - HttpFields fields = new HttpFields(); - fields.add(HttpHeader.HOST, "127.0.0.1"); - fields.add(HttpHeader.UPGRADE, "websocket"); - fields.add(HttpHeader.CONNECTION, "Upgrade"); - fields.add(HttpHeader.SEC_WEBSOCKET_KEY, new String(B64Code.encode("0123456701234567".getBytes()))); - fields.add(HttpHeader.SEC_WEBSOCKET_VERSION, "13"); - fields.add(HttpHeader.PRAGMA, "no-cache"); - fields.add(HttpHeader.CACHE_CONTROL, "no-cache"); - fields.add(HttpHeader.SEC_WEBSOCKET_SUBPROTOCOL, "test"); - if (extensions != null) - fields.add(HttpHeader.SEC_WEBSOCKET_EXTENSIONS, extensions); - - return fields; - } - - protected static String getUpgradeResponse(InputStream in) throws IOException - { - int state = 0; - StringBuilder buffer = new StringBuilder(); - while (state < 4) - { - int i = in.read(); - if (i < 0) - throw new EOFException(); - int b = (byte)(i & 0xff); - buffer.append((char)b); - switch (state) - { - case 0: - state = (b == '\r')?1:0; - break; - case 1: - state = (b == '\n')?2:0; - break; - case 2: - state = (b == '\r')?3:0; - break; - case 3: - state = (b == '\n')?4:0; - break; - default: - state = 0; - } - } - - return buffer.toString(); - } -} \ No newline at end of file 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 b5b8e39c2f4..d976112bb37 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 @@ -121,7 +121,7 @@ public final class RFC6455Handshaker implements Handshaker } if (negotiation.getKey() == null) - throw new BadMessageException("not upgraded no key"); + throw new BadMessageException("Missing request header 'Sec-WebSocket-Key'"); // Negotiate the FrameHandler FrameHandler handler = negotiator.negotiate(negotiation); diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketNegotiationTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketNegotiationTest.java index a3c310c8cf8..3068a0b8f26 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketNegotiationTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketNegotiationTest.java @@ -19,6 +19,9 @@ package org.eclipse.jetty.websocket.core; import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -27,6 +30,7 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jetty.client.HttpRequest; import org.eclipse.jetty.client.HttpResponse; +import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.util.Callback; @@ -44,11 +48,12 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -public class WebSocketNegotiationTest +public class WebSocketNegotiationTest extends WebSocketTester { public static class EchoFrameHandler extends TestFrameHandler { @@ -78,7 +83,6 @@ public class WebSocketNegotiationTest return new EchoFrameHandler(); } - String subprotocol = negotiation.getOfferedSubprotocols().get(0); negotiation.setSubprotocol(subprotocol); switch (subprotocol) @@ -91,13 +95,6 @@ public class WebSocketNegotiationTest negotiation.setNegotiatedExtensions(List.of(ExtensionConfig.parse("permessage-deflate;server_no_context_takeover"))); break; - case "testInvalidExtensionParameter": - break; - - case "testAcceptTwoExtensionsOfSameName": - // We should automatically be selecting just one extension out of these two - break; - case "testNotAcceptingExtensions": negotiation.setNegotiatedExtensions(Collections.EMPTY_LIST); break; @@ -106,6 +103,12 @@ public class WebSocketNegotiationTest negotiation.setSubprotocol(null); break; + case "test": + case "testInvalidExtensionParameter": + case "testAcceptTwoExtensionsOfSameName": + case "testInvalidUpgradeRequest": + break; + default: return null; } @@ -189,7 +192,6 @@ public class WebSocketNegotiationTest assertThat(extensionHeader.get(5, TimeUnit.SECONDS), is("permessage-deflate;server_no_context_takeover")); } - @Test public void testInvalidExtensionParameter() throws Exception { @@ -302,4 +304,36 @@ public class WebSocketNegotiationTest assertThat(t.getMessage(), containsString("500 Server Error")); } } + + @Test + public void testValidUpgradeRequest() throws Exception + { + Socket client = new Socket(); + client.connect(new InetSocketAddress("127.0.0.1", server.getLocalPort())); + + HttpFields httpFields = newUpgradeRequest(null); + String upgradeRequest = "GET / HTTP/1.1\r\n" + httpFields; + client.getOutputStream().write(upgradeRequest.getBytes(StandardCharsets.ISO_8859_1)); + String response = getUpgradeResponse(client.getInputStream()); + + assertThat(response, startsWith("HTTP/1.1 101 Switching Protocols")); + assertThat(response, containsString("Sec-WebSocket-Protocol: test")); + assertThat(response, containsString("Sec-WebSocket-Accept: +WahVcVmeMLKQUMm0fvPrjSjwzI=")); + } + + @Test + public void testInvalidUpgradeRequestNoKey() throws Exception + { + Socket client = new Socket(); + client.connect(new InetSocketAddress("127.0.0.1", server.getLocalPort())); + + HttpFields httpFields = newUpgradeRequest(null); + httpFields.remove(HttpHeader.SEC_WEBSOCKET_KEY); + + String upgradeRequest = "GET / HTTP/1.1\r\n" + httpFields; + client.getOutputStream().write(upgradeRequest.getBytes(StandardCharsets.ISO_8859_1)); + String response = getUpgradeResponse(client.getInputStream()); + + assertThat(response, containsString("400 Bad Request")); + } } \ No newline at end of file diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketTester.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketTester.java index ecb38860f09..27cdf501763 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketTester.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/core/WebSocketTester.java @@ -84,17 +84,9 @@ public class WebSocketTester { return newClient(port, false, extensions); } - - protected Socket newClient(int port, boolean tls, String extensions) throws Exception + + protected static HttpFields newUpgradeRequest(String extensions) { - Socket client; - if (!tls) - client = new Socket(); - else - client = sslContextFactory.newSslSocket(); - - client.connect(new InetSocketAddress("127.0.0.1", port)); - HttpFields fields = new HttpFields(); fields.add(HttpHeader.HOST, "127.0.0.1"); fields.add(HttpHeader.UPGRADE, "websocket"); @@ -107,10 +99,11 @@ public class WebSocketTester if (extensions != null) fields.add(HttpHeader.SEC_WEBSOCKET_EXTENSIONS, extensions); - client.getOutputStream().write(("GET / HTTP/1.1\r\n" + fields.toString()).getBytes(StandardCharsets.ISO_8859_1)); - - InputStream in = client.getInputStream(); + return fields; + } + protected static String getUpgradeResponse(InputStream in) throws IOException + { int state = 0; StringBuilder buffer = new StringBuilder(); while (state < 4) @@ -139,7 +132,23 @@ public class WebSocketTester } } - String response = buffer.toString(); + return buffer.toString(); + } + + protected Socket newClient(int port, boolean tls, String extensions) throws Exception + { + Socket client; + if (!tls) + client = new Socket(); + else + client = sslContextFactory.newSslSocket(); + + client.connect(new InetSocketAddress("127.0.0.1", port)); + + String upgradeRequest = "GET / HTTP/1.1\r\n" + newUpgradeRequest(extensions); + client.getOutputStream().write(upgradeRequest.getBytes(StandardCharsets.ISO_8859_1)); + String response = getUpgradeResponse(client.getInputStream()); + assertThat(response, startsWith("HTTP/1.1 101 Switching Protocols")); assertThat(response, containsString("Sec-WebSocket-Protocol: test")); assertThat(response, containsString("Sec-WebSocket-Accept: +WahVcVmeMLKQUMm0fvPrjSjwzI="));