Issue #5096 - add WebSocketFilter lazily if only using JettyWebSocket…

…Servlet Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Aug 3, 2020 · 9cde41a · 9cde41a
1 parent fe56838
commit 9cde41a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 8 deletions.
diff --git a/...erver/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/...erver/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java
@@ -43,6 +43,7 @@
 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 static JettyWebSocketServerContainer ensureContainer(ServletContext servl
 
     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 static JettyWebSocketServerContainer ensureContainer(ServletContext servl
      */
     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 void addMapping(String pathSpec, JettyWebSocketCreator creator)
         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/.../org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java b/.../org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java
@@ -22,13 +22,11 @@
 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 static void configure(ServletContextHandler context, Configurator configu
     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-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java b/...jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java
@@ -27,20 +27,23 @@
 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;
 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
 {
     private Server server;
     private ServerConnector connector;
     private WebSocketClient client;
+    private ServletContextHandler contextHandler;
 
     @BeforeEach
     public void start() throws Exception
@@ -49,12 +52,11 @@ public void start() throws Exception
         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 void stop() throws Exception
     }
 
     @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<Session> connect = client.connect(socket, uri);