From 0493a111067fc57a1e55da2e6eb8608c34593d9f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 13 Nov 2020 15:44:18 +1100 Subject: [PATCH 1/7] Use Filter name to identify the WebSocketUpgradeFilter. Don't allow configuration of WebSocketMapping attribute. The WebSocketUpgradeFilter is identified by it's name, which must be set as the fully qualified class name. Signed-off-by: Lachlan Roberts --- ...xWebSocketServletContainerInitializer.java | 2 +- .../JavaxWebSocketServerContainer.java | 2 +- .../server/JettyWebSocketServerContainer.java | 2 +- .../tests/JettyWebSocketFilterTest.java | 6 ++--- .../util/server/WebSocketUpgradeFilter.java | 23 +++--------------- .../server/internal/WebSocketMapping.java | 24 +++++-------------- 6 files changed, 15 insertions(+), 44 deletions(-) diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java index 4325b3240c89..2673ea16fccc 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java @@ -153,7 +153,7 @@ private static ServerContainer initialize(ServletContextHandler context) { WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServer(), context.getServletContext()); FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(context.getServletContext()); - WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY); + WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext()); serverContainer = JavaxWebSocketServerContainer.ensureContainer(context.getServletContext()); if (LOG.isDebugEnabled()) diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java index f9218c819d95..4799fcee6a97 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java @@ -98,7 +98,7 @@ public static JavaxWebSocketServerContainer ensureContainer(ServletContext servl // Create the Jetty ServerContainer implementation container = new JavaxWebSocketServerContainer( - WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY), + WebSocketMapping.ensureMapping(servletContext), WebSocketServerComponents.getWebSocketComponents(servletContext), coreClientSupplier); contextHandler.addManaged(container); 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 1729cd545f0f..4837b7dfd5bf 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 @@ -77,7 +77,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl // Create the Jetty ServerContainer implementation container = new JettyWebSocketServerContainer( contextHandler, - WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY), + WebSocketMapping.ensureMapping(servletContext), WebSocketServerComponents.getWebSocketComponents(servletContext), executor); servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); contextHandler.addManaged(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 c0be53db2630..477098c266ba 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 @@ -93,7 +93,7 @@ public void testWebSocketUpgradeFilter() throws Exception // After mapping is added we have an UpgradeFilter. assertThat(contextHandler.getServletHandler().getFilters().length, is(1)); - FilterHolder filterHolder = contextHandler.getServletHandler().getFilter("WebSocketUpgradeFilter"); + FilterHolder filterHolder = contextHandler.getServletHandler().getFilter(WebSocketUpgradeFilter.class.getName()); assertNotNull(filterHolder); assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED)); assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class)); @@ -127,7 +127,7 @@ public void testLazyWebSocketUpgradeFilter() throws Exception // After mapping is added we have an UpgradeFilter. container.addMapping("/", EchoSocket.class); assertThat(contextHandler.getServletHandler().getFilters().length, is(1)); - FilterHolder filterHolder = contextHandler.getServletHandler().getFilter("WebSocketUpgradeFilter"); + FilterHolder filterHolder = contextHandler.getServletHandler().getFilter(WebSocketUpgradeFilter.class.getName()); assertNotNull(filterHolder); assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED)); assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class)); @@ -164,7 +164,7 @@ public void init() // After mapping is added we have an UpgradeFilter. assertThat(contextHandler.getServletHandler().getFilters().length, is(1)); - FilterHolder filterHolder = contextHandler.getServletHandler().getFilter("WebSocketUpgradeFilter"); + FilterHolder filterHolder = contextHandler.getServletHandler().getFilter(WebSocketUpgradeFilter.class.getName()); assertNotNull(filterHolder); assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED)); assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class)); diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java index 176544be380a..256528a2cfe2 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java @@ -77,11 +77,6 @@ public class WebSocketUpgradeFilter implements Filter, Dumpable private static final Logger LOG = LoggerFactory.getLogger(WebSocketUpgradeFilter.class); private static final AutoLock LOCK = new AutoLock(); - /** - * The init parameter name used to define {@link ServletContext} attribute used to share the {@link WebSocketMapping}. - */ - public static final String MAPPING_ATTRIBUTE_INIT_PARAM = "jetty.websocket.WebSocketMapping"; - /** * Return any {@link WebSocketUpgradeFilter} already present on the {@link ServletContext}. * @@ -92,12 +87,7 @@ private static FilterHolder getFilter(ServletContext servletContext) { ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext)); ServletHandler servletHandler = contextHandler.getChildHandlerByClass(ServletHandler.class); - for (FilterHolder holder : servletHandler.getFilters()) - { - if (holder.getInitParameter(MAPPING_ATTRIBUTE_INIT_PARAM) != null) - return holder; - } - return null; + return servletHandler.getFilter(WebSocketUpgradeFilter.class.getName()); } /** @@ -121,11 +111,9 @@ public static FilterHolder ensureFilter(ServletContext servletContext) if (existingFilter != null) return existingFilter; - final String name = "WebSocketUpgradeFilter"; final String pathSpec = "/*"; FilterHolder holder = new FilterHolder(new WebSocketUpgradeFilter()); - holder.setName(name); - holder.setInitParameter(MAPPING_ATTRIBUTE_INIT_PARAM, WebSocketMapping.DEFAULT_KEY); + holder.setName(WebSocketUpgradeFilter.class.getName()); holder.setAsyncSupported(true); ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext)); @@ -174,12 +162,7 @@ public void dump(Appendable out, String indent) throws IOException @Override public void init(FilterConfig config) throws ServletException { - final ServletContext context = config.getServletContext(); - - String mappingKey = config.getInitParameter(MAPPING_ATTRIBUTE_INIT_PARAM); - if (mappingKey == null) - throw new ServletException("the WebSocketMapping init param must be set"); - mapping = WebSocketMapping.ensureMapping(context, mappingKey); + mapping = WebSocketMapping.ensureMapping(config.getServletContext()); String max = config.getInitParameter("idleTimeout"); if (max == null) diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java index e93c0c4c0192..d7bb24b930ad 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java @@ -59,21 +59,11 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener { private static final Logger LOG = LoggerFactory.getLogger(WebSocketMapping.class); + public static final String WEBSOCKET_MAPPING_ATTRIBUTE = WebSocketMapping.class.getName(); - public static WebSocketMapping getMapping(ServletContext servletContext, String mappingKey) + public static WebSocketMapping getMapping(ServletContext servletContext) { - Object mappingObject = servletContext.getAttribute(mappingKey); - if (mappingObject != null) - { - if (mappingObject instanceof WebSocketMapping) - return (WebSocketMapping)mappingObject; - else - throw new IllegalStateException( - String.format("ContextHandler attribute %s is not of type WebSocketMapping: {%s}", - mappingKey, mappingObject.toString())); - } - - return null; + return (WebSocketMapping)servletContext.getAttribute(WEBSOCKET_MAPPING_ATTRIBUTE); } public WebSocketCreator getMapping(PathSpec pathSpec) @@ -82,13 +72,13 @@ public WebSocketCreator getMapping(PathSpec pathSpec) return cn == null ? null : cn.getWebSocketCreator(); } - public static WebSocketMapping ensureMapping(ServletContext servletContext, String mappingKey) + public static WebSocketMapping ensureMapping(ServletContext servletContext) { - WebSocketMapping mapping = getMapping(servletContext, mappingKey); + WebSocketMapping mapping = getMapping(servletContext); if (mapping == null) { mapping = new WebSocketMapping(WebSocketServerComponents.getWebSocketComponents(servletContext)); - servletContext.setAttribute(mappingKey, mapping); + servletContext.setAttribute(WEBSOCKET_MAPPING_ATTRIBUTE, mapping); } return mapping; @@ -133,8 +123,6 @@ else if (rawSpec.startsWith("uri-template|")) throw new IllegalArgumentException("Unrecognized path spec syntax [" + rawSpec + "]"); } - public static final String DEFAULT_KEY = "jetty.websocket.defaultMapping"; - private final PathMappings mappings = new PathMappings<>(); private final WebSocketComponents components; private final Handshaker handshaker = Handshaker.newInstance(); From 165beff3f36fddf0c9a5e105a39a9ac12e41d658 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Sat, 14 Nov 2020 10:43:14 +1100 Subject: [PATCH 2/7] add test for multiple WebSocketUpgradeFilters Signed-off-by: Lachlan Roberts --- .../tests/JettyWebSocketFilterTest.java | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) 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 477098c266ba..b269f72f4c4c 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 @@ -18,9 +18,17 @@ package org.eclipse.jetty.websocket.tests; +import java.io.IOException; import java.net.URI; +import java.util.Arrays; +import java.util.EnumSet; +import java.util.List; +import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import javax.servlet.DispatcherType; import javax.servlet.http.HttpServlet; import org.eclipse.jetty.server.Server; @@ -30,6 +38,8 @@ import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage; +import org.eclipse.jetty.websocket.api.annotations.WebSocket; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; @@ -38,9 +48,11 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class JettyWebSocketFilterTest @@ -182,4 +194,101 @@ public void init() String msg = socket.textMessages.poll(); assertThat(msg, is("hello world")); } + + @Test + public void testMultipleWebSocketUpgradeFilter() throws Exception + { + String idleTimeoutFilter1 = "4999"; + String idleTimeoutFilter2 = "3999"; + start((context, container) -> + { + ServletContextHandler contextHandler = Objects.requireNonNull(ServletContextHandler.getServletContextHandler(context)); + + // This filter replaces the default filter as we use the pre-defined name. + FilterHolder filterHolder = new FilterHolder(WebSocketUpgradeFilter.class); + filterHolder.setName(WebSocketUpgradeFilter.class.getName()); + filterHolder.setInitParameter("idleTimeout", idleTimeoutFilter1); + contextHandler.addFilter(filterHolder, "/primaryFilter/*", EnumSet.of(DispatcherType.REQUEST)); + + // This is an additional filter. + filterHolder = new FilterHolder(WebSocketUpgradeFilter.class); + filterHolder.setName("Secondary Upgrade Filter"); + filterHolder.setInitParameter("idleTimeout", idleTimeoutFilter2); + contextHandler.addFilter(filterHolder, "/secondaryFilter/*", EnumSet.of(DispatcherType.REQUEST)); + + // Add mappings to the server container (same WebSocketMappings is referenced by both upgrade filters). + container.addMapping("/echo", EchoSocket.class); + container.addMapping("/primaryFilter/echo", LowerCaseEchoSocket.class); + container.addMapping("/secondaryFilter/echo", UpperCaseEchoSocket.class); + }); + + // Verify we have manually added 2 WebSocketUpgrade Filters. + List upgradeFilters = Arrays.stream(contextHandler.getServletHandler().getFilters()) + .filter(holder -> holder.getFilter() instanceof WebSocketUpgradeFilter) + .collect(Collectors.toList()); + assertThat(contextHandler.getServletHandler().getFilters().length, is(2)); + assertThat(upgradeFilters.size(), is(2)); + for (FilterHolder filterHolder : upgradeFilters) + { + assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED)); + assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + } + + // The /echo path should not match either of the upgrade filters even though it has a valid mapping, we get 404 response. + URI firstUri = URI.create("ws://localhost:" + connector.getLocalPort() + "/echo"); + ExecutionException error = assertThrows(ExecutionException.class, () -> client.connect(new EventSocket(), firstUri).get(5, TimeUnit.SECONDS)); + assertThat(error.getMessage(), containsString("404 Not Found")); + + // The /primaryFilter/echo path should convert to lower case and have idleTimeout configured on the first upgradeFilter. + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/primaryFilter/echo"); + EventSocket socket = new EventSocket(); + CompletableFuture connect = client.connect(socket, uri); + try (Session session = connect.get(5, TimeUnit.SECONDS)) + { + session.getRemote().sendString("hElLo wOrLd"); + session.getRemote().sendString("getIdleTimeout"); + } + assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(socket.textMessages.poll(), is("hello world")); + assertThat(socket.textMessages.poll(), is(idleTimeoutFilter1)); + + // The /secondaryFilter/echo path should convert to upper case and have idleTimeout configured on the second upgradeFilter. + uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/secondaryFilter/echo"); + socket = new EventSocket(); + connect = client.connect(socket, uri); + try (Session session = connect.get(5, TimeUnit.SECONDS)) + { + session.getRemote().sendString("hElLo wOrLd"); + session.getRemote().sendString("getIdleTimeout"); + } + assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(socket.textMessages.poll(), is("HELLO WORLD")); + assertThat(socket.textMessages.poll(), is(idleTimeoutFilter2)); + } + + @WebSocket + public static class LowerCaseEchoSocket + { + @OnWebSocketMessage + public void onMessage(Session session, String message) throws IOException + { + if ("getIdleTimeout".equals(message)) + session.getRemote().sendString(Long.toString(session.getIdleTimeout().toMillis())); + else + session.getRemote().sendString(message.toLowerCase()); + } + } + + @WebSocket + public static class UpperCaseEchoSocket + { + @OnWebSocketMessage + public void onMessage(Session session, String message) throws IOException + { + if ("getIdleTimeout".equals(message)) + session.getRemote().sendString(Long.toString(session.getIdleTimeout().toMillis())); + else + session.getRemote().sendString(message.toUpperCase()); + } + } } From 6dad0b1b7e62b47efb2d4e8b748446a15479d4f5 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Sat, 14 Nov 2020 10:46:09 +1100 Subject: [PATCH 3/7] rename WebSocketMapping to WebSocketMappings Signed-off-by: Lachlan Roberts --- ...xWebSocketServletContainerInitializer.java | 4 ++-- .../JavaxWebSocketServerContainer.java | 24 +++++++++---------- .../src/test/resources/alt-filter-web.xml | 8 ++----- .../server/JettyWebSocketServerContainer.java | 18 +++++++------- .../server/JettyWebSocketServlet.java | 18 +++++++------- ...yWebSocketServletContainerInitializer.java | 4 ++-- .../util/server/WebSocketUpgradeFilter.java | 8 +++---- ...ketMapping.java => WebSocketMappings.java} | 22 ++++++++--------- 8 files changed, 51 insertions(+), 55 deletions(-) rename jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/{WebSocketMapping.java => WebSocketMappings.java} (93%) diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java index 2673ea16fccc..41933a8f0dba 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java @@ -40,7 +40,7 @@ import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents; import org.eclipse.jetty.websocket.javax.server.internal.JavaxWebSocketServerContainer; import org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter; -import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; +import org.eclipse.jetty.websocket.util.server.internal.WebSocketMappings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -153,7 +153,7 @@ private static ServerContainer initialize(ServletContextHandler context) { WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServer(), context.getServletContext()); FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(context.getServletContext()); - WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext()); + WebSocketMappings mapping = WebSocketMappings.ensureMapping(context.getServletContext()); serverContainer = JavaxWebSocketServerContainer.ensureContainer(context.getServletContext()); if (LOG.isDebugEnabled()) diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java index 4799fcee6a97..55708c7d3a51 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java @@ -43,7 +43,7 @@ import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; import org.eclipse.jetty.websocket.util.InvalidSignatureException; import org.eclipse.jetty.websocket.util.ReflectUtils; -import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; +import org.eclipse.jetty.websocket.util.server.internal.WebSocketMappings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -98,7 +98,7 @@ public static JavaxWebSocketServerContainer ensureContainer(ServletContext servl // Create the Jetty ServerContainer implementation container = new JavaxWebSocketServerContainer( - WebSocketMapping.ensureMapping(servletContext), + WebSocketMappings.ensureMapping(servletContext), WebSocketServerComponents.getWebSocketComponents(servletContext), coreClientSupplier); contextHandler.addManaged(container); @@ -109,7 +109,7 @@ public static JavaxWebSocketServerContainer ensureContainer(ServletContext servl return container; } - private final WebSocketMapping webSocketMapping; + private final WebSocketMappings webSocketMappings; private final JavaxWebSocketServerFrameHandlerFactory frameHandlerFactory; private List> deferredEndpointClasses; private List deferredEndpointConfigs; @@ -117,31 +117,31 @@ public static JavaxWebSocketServerContainer ensureContainer(ServletContext servl /** * Main entry point for {@link JavaxWebSocketServletContainerInitializer}. * - * @param webSocketMapping the {@link WebSocketMapping} that this container belongs to + * @param webSocketMappings the {@link WebSocketMappings} that this container belongs to */ - public JavaxWebSocketServerContainer(WebSocketMapping webSocketMapping) + public JavaxWebSocketServerContainer(WebSocketMappings webSocketMappings) { - this(webSocketMapping, new WebSocketComponents()); + this(webSocketMappings, new WebSocketComponents()); } - public JavaxWebSocketServerContainer(WebSocketMapping webSocketMapping, WebSocketComponents components) + public JavaxWebSocketServerContainer(WebSocketMappings webSocketMappings, WebSocketComponents components) { super(components); - this.webSocketMapping = webSocketMapping; + this.webSocketMappings = webSocketMappings; this.frameHandlerFactory = new JavaxWebSocketServerFrameHandlerFactory(this); } /** * Main entry point for {@link JavaxWebSocketServletContainerInitializer}. * - * @param webSocketMapping the {@link WebSocketMapping} that this container belongs to + * @param webSocketMappings the {@link WebSocketMappings} that this container belongs to * @param components the {@link WebSocketComponents} instance to use * @param coreClientSupplier the supplier of the {@link WebSocketCoreClient} instance to use */ - public JavaxWebSocketServerContainer(WebSocketMapping webSocketMapping, WebSocketComponents components, Function coreClientSupplier) + public JavaxWebSocketServerContainer(WebSocketMappings webSocketMappings, WebSocketComponents components, Function coreClientSupplier) { super(components, coreClientSupplier); - this.webSocketMapping = webSocketMapping; + this.webSocketMappings = webSocketMappings; this.frameHandlerFactory = new JavaxWebSocketServerFrameHandlerFactory(this); } @@ -264,7 +264,7 @@ private void addEndpointMapping(ServerEndpointConfig config) throws DeploymentEx frameHandlerFactory.getMetadata(config.getEndpointClass(), config); JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, getExtensionRegistry()); PathSpec pathSpec = new UriTemplatePathSpec(config.getPath()); - webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer); + webSocketMappings.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer); } catch (InvalidSignatureException e) { diff --git a/jetty-websocket/websocket-javax-tests/src/test/resources/alt-filter-web.xml b/jetty-websocket/websocket-javax-tests/src/test/resources/alt-filter-web.xml index ccf98c85030a..867fead5585c 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/resources/alt-filter-web.xml +++ b/jetty-websocket/websocket-javax-tests/src/test/resources/alt-filter-web.xml @@ -12,16 +12,12 @@ - wsuf-test + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter - - jetty.websocket.WebSocketMapping - jetty.websocket.defaultMapping - - wsuf-test + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter /echo/* 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 4837b7dfd5bf..df55e866e82f 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 @@ -47,7 +47,7 @@ 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.eclipse.jetty.websocket.util.server.internal.WebSocketMappings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -77,7 +77,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl // Create the Jetty ServerContainer implementation container = new JettyWebSocketServerContainer( contextHandler, - WebSocketMapping.ensureMapping(servletContext), + WebSocketMappings.ensureMapping(servletContext), WebSocketServerComponents.getWebSocketComponents(servletContext), executor); servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); contextHandler.addManaged(container); @@ -90,7 +90,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 WebSocketMappings webSocketMappings; private final WebSocketComponents components; private final FrameHandlerFactory frameHandlerFactory; private final Executor executor; @@ -102,14 +102,14 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl /** * Main entry point for {@link JettyWebSocketServletContainerInitializer}. * - * @param webSocketMapping the {@link WebSocketMapping} that this container belongs to + * @param webSocketMappings the {@link WebSocketMappings} that this container belongs to * @param components the {@link WebSocketComponents} instance to use * @param executor the {@link Executor} to use */ - JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMapping webSocketMapping, WebSocketComponents components, Executor executor) + JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMappings webSocketMappings, WebSocketComponents components, Executor executor) { this.contextHandler = contextHandler; - this.webSocketMapping = webSocketMapping; + this.webSocketMappings = webSocketMappings; this.executor = executor; this.components = components; @@ -129,12 +129,12 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl public void addMapping(String pathSpec, JettyWebSocketCreator creator) { - PathSpec ps = WebSocketMapping.parsePathSpec(pathSpec); - if (webSocketMapping.getMapping(ps) != null) + PathSpec ps = WebSocketMappings.parsePathSpec(pathSpec); + if (webSocketMappings.getMapping(ps) != null) throw new WebSocketException("Duplicate WebSocket Mapping for PathSpec"); WebSocketUpgradeFilter.ensureFilter(contextHandler.getServletContext()); - webSocketMapping.addMapping(ps, + webSocketMappings.addMapping(ps, (req, resp) -> creator.createWebSocket(new DelegatedServerUpgradeRequest(req), new DelegatedServerUpgradeResponse(resp)), frameHandlerFactory, customizer); } diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java index eb5f4016c5ec..7ce3a0e74783 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java @@ -40,14 +40,14 @@ import org.eclipse.jetty.websocket.util.server.internal.ServerUpgradeRequest; import org.eclipse.jetty.websocket.util.server.internal.ServerUpgradeResponse; import org.eclipse.jetty.websocket.util.server.internal.WebSocketCreator; -import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; +import org.eclipse.jetty.websocket.util.server.internal.WebSocketMappings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Abstract Servlet used to bridge the Servlet API to the WebSocket API. *

- * To use this servlet, you will be required to register your websockets with the {@link WebSocketMapping} so that it can create your websockets under the + * To use this servlet, you will be required to register your websockets with the {@link WebSocketMappings} so that it can create your websockets under the * appropriate conditions. *

*

The most basic implementation would be as follows:

@@ -68,11 +68,11 @@ * } * *

- * Only request that conforms to a "WebSocket: Upgrade" handshake request will trigger the {@link WebSocketMapping} handling of creating + * Only request that conforms to a "WebSocket: Upgrade" handshake request will trigger the {@link WebSocketMappings} handling of creating * WebSockets. All other requests are treated as normal servlet requests. The configuration defined by this servlet init parameters will * be used as the customizer for any mappings created by {@link JettyWebSocketServletFactory#addMapping(String, JettyWebSocketCreator)} during * {@link #configure(JettyWebSocketServletFactory)} calls. The request upgrade may be peformed by this servlet, or is may be performed by a - * {@link WebSocketUpgradeFilter} instance that will share the same {@link WebSocketMapping} instance. If the filter is used, then the + * {@link WebSocketUpgradeFilter} instance that will share the same {@link WebSocketMappings} instance. If the filter is used, then the * filter configuraton is used as the default configuration prior to this servlets configuration being applied. *

*

@@ -99,7 +99,7 @@ public abstract class JettyWebSocketServlet extends HttpServlet private static final Logger LOG = LoggerFactory.getLogger(JettyWebSocketServlet.class); private final CustomizedWebSocketServletFactory customizer = new CustomizedWebSocketServletFactory(); - private WebSocketMapping mapping; + private WebSocketMappings mapping; private WebSocketComponents components; /** @@ -133,7 +133,7 @@ public void init() throws ServletException ServletContext servletContext = getServletContext(); components = WebSocketServerComponents.getWebSocketComponents(servletContext); - mapping = new WebSocketMapping(components); + mapping = new WebSocketMappings(components); String max = getInitParameter("idleTimeout"); if (max == null) @@ -208,7 +208,7 @@ public Set getAvailableExtensionNames() @Override public void addMapping(String pathSpec, JettyWebSocketCreator creator) { - mapping.addMapping(WebSocketMapping.parsePathSpec(pathSpec), new WrappedJettyCreator(creator), getFactory(), this); + mapping.addMapping(WebSocketMappings.parsePathSpec(pathSpec), new WrappedJettyCreator(creator), getFactory(), this); } @Override @@ -249,7 +249,7 @@ public void setCreator(JettyWebSocketCreator creator) @Override public JettyWebSocketCreator getMapping(String pathSpec) { - WebSocketCreator creator = mapping.getMapping(WebSocketMapping.parsePathSpec(pathSpec)); + WebSocketCreator creator = mapping.getMapping(WebSocketMappings.parsePathSpec(pathSpec)); if (creator instanceof WrappedJettyCreator) return ((WrappedJettyCreator)creator).getJettyWebSocketCreator(); @@ -259,7 +259,7 @@ public JettyWebSocketCreator getMapping(String pathSpec) @Override public boolean removeMapping(String pathSpec) { - return mapping.removeMapping(WebSocketMapping.parsePathSpec(pathSpec)); + return mapping.removeMapping(WebSocketMappings.parsePathSpec(pathSpec)); } } 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 bbe0e33bda98..ad064fbbe5bf 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 @@ -27,7 +27,7 @@ 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.internal.WebSocketMapping; +import org.eclipse.jetty.websocket.util.server.internal.WebSocketMappings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,7 +48,7 @@ public interface Configurator * during the {@link ServletContext} initialization phase. * * @param context the context to add listener to. - * @param configurator a lambda that is called to allow the {@link WebSocketMapping} to + * @param configurator a lambda that is called to allow the {@link WebSocketMappings} to * be configured during {@link ServletContext} initialization phase */ public static void configure(ServletContextHandler context, Configurator configurator) diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java index 256528a2cfe2..dd45688a16d4 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java @@ -40,7 +40,7 @@ import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.Configuration; -import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; +import org.eclipse.jetty.websocket.util.server.internal.WebSocketMappings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,7 +49,7 @@ *

* The configuration applied to this filter via init params will be used as the the default * configuration of any websocket upgraded by this filter, prior to the configuration of the - * websocket applied by the {@link WebSocketMapping}. + * websocket applied by the {@link WebSocketMappings}. *

*

* Configuration / Init-Parameters: @@ -126,7 +126,7 @@ public static FilterHolder ensureFilter(ServletContext servletContext) } private final Configuration.ConfigurationCustomizer defaultCustomizer = new Configuration.ConfigurationCustomizer(); - private WebSocketMapping mapping; + private WebSocketMappings mapping; @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException @@ -162,7 +162,7 @@ public void dump(Appendable out, String indent) throws IOException @Override public void init(FilterConfig config) throws ServletException { - mapping = WebSocketMapping.ensureMapping(config.getServletContext()); + mapping = WebSocketMappings.ensureMapping(config.getServletContext()); String max = config.getInitParameter("idleTimeout"); if (max == null) diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMappings.java similarity index 93% rename from jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java rename to jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMappings.java index d7bb24b930ad..687818b1c96a 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMappings.java @@ -56,14 +56,14 @@ * wrap that POJO with a {@link FrameHandler} and the customizer is used to configure the resulting * {@link CoreSession}.

*/ -public class WebSocketMapping implements Dumpable, LifeCycle.Listener +public class WebSocketMappings implements Dumpable, LifeCycle.Listener { - private static final Logger LOG = LoggerFactory.getLogger(WebSocketMapping.class); - public static final String WEBSOCKET_MAPPING_ATTRIBUTE = WebSocketMapping.class.getName(); + private static final Logger LOG = LoggerFactory.getLogger(WebSocketMappings.class); + public static final String WEBSOCKET_MAPPING_ATTRIBUTE = WebSocketMappings.class.getName(); - public static WebSocketMapping getMapping(ServletContext servletContext) + public static WebSocketMappings getMapping(ServletContext servletContext) { - return (WebSocketMapping)servletContext.getAttribute(WEBSOCKET_MAPPING_ATTRIBUTE); + return (WebSocketMappings)servletContext.getAttribute(WEBSOCKET_MAPPING_ATTRIBUTE); } public WebSocketCreator getMapping(PathSpec pathSpec) @@ -72,12 +72,12 @@ public WebSocketCreator getMapping(PathSpec pathSpec) return cn == null ? null : cn.getWebSocketCreator(); } - public static WebSocketMapping ensureMapping(ServletContext servletContext) + public static WebSocketMappings ensureMapping(ServletContext servletContext) { - WebSocketMapping mapping = getMapping(servletContext); + WebSocketMappings mapping = getMapping(servletContext); if (mapping == null) { - mapping = new WebSocketMapping(WebSocketServerComponents.getWebSocketComponents(servletContext)); + mapping = new WebSocketMappings(WebSocketServerComponents.getWebSocketComponents(servletContext)); servletContext.setAttribute(WEBSOCKET_MAPPING_ATTRIBUTE, mapping); } @@ -127,12 +127,12 @@ else if (rawSpec.startsWith("uri-template|")) private final WebSocketComponents components; private final Handshaker handshaker = Handshaker.newInstance(); - public WebSocketMapping() + public WebSocketMappings() { this(new WebSocketComponents()); } - public WebSocketMapping(WebSocketComponents components) + public WebSocketMappings(WebSocketComponents components) { this.components = components; } @@ -141,7 +141,7 @@ public WebSocketMapping(WebSocketComponents components) public void lifeCycleStopping(LifeCycle context) { ContextHandler contextHandler = (ContextHandler)context; - WebSocketMapping mapping = contextHandler.getBean(WebSocketMapping.class); + WebSocketMappings mapping = contextHandler.getBean(WebSocketMappings.class); if (mapping == this) { contextHandler.removeBean(mapping); From 3b22c6bc66115bc013b7b696d10dbe0a3d22e20d Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 16 Nov 2020 15:57:28 +1100 Subject: [PATCH 4/7] fix broken test: AltFilterTest Signed-off-by: Lachlan Roberts --- .../jetty/websocket/javax/tests/server/AltFilterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/server/AltFilterTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/server/AltFilterTest.java index 31df6bdda450..831e991e7ce9 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/server/AltFilterTest.java +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/server/AltFilterTest.java @@ -61,7 +61,7 @@ public void testEcho() throws Exception { wsb.start(); - FilterHolder filterWebXml = app.getWebAppContext().getServletHandler().getFilter("wsuf-test"); + FilterHolder filterWebXml = app.getWebAppContext().getServletHandler().getFilter(WebSocketUpgradeFilter.class.getName()); assertThat("Filter[wsuf-test]", filterWebXml, notNullValue()); FilterHolder filterSCI = app.getWebAppContext().getServletHandler().getFilter("Jetty_WebSocketUpgradeFilter"); From f52e61156d3118b40b57294df906a27a4fd82b7e Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 17 Nov 2020 20:36:42 +1100 Subject: [PATCH 5/7] add test for a subclassed WebSocketUpgradeFilter Signed-off-by: Lachlan Roberts --- .../tests/JettyWebSocketFilterTest.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) 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 b269f72f4c4c..59b0530193a4 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 @@ -29,6 +29,8 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import javax.servlet.DispatcherType; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import org.eclipse.jetty.server.Server; @@ -266,6 +268,46 @@ public void testMultipleWebSocketUpgradeFilter() throws Exception assertThat(socket.textMessages.poll(), is(idleTimeoutFilter2)); } + @Test + public void testCustomUpgradeFilter() throws Exception + { + start((context, container) -> + { + ServletContextHandler contextHandler = Objects.requireNonNull(ServletContextHandler.getServletContextHandler(context)); + + // This custom filter replaces the default filter as we use the pre-defined name, and adds mapping in init(). + FilterHolder filterHolder = new FilterHolder(MyUpgradeFilter.class); + filterHolder.setName(WebSocketUpgradeFilter.class.getName()); + contextHandler.addFilter(filterHolder, "/*", EnumSet.of(DispatcherType.REQUEST)); + }); + + FilterHolder[] holders = contextHandler.getServletHandler().getFilters(); + assertThat(holders.length, is(1)); + assertThat(holders[0].getFilter(), instanceOf(MyUpgradeFilter.class)); + + // We can reach the echo endpoint and get correct response. + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/echo"); + EventSocket socket = new EventSocket(); + CompletableFuture connect = client.connect(socket, uri); + try (Session session = connect.get(5, TimeUnit.SECONDS)) + { + session.getRemote().sendString("hElLo wOrLd"); + } + assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(socket.textMessages.poll(), is("hElLo wOrLd")); + } + + public static class MyUpgradeFilter extends WebSocketUpgradeFilter + { + @Override + public void init(FilterConfig config) throws ServletException + { + JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(config.getServletContext()); + container.addMapping("/echo", EchoSocket.class); + super.init(config); + } + } + @WebSocket public static class LowerCaseEchoSocket { From aba2c93eae4ea3d5b22d686d764c04d26ed48a33 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 18 Nov 2020 15:10:14 +1100 Subject: [PATCH 6/7] Add tests for the ordering of the default WebSocketUpgradeFilter. Signed-off-by: Lachlan Roberts --- .../tests/JettyWebSocketFilterTest.java | 117 ++++++++++++++++-- .../websocket/tests/JettyWebSocketWebApp.java | 84 +++++++++++++ .../src/test/resources/wsuf-ordering1.xml | 21 ++++ .../src/test/resources/wsuf-ordering2.xml | 32 +++++ 4 files changed, 247 insertions(+), 7 deletions(-) create mode 100644 jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java create mode 100644 jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml create mode 100644 jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml 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 59b0530193a4..3d102e9ede05 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 @@ -20,6 +20,7 @@ import java.io.IOException; import java.net.URI; +import java.nio.file.Path; import java.util.Arrays; import java.util.EnumSet; import java.util.List; @@ -30,7 +31,10 @@ import java.util.stream.Collectors; import javax.servlet.DispatcherType; import javax.servlet.FilterConfig; +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; import javax.servlet.ServletException; +import javax.servlet.annotation.WebListener; import javax.servlet.http.HttpServlet; import org.eclipse.jetty.server.Server; @@ -38,15 +42,18 @@ import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage; import org.eclipse.jetty.websocket.api.annotations.WebSocket; import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.core.WebSocketConstants; import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; import org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -64,6 +71,16 @@ public class JettyWebSocketFilterTest private WebSocketClient client; private ServletContextHandler contextHandler; + @BeforeEach + public void before() + { + server = new Server(); + connector = new ServerConnector(server); + server.addConnector(connector); + + client = new WebSocketClient(); + } + public void start(JettyWebSocketServletContainerInitializer.Configurator configurator) throws Exception { start(configurator, null); @@ -76,20 +93,14 @@ public void start(ServletHolder servletHolder) throws Exception public void start(JettyWebSocketServletContainerInitializer.Configurator configurator, ServletHolder servletHolder) throws Exception { - server = new Server(); - connector = new ServerConnector(server); - server.addConnector(connector); - contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS); contextHandler.setContextPath("/"); if (servletHolder != null) contextHandler.addServlet(servletHolder, "/"); server.setHandler(contextHandler); - JettyWebSocketServletContainerInitializer.configure(contextHandler, configurator); - server.start(); - client = new WebSocketClient(); + server.start(); client.start(); } @@ -297,6 +308,98 @@ public void testCustomUpgradeFilter() throws Exception assertThat(socket.textMessages.poll(), is("hElLo wOrLd")); } + @Test + public void testDefaultWebSocketUpgradeFilterOrdering() throws Exception + { + String timeoutFromAltFilter = "5999"; + JettyWebSocketWebApp webApp = new JettyWebSocketWebApp("wsuf-ordering1"); + Path webXml = MavenTestingUtils.getTestResourcePath("wsuf-ordering1.xml"); + webApp.copyWebXml(webXml); + webApp.copyClass(WebSocketEchoServletContextListener.class); + webApp.copyClass(WebSocketEchoServletContextListener.EchoSocket.class); + + server.setHandler(webApp); + server.start(); + client.start(); + + // We have both websocket upgrade filters installed. + FilterHolder[] filterHolders = webApp.getServletHandler().getFilters(); + assertThat(filterHolders.length, is(2)); + assertThat(filterHolders[0].getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + assertThat(filterHolders[1].getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + + // The custom filter defined in web.xml should be first in line so it will do the upgrade. + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + webApp.getContextPath() + "/echo"); + EventSocket socket = new EventSocket(); + CompletableFuture connect = client.connect(socket, uri); + try (Session session = connect.get(5, TimeUnit.SECONDS)) + { + session.getRemote().sendString("hello world"); + session.getRemote().sendString("getIdleTimeout"); + } + assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(socket.textMessages.poll(), is("hello world")); + assertThat(socket.textMessages.poll(), is(timeoutFromAltFilter)); + } + + @Test + public void testWebSocketUpgradeFilterOrdering() throws Exception + { + String defaultIdleTimeout = Long.toString(WebSocketConstants.DEFAULT_IDLE_TIMEOUT.toMillis()); + JettyWebSocketWebApp webApp = new JettyWebSocketWebApp("wsuf-ordering2"); + Path webXml = MavenTestingUtils.getTestResourcePath("wsuf-ordering2.xml"); + webApp.copyWebXml(webXml); + webApp.copyClass(WebSocketEchoServletContextListener.class); + webApp.copyClass(WebSocketEchoServletContextListener.EchoSocket.class); + + server.setHandler(webApp); + server.start(); + client.start(); + + // We have both websocket upgrade filters installed. + FilterHolder[] filterHolders = webApp.getServletHandler().getFilters(); + assertThat(filterHolders.length, is(2)); + assertThat(filterHolders[0].getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + assertThat(filterHolders[1].getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + + // The custom filter defined in web.xml should be first in line so it will do the upgrade. + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + webApp.getContextPath() + "/echo"); + EventSocket socket = new EventSocket(); + CompletableFuture connect = client.connect(socket, uri); + try (Session session = connect.get(5, TimeUnit.SECONDS)) + { + session.getRemote().sendString("hello world"); + session.getRemote().sendString("getIdleTimeout"); + } + assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(socket.textMessages.poll(), is("hello world")); + assertThat(socket.textMessages.poll(), is(defaultIdleTimeout)); + } + + @WebListener + public static class WebSocketEchoServletContextListener implements ServletContextListener + { + @Override + public void contextInitialized(ServletContextEvent sce) + { + JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(sce.getServletContext()); + container.addMapping("/echo", EchoSocket.class); + } + + @WebSocket + public static class EchoSocket + { + @OnWebSocketMessage + public void onMessage(Session session, String message) throws IOException + { + if ("getIdleTimeout".equals(message)) + session.getRemote().sendString(Long.toString(session.getIdleTimeout().toMillis())); + else + session.getRemote().sendString(message); + } + } + } + public static class MyUpgradeFilter extends WebSocketUpgradeFilter { @Override diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java new file mode 100644 index 000000000000..c8845602201b --- /dev/null +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java @@ -0,0 +1,84 @@ +package org.eclipse.jetty.websocket.tests; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.net.URL; +import java.nio.file.Path; + +import org.eclipse.jetty.toolchain.test.FS; +import org.eclipse.jetty.toolchain.test.IO; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.resource.PathResource; +import org.eclipse.jetty.webapp.WebAppContext; +import org.eclipse.jetty.websocket.server.config.JettyWebSocketConfiguration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.notNullValue; + +public class JettyWebSocketWebApp extends WebAppContext +{ + private static final Logger LOG = LoggerFactory.getLogger(JettyWebSocketWebApp.class); + + private final Path contextDir; + private final Path webInf; + private final Path classesDir; + + public JettyWebSocketWebApp(String contextName) + { + // Ensure context directory. + Path testDir = MavenTestingUtils.getTargetTestingPath(JettyWebSocketWebApp.class.getName()); + contextDir = testDir.resolve(contextName); + FS.ensureEmpty(contextDir); + + // Ensure WEB-INF directories. + webInf = contextDir.resolve("WEB-INF"); + FS.ensureDirExists(webInf); + classesDir = webInf.resolve("classes"); + FS.ensureDirExists(classesDir); + + // Configure the WebAppContext. + setContextPath("/" + contextName); + setBaseResource(new PathResource(contextDir)); + addConfiguration(new JettyWebSocketConfiguration()); + } + + public Path getContextDir() + { + return contextDir; + } + + public void createWebXml() throws IOException + { + String emptyWebXml = "" + + ""; + + Path webXml = webInf.resolve("web.xml"); + try (FileWriter writer = new FileWriter(webXml.toFile())) + { + writer.write(emptyWebXml); + } + } + + public void copyWebXml(Path webXml) throws IOException + { + IO.copy(webXml.toFile(), webInf.resolve("web.xml").toFile()); + } + + public void copyClass(Class clazz) throws Exception + { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + String endpointPath = TypeUtil.toClassReference(clazz); + URL classUrl = cl.getResource(endpointPath); + assertThat("Class URL for: " + clazz, classUrl, notNullValue()); + Path destFile = classesDir.resolve(endpointPath); + FS.ensureDirExists(destFile.getParent()); + File srcFile = new File(classUrl.toURI()); + IO.copy(srcFile, destFile.toFile()); + } +} diff --git a/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml new file mode 100644 index 000000000000..d5ef3f972e70 --- /dev/null +++ b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml @@ -0,0 +1,21 @@ + + + + + wsuf-alt + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + + idleTimeout + 5999 + + + + wsuf-alt + /* + + diff --git a/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml new file mode 100644 index 000000000000..ca19ae86c7da --- /dev/null +++ b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml @@ -0,0 +1,32 @@ + + + + + + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + + + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + /* + + + + + wsuf-alt + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + + idleTimeout + 5999 + + + + wsuf-alt + /* + + From 6a83a261e10d7522b9689b63e3eed8346442d579 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 18 Nov 2020 21:27:25 +1100 Subject: [PATCH 7/7] Always add the default WebSocketUpgradeFilter as the first filter. Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/servlet/ServletHandler.java | 17 ++++++++++++ .../tests/JettyWebSocketFilterTest.java | 8 +++--- .../websocket/tests/JettyWebSocketWebApp.java | 18 +++++++++++++ .../src/test/resources/wsuf-ordering1.xml | 8 +++--- .../src/test/resources/wsuf-ordering2.xml | 26 +++++++++---------- .../util/server/WebSocketUpgradeFilter.java | 22 +++++++++++----- 6 files changed, 72 insertions(+), 27 deletions(-) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 04e1ffc7d1e1..d1e635d4da9d 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -1084,6 +1084,23 @@ public void addFilter(FilterHolder filter) } } + /** + * Convenience method to add a preconstructed FilterHolder + * + * @param filter the filter holder + */ + public void prependFilter(FilterHolder filter) + { + if (filter == null) + return; + + try (AutoLock l = lock()) + { + if (!containsFilterHolder(filter)) + setFilters(ArrayUtil.prependToArray(filter, getFilters(), FilterHolder.class)); + } + } + /** * Convenience method to add a preconstructed FilterMapping * 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 3d102e9ede05..a11772e67e17 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 @@ -311,7 +311,7 @@ public void testCustomUpgradeFilter() throws Exception @Test public void testDefaultWebSocketUpgradeFilterOrdering() throws Exception { - String timeoutFromAltFilter = "5999"; + String defaultIdleTimeout = Long.toString(WebSocketConstants.DEFAULT_IDLE_TIMEOUT.toMillis()); JettyWebSocketWebApp webApp = new JettyWebSocketWebApp("wsuf-ordering1"); Path webXml = MavenTestingUtils.getTestResourcePath("wsuf-ordering1.xml"); webApp.copyWebXml(webXml); @@ -339,13 +339,13 @@ public void testDefaultWebSocketUpgradeFilterOrdering() throws Exception } assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); assertThat(socket.textMessages.poll(), is("hello world")); - assertThat(socket.textMessages.poll(), is(timeoutFromAltFilter)); + assertThat(socket.textMessages.poll(), is(defaultIdleTimeout)); } @Test public void testWebSocketUpgradeFilterOrdering() throws Exception { - String defaultIdleTimeout = Long.toString(WebSocketConstants.DEFAULT_IDLE_TIMEOUT.toMillis()); + String timeoutFromAltFilter = "5999"; JettyWebSocketWebApp webApp = new JettyWebSocketWebApp("wsuf-ordering2"); Path webXml = MavenTestingUtils.getTestResourcePath("wsuf-ordering2.xml"); webApp.copyWebXml(webXml); @@ -373,7 +373,7 @@ public void testWebSocketUpgradeFilterOrdering() throws Exception } assertTrue(socket.closeLatch.await(5, TimeUnit.SECONDS)); assertThat(socket.textMessages.poll(), is("hello world")); - assertThat(socket.textMessages.poll(), is(defaultIdleTimeout)); + assertThat(socket.textMessages.poll(), is(timeoutFromAltFilter)); } @WebListener diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java index c8845602201b..cbfc6cd119ff 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketWebApp.java @@ -1,3 +1,21 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + package org.eclipse.jetty.websocket.tests; import java.io.File; diff --git a/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml index d5ef3f972e70..b93d673a43b2 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml +++ b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering1.xml @@ -2,11 +2,11 @@ + xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_4_0.xsd" + metadata-complete="false" + version="4.0"> - + wsuf-alt org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter diff --git a/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml index ca19ae86c7da..a733873a6712 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml +++ b/jetty-websocket/websocket-jetty-tests/src/test/resources/wsuf-ordering2.xml @@ -2,31 +2,31 @@ + xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_4_0.xsd" + metadata-complete="false" + version="4.0"> - + - org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + wsuf-alt org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + + idleTimeout + 5999 + - org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter + wsuf-alt /* - + - wsuf-alt + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter - - idleTimeout - 5999 - - wsuf-alt + org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter /* diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java index dd45688a16d4..546c082171a0 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java @@ -35,6 +35,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.FilterMapping; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.Dumpable; @@ -78,12 +79,12 @@ public class WebSocketUpgradeFilter implements Filter, Dumpable private static final AutoLock LOCK = new AutoLock(); /** - * Return any {@link WebSocketUpgradeFilter} already present on the {@link ServletContext}. + * Return the default {@link WebSocketUpgradeFilter} if present on the {@link ServletContext}. * * @param servletContext the {@link ServletContext} to use. * @return the configured default {@link WebSocketUpgradeFilter} instance. */ - private static FilterHolder getFilter(ServletContext servletContext) + public static FilterHolder getFilter(ServletContext servletContext) { ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext)); ServletHandler servletHandler = contextHandler.getChildHandlerByClass(ServletHandler.class); @@ -111,14 +112,23 @@ public static FilterHolder ensureFilter(ServletContext servletContext) if (existingFilter != null) return existingFilter; + ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext)); + ServletHandler servletHandler = contextHandler.getChildHandlerByClass(ServletHandler.class); + final String pathSpec = "/*"; FilterHolder holder = new FilterHolder(new WebSocketUpgradeFilter()); holder.setName(WebSocketUpgradeFilter.class.getName()); - holder.setAsyncSupported(true); - ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext)); - ServletHandler servletHandler = contextHandler.getChildHandlerByClass(ServletHandler.class); - servletHandler.addFilterWithMapping(holder, pathSpec, EnumSet.of(DispatcherType.REQUEST)); + + FilterMapping mapping = new FilterMapping(); + mapping.setFilterName(holder.getName()); + mapping.setPathSpec(pathSpec); + mapping.setDispatcherTypes(EnumSet.of(DispatcherType.REQUEST)); + + // Add the default WebSocketUpgradeFilter as the first filter in the list. + servletHandler.prependFilter(holder); + servletHandler.prependFilterMapping(mapping); + if (LOG.isDebugEnabled()) LOG.debug("Adding {} mapped to {} in {}", holder, pathSpec, servletContext); return holder;