From 0493a111067fc57a1e55da2e6eb8608c34593d9f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 13 Nov 2020 15:44:18 +1100 Subject: [PATCH] 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 4325b3240c8..2673ea16fcc 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 @@ public class JavaxWebSocketServletContainerInitializer implements ServletContain { 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 f9218c819d9..4799fcee6a9 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 class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer // 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 1729cd545f0..4837b7dfd5b 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 class JettyWebSocketServerContainer extends ContainerLifeCycle implements // 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 c0be53db263..477098c266b 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 class JettyWebSocketFilterTest // 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 class JettyWebSocketFilterTest // 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 class JettyWebSocketFilterTest // 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 176544be380..256528a2cfe 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 @@ public class WebSocketUpgradeFilter implements Filter, Dumpable { 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 class WebSocketUpgradeFilter implements Filter, Dumpable 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 class WebSocketUpgradeFilter implements Filter, Dumpable @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 e93c0c4c019..d7bb24b930a 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 @@ import org.slf4j.LoggerFactory; 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 class WebSocketMapping implements Dumpable, LifeCycle.Listener 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 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener 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();