diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java index 37db5c9c88a..f028132af0a 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.websocket.tests; import java.io.IOException; import java.io.InterruptedIOException; +import java.net.ConnectException; import java.net.URI; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -41,6 +42,7 @@ import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.ClientConnector; +import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.SecureRequestCustomizer; @@ -49,10 +51,12 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.log.StacklessLogging; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.api.UpgradeException; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.server.JettyWebSocketServlet; import org.eclipse.jetty.websocket.server.JettyWebSocketServletFactory; @@ -63,6 +67,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsStringIgnoringCase; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -108,7 +113,7 @@ public class WebSocketOverHTTP2Test ServletContextHandler context = new ServletContextHandler(server, "/"); context.addServlet(new ServletHolder(servlet), "/ws/*"); - JettyWebSocketServletContainerInitializer.initialize(context); + JettyWebSocketServletContainerInitializer.configure(context, null); server.start(); } @@ -229,12 +234,85 @@ public class WebSocketOverHTTP2Test assertTrue(wsEndPoint.closeLatch.await(5, TimeUnit.SECONDS)); } + @Test void testWebSocketConnectPortDoesNotExist() throws Exception + { + startServer(); + startClient(clientConnector -> new ClientConnectionFactoryOverHTTP2.H2(new HTTP2Client(clientConnector))); + + EventSocket wsEndPoint = new EventSocket(); + URI uri = URI.create("ws://localhost:" + (connector.getLocalPort()+1) + "/ws/echo"); + + ExecutionException failure = Assertions.assertThrows(ExecutionException.class, () -> + wsClient.connect(wsEndPoint, uri).get(5, TimeUnit.SECONDS)); + + Throwable cause = failure.getCause(); + assertThat(cause, instanceOf(ConnectException.class)); + assertThat(cause.getMessage(), containsStringIgnoringCase("Connection refused")); + } + + @Test void testWebSocketNotFound() throws Exception + { + startServer(); + startClient(clientConnector -> new ClientConnectionFactoryOverHTTP2.H2(new HTTP2Client(clientConnector))); + + EventSocket wsEndPoint = new EventSocket(); + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/nothing"); + + ExecutionException failure = Assertions.assertThrows(ExecutionException.class, () -> + wsClient.connect(wsEndPoint, uri).get(5, TimeUnit.SECONDS)); + + Throwable cause = failure.getCause(); + assertThat(cause, instanceOf(UpgradeException.class)); + assertThat(cause.getMessage(), containsStringIgnoringCase("Unexpected HTTP Response Status Code: 501")); + } + + @Test void testNotNegotiated() throws Exception + { + startServer(); + startClient(clientConnector -> new ClientConnectionFactoryOverHTTP2.H2(new HTTP2Client(clientConnector))); + + EventSocket wsEndPoint = new EventSocket(); + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/ws/null"); + + ExecutionException failure = Assertions.assertThrows(ExecutionException.class, () -> + wsClient.connect(wsEndPoint, uri).get(5, TimeUnit.SECONDS)); + + Throwable cause = failure.getCause(); + assertThat(cause, instanceOf(UpgradeException.class)); + assertThat(cause.getMessage(), containsStringIgnoringCase("Unexpected HTTP Response Status Code: 503")); + } + + @Test void testThrowFromCreator() throws Exception + { + startServer(); + startClient(clientConnector -> new ClientConnectionFactoryOverHTTP2.H2(new HTTP2Client(clientConnector))); + + EventSocket wsEndPoint = new EventSocket(); + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/ws/throw"); + + ExecutionException failure; + try (StacklessLogging stacklessLogging = new StacklessLogging(HttpChannel.class)) + { + failure = Assertions.assertThrows(ExecutionException.class, () -> + wsClient.connect(wsEndPoint, uri).get(5, TimeUnit.SECONDS)); + } + + Throwable cause = failure.getCause(); + assertThat(cause, instanceOf(UpgradeException.class)); + assertThat(cause.getMessage(), containsStringIgnoringCase("Unexpected HTTP Response Status Code: 500")); + } + private static class TestJettyWebSocketServlet extends JettyWebSocketServlet { @Override protected void configure(JettyWebSocketServletFactory factory) { factory.addMapping("/ws/echo", (request, response) -> new EchoSocket()); + factory.addMapping("/ws/null", (request, response) -> null); + factory.addMapping("/ws/throw", (request, response) -> + { + throw new RuntimeException("throwing from creator"); + }); } } } 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 5ad5283d924..6187c99e358 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 @@ -140,7 +140,7 @@ public abstract class Negotiation } } - public abstract boolean isSuccessful(); + public abstract boolean validateHeaders(); public String getVersion() { diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java index fc8f1dbfe83..0eff2f5e3c4 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java @@ -72,12 +72,8 @@ public abstract class AbstractHandshaker implements Handshaker // Negotiate the FrameHandler FrameHandler handler = negotiator.negotiate(negotiation); - if (handler == null) - { - if (LOG.isDebugEnabled()) - LOG.debug("not upgraded: no frame handler provided {}", request); + if (!validateFrameHandler(handler, response)) return false; - } // Handle error responses Request baseRequest = negotiation.getBaseRequest(); @@ -175,9 +171,11 @@ public abstract class AbstractHandshaker implements Handshaker protected abstract Negotiation newNegotiation(HttpServletRequest request, HttpServletResponse response, WebSocketComponents webSocketComponents); + protected abstract boolean validateFrameHandler(FrameHandler frameHandler, HttpServletResponse response); + protected boolean validateNegotiation(Negotiation negotiation) { - if (!negotiation.isSuccessful()) + if (!negotiation.validateHeaders()) { if (LOG.isDebugEnabled()) LOG.debug("not upgraded: no upgrade header or connection upgrade", negotiation.getBaseRequest()); 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 7053def75d9..4815e04f87c 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 @@ -32,6 +32,7 @@ import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.websocket.core.FrameHandler; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.internal.WebSocketConnection; import org.eclipse.jetty.websocket.core.internal.WebSocketCore; @@ -80,6 +81,19 @@ public final class RFC6455Handshaker extends AbstractHandshaker return true; } + @Override + protected boolean validateFrameHandler(FrameHandler frameHandler, HttpServletResponse response) + { + if (frameHandler == null) + { + if (LOG.isDebugEnabled()) + LOG.debug("not upgraded: no frame handler provided"); + return false; + } + + return true; + } + @Override protected WebSocketConnection createWebSocketConnection(Request baseRequest, WebSocketCoreSession coreSession) { diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Negotiation.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Negotiation.java index 1703787ab75..745a67c2760 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Negotiation.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC6455Negotiation.java @@ -78,7 +78,7 @@ public class RFC6455Negotiation extends Negotiation } @Override - public boolean isSuccessful() + public boolean validateHeaders() { return successful; } diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Handshaker.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Handshaker.java index ae9c19364c1..378fe50eadc 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Handshaker.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Handshaker.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.websocket.core.FrameHandler; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.internal.WebSocketConnection; import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession; @@ -62,6 +63,20 @@ public class RFC8441Handshaker extends AbstractHandshaker return new RFC8441Negotiation(Request.getBaseRequest(request), request, response, webSocketComponents); } + @Override + protected boolean validateFrameHandler(FrameHandler frameHandler, HttpServletResponse response) + { + if (frameHandler == null) + { + if (LOG.isDebugEnabled()) + LOG.debug("not upgraded: no frame handler provided"); + + response.setStatus(HttpStatus.SERVICE_UNAVAILABLE_503); + } + + return true; + } + @Override protected WebSocketConnection createWebSocketConnection(Request baseRequest, WebSocketCoreSession coreSession) { diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Negotiation.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Negotiation.java index eaee84d6437..d0e0452b087 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Negotiation.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/RFC8441Negotiation.java @@ -35,7 +35,7 @@ public class RFC8441Negotiation extends Negotiation } @Override - public boolean isSuccessful() + public boolean validateHeaders() { MetaData.Request metaData = getBaseRequest().getMetaData(); if (metaData == null)