From 9cde41a75cbf006fa568e0b40517c41556219949 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 3 Aug 2020 19:01:41 +1000 Subject: [PATCH] Issue #5096 - add WebSocketFilter lazily if only using JettyWebSocketServlet Signed-off-by: Lachlan Roberts --- .../server/JettyWebSocketServerContainer.java | 4 ++++ ...yWebSocketServletContainerInitializer.java | 5 +---- .../tests/JettyWebSocketFilterTest.java | 22 +++++++++++++++---- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 5eadef61ef3..0eb52089704 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -43,6 +43,7 @@ import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; import org.eclipse.jetty.websocket.server.internal.JettyServerFrameHandlerFactory; import org.eclipse.jetty.websocket.util.ReflectUtils; +import org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter; import org.eclipse.jetty.websocket.util.server.internal.FrameHandlerFactory; import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; import org.slf4j.Logger; @@ -86,6 +87,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements private static final Logger LOG = LoggerFactory.getLogger(JettyWebSocketServerContainer.class); + private final ServletContextHandler contextHandler; private final WebSocketMapping webSocketMapping; private final WebSocketComponents webSocketComponents; private final FrameHandlerFactory frameHandlerFactory; @@ -104,6 +106,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements */ JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMapping webSocketMapping, WebSocketComponents webSocketComponents, Executor executor) { + this.contextHandler = contextHandler; this.webSocketMapping = webSocketMapping; this.webSocketComponents = webSocketComponents; this.executor = executor; @@ -128,6 +131,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements if (webSocketMapping.getMapping(ps) != null) throw new WebSocketException("Duplicate WebSocket Mapping for PathSpec"); + WebSocketUpgradeFilter.ensureFilter(contextHandler.getServletContext()); webSocketMapping.addMapping(ps, (req, resp) -> creator.createWebSocket(new JettyServerUpgradeRequest(req), new JettyServerUpgradeResponse(resp)), frameHandlerFactory, customizer); diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java index 3b164c017f8..4740fbc75f8 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java @@ -22,13 +22,11 @@ import java.util.Set; import javax.servlet.ServletContainerInitializer; import javax.servlet.ServletContext; -import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.listener.ContainerInitializer; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents; import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; -import org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter; import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,12 +89,11 @@ public class JettyWebSocketServletContainerInitializer implements ServletContain private static JettyWebSocketServerContainer initialize(ServletContextHandler context) { WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServletContext()); - FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(context.getServletContext()); WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY); JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(context.getServletContext()); if (LOG.isDebugEnabled()) - LOG.debug("configureContext {} {} {} {}", container, mapping, filterHolder, components); + LOG.debug("configureContext {} {} {}", container, mapping, components); return container; } diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java index 488eb796a9b..e1786c196b8 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -34,6 +35,7 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class JettyWebSocketFilterTest @@ -41,6 +43,7 @@ public class JettyWebSocketFilterTest private Server server; private ServerConnector connector; private WebSocketClient client; + private ServletContextHandler contextHandler; @BeforeEach public void start() throws Exception @@ -49,12 +52,11 @@ public class JettyWebSocketFilterTest connector = new ServerConnector(server); server.addConnector(connector); - ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS); + contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS); contextHandler.setContextPath("/"); server.setHandler(contextHandler); - JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> - container.addMapping("/", (req, resp) -> new EchoSocket())); + JettyWebSocketServletContainerInitializer.configure(contextHandler, null); server.start(); client = new WebSocketClient(); @@ -69,8 +71,20 @@ public class JettyWebSocketFilterTest } @Test - public void test() throws Exception + public void testLazyWebSocketUpgradeFilter() throws Exception { + // JettyWebSocketServerContainer has already been created. + JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(contextHandler.getServletContext()); + assertNotNull(container); + + // We should have no WebSocketUpgradeFilter installed because we have added no mappings. + assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); + + // After mapping is added we have an UpgradeFilter. + container.addMapping("/", EchoSocket.class); + assertThat(contextHandler.getServletHandler().getFilters().length, is(1)); + + // Test we can upgrade to websocket and send a message. URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath"); EventSocket socket = new EventSocket(); CompletableFuture connect = client.connect(socket, uri);