From e9df97e314a972666fbfc2125552db37f8521f6e Mon Sep 17 00:00:00 2001 From: lachan-roberts Date: Mon, 11 Mar 2019 01:53:44 +1100 Subject: [PATCH] Issue #3446 - allow jetty-websocket upgrades with WebSocketUpgradeFilter JettyWebSocketServerContainer moved out of internal and is now used in a similar way to the Javax server container, and can now be used to configure upgrades using the shared mapping with the WSUpgradeFilter the JettyWSSCI now works in the same way as the JavaxWSSCI, the JettyWebSocketEmbeddedStarter has been removed and the configureContext now returns the JettyWebSocketServerContainer WebSocket upgrade failures due to exceptions will now be logged as a warning in WebSocketMapping rather than being ignored Signed-off-by: lachan-roberts --- .../JettyServerFrameHandlerFactory.java | 6 +- .../server/JettyWebSocketServerContainer.java | 86 +++++++-- ...yWebSocketServletContainerInitializer.java | 49 ++--- .../websocket/servlet/WebSocketMapping.java | 179 +++++++++--------- .../websocket/servlet/WebSocketServlet.java | 3 +- 5 files changed, 173 insertions(+), 150 deletions(-) diff --git a/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyServerFrameHandlerFactory.java b/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyServerFrameHandlerFactory.java index 8422f8e3d80..d9f6fa4916e 100644 --- a/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyServerFrameHandlerFactory.java +++ b/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyServerFrameHandlerFactory.java @@ -19,8 +19,8 @@ package org.eclipse.jetty.websocket.server; import java.util.concurrent.CompletableFuture; + import javax.servlet.ServletContext; -import javax.servlet.ServletException; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.servlet.ServletContextHandler; @@ -29,7 +29,6 @@ import org.eclipse.jetty.websocket.common.JettyWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.common.WebSocketContainer; import org.eclipse.jetty.websocket.core.FrameHandler; import org.eclipse.jetty.websocket.server.internal.DelegatedJettyServletUpgradeRequest; -import org.eclipse.jetty.websocket.server.internal.JettyWebSocketServerContainer; import org.eclipse.jetty.websocket.server.internal.UpgradeResponseAdapter; import org.eclipse.jetty.websocket.servlet.FrameHandlerFactory; import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest; @@ -40,14 +39,13 @@ public class JettyServerFrameHandlerFactory implements FrameHandlerFactory, LifeCycle.Listener { public static JettyServerFrameHandlerFactory ensureFactory(ServletContext servletContext) - throws ServletException { ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Jetty Websocket"); JettyServerFrameHandlerFactory factory = contextHandler.getBean(JettyServerFrameHandlerFactory.class); if (factory == null) { - JettyWebSocketServerContainer container = new JettyWebSocketServerContainer(contextHandler); + JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(servletContext); servletContext.setAttribute(WebSocketContainer.class.getName(), container); factory = new JettyServerFrameHandlerFactory(container); contextHandler.addManaged(factory); diff --git a/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 4b28f6d1ca4..86246819701 100644 --- a/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -16,7 +16,7 @@ // ======================================================================== // -package org.eclipse.jetty.websocket.server.internal; +package org.eclipse.jetty.websocket.server; import java.util.ArrayList; import java.util.Collection; @@ -24,37 +24,93 @@ import java.util.List; import java.util.concurrent.Executor; import java.util.function.Consumer; +import javax.servlet.ServletContext; + +import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.common.SessionTracker; import org.eclipse.jetty.websocket.common.WebSocketContainer; import org.eclipse.jetty.websocket.common.WebSocketSessionListener; +import org.eclipse.jetty.websocket.core.FrameHandler; +import org.eclipse.jetty.websocket.core.WebSocketComponents; +import org.eclipse.jetty.websocket.core.WebSocketException; +import org.eclipse.jetty.websocket.servlet.FrameHandlerFactory; +import org.eclipse.jetty.websocket.servlet.WebSocketCreator; +import org.eclipse.jetty.websocket.servlet.WebSocketMapping; -public class JettyWebSocketServerContainer implements WebSocketContainer +public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, LifeCycle.Listener { + + public static JettyWebSocketServerContainer ensureContainer(ServletContext servletContext) + { + ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Javax Websocket"); + if (contextHandler.getServer() == null) + throw new IllegalStateException("Server has not been set on the ServletContextHandler"); + + JettyWebSocketServerContainer container = contextHandler.getBean(JettyWebSocketServerContainer.class); + if (container==null) + { + // Find Pre-Existing executor + Executor executor = (Executor)servletContext.getAttribute("org.eclipse.jetty.server.Executor"); + if (executor == null) + executor = contextHandler.getServer().getThreadPool(); + + // Create the Jetty ServerContainer implementation + container = new JettyWebSocketServerContainer( + WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY), + WebSocketComponents.ensureWebSocketComponents(servletContext), executor); + contextHandler.addManaged(container); + contextHandler.addLifeCycleListener(container); + } + + return container; + } + private final static Logger LOG = Log.getLogger(JettyWebSocketServerContainer.class); + + private final WebSocketMapping webSocketMapping; + private final WebSocketComponents webSocketComponents; + private final FrameHandlerFactory frameHandlerFactory; private final Executor executor; + private final FrameHandler.ConfigurationCustomizer customizer = new FrameHandler.ConfigurationCustomizer(); + private final List sessionListeners = new ArrayList<>(); private final SessionTracker sessionTracker = new SessionTracker(); - public JettyWebSocketServerContainer(ContextHandler handler) + /** + * Main entry point for {@link JettyWebSocketServletContainerInitializer}. + * @param webSocketMapping the {@link WebSocketMapping} that this container belongs to + * @param webSocketComponents the {@link WebSocketComponents} instance to use + * @param executor the {@link Executor} to use + */ + public JettyWebSocketServerContainer(WebSocketMapping webSocketMapping, WebSocketComponents webSocketComponents, Executor executor) { - Executor executor = (Executor) handler - .getAttribute("org.eclipse.jetty.server.Executor"); - if (executor == null) - { - executor = handler.getServer().getThreadPool(); - } - if (executor == null) - { - executor = new QueuedThreadPool(); // default settings - } + this.webSocketMapping = webSocketMapping; + this.webSocketComponents = webSocketComponents; this.executor = executor; + this.frameHandlerFactory = new JettyServerFrameHandlerFactory(this); + addSessionListener(sessionTracker); - handler.addBean(sessionTracker); + } + + + public void addMapping(String pathSpec, WebSocketCreator creator) + { + addMapping(WebSocketMapping.parsePathSpec(pathSpec), creator); + } + + public void addMapping(PathSpec pathSpec, WebSocketCreator creator) throws WebSocketException + { + if (webSocketMapping.getMapping(pathSpec) != null) + throw new WebSocketException("Duplicate WebSocket Mapping for PathSpec"); + + webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, customizer); } @Override diff --git a/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServletContainerInitializer.java b/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServletContainerInitializer.java index 4065f5d0783..b68747a060a 100644 --- a/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServletContainerInitializer.java +++ b/jetty-websocket/jetty-websocket-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServletContainerInitializer.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.websocket.server; -import java.util.Collections; import java.util.Set; import javax.servlet.ServletContainerInitializer; @@ -27,7 +26,6 @@ import javax.servlet.ServletException; import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; -import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.websocket.core.WebSocketComponents; @@ -41,47 +39,24 @@ public class JettyWebSocketServletContainerInitializer implements ServletContain { private static final Logger LOG = Log.getLogger(JettyWebSocketServletContainerInitializer.class); - public static class JettyWebSocketEmbeddedStarter extends AbstractLifeCycle implements ServletContextHandler.ServletContainerInitializerCaller + public static JettyWebSocketServerContainer configureContext(ServletContextHandler context) { - private ServletContainerInitializer sci; - private ServletContextHandler context; + WebSocketComponents components = WebSocketComponents.ensureWebSocketComponents(context.getServletContext()); + FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(context.getServletContext()); + WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY); + JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(context.getServletContext()); + JettyServerFrameHandlerFactory.ensureFactory(context.getServletContext()); - public JettyWebSocketEmbeddedStarter(ServletContainerInitializer sci, ServletContextHandler context) - { - this.sci = sci; - this.context = context; - } + if (LOG.isDebugEnabled()) + LOG.debug("onStartup {} {} {} {}", container, mapping, filterHolder, components); - public void doStart() - { - try - { - Set> c = Collections.emptySet(); - sci.onStartup(c, context.getServletContext()); - } - catch (Exception e) - { - throw new RuntimeException(e); - } - } - } - - public static void configureContext(ServletContextHandler context) - { - JettyWebSocketServletContainerInitializer sci = new JettyWebSocketServletContainerInitializer(); - JettyWebSocketEmbeddedStarter starter = new JettyWebSocketEmbeddedStarter(sci, context); - context.addBean(starter); + return container; } @Override - public void onStartup(Set> c, ServletContext servletContext) throws ServletException + public void onStartup(Set> c, ServletContext context) throws ServletException { - WebSocketComponents components = WebSocketComponents.ensureWebSocketComponents(servletContext); - FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(servletContext); - WebSocketMapping mapping = WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY); - JettyServerFrameHandlerFactory factory = JettyServerFrameHandlerFactory.ensureFactory(servletContext); - - if (LOG.isDebugEnabled()) - LOG.debug("onStartup {} {} {} {}", factory, mapping, filterHolder, components); + ServletContextHandler contextHandler = ServletContextHandler.getServletContextHandler(context,"Jetty WebSocket SCI"); + JettyWebSocketServletContainerInitializer.configureContext(contextHandler); } } diff --git a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java index 6a7c9f8c728..2476a929ac5 100644 --- a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java +++ b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketMapping.java @@ -93,96 +93,6 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener return mapping; } - public static final String DEFAULT_KEY = "org.eclipse.jetty.websocket.servlet.WebSocketMapping"; - - private final PathMappings mappings = new PathMappings<>(); - private final WebSocketComponents components; - private final Handshaker handshaker = Handshaker.newInstance(); - - public WebSocketMapping() - { - this(new WebSocketComponents()); - } - - public WebSocketMapping(WebSocketComponents components) - { - this.components = components; - } - - @Override - public void lifeCycleStopping(LifeCycle context) - { - ContextHandler contextHandler = (ContextHandler) context; - WebSocketMapping mapping = contextHandler.getBean(WebSocketMapping.class); - if (mapping == this) - { - contextHandler.removeBean(mapping); - mappings.reset(); - } - } - - /** - * Manually add a WebSocket mapping. - *

- * If mapping is added before this configuration is started, then it is persisted through - * stop/start of this configuration's lifecycle. Otherwise it will be removed when - * this configuration is stopped. - *

- * - * @param pathSpec the pathspec to respond on - * @param creator the websocket creator to activate on the provided mapping. - * @param factory the factory to use to create a FrameHandler for the websocket - * @param customizer the customizer to use to customize the WebSocket session. - */ - public void addMapping(PathSpec pathSpec, WebSocketCreator creator, FrameHandlerFactory factory, FrameHandler.Customizer customizer) - throws WebSocketException - { - // TODO evaluate why this can't be done - //if (getMapping(pathSpec) != null) - // throw new WebSocketException("Duplicate WebSocket Mapping for PathSpec"); - - mappings.put(pathSpec, new Negotiator(creator, factory, customizer)); - } - - public WebSocketCreator getMapping(PathSpec pathSpec) - { - Negotiator cn = mappings.get(pathSpec); - return cn == null?null:cn.getWebSocketCreator(); - } - - public boolean removeMapping(PathSpec pathSpec) - { - return mappings.remove(pathSpec); - } - - @Override - public String dump() - { - return Dumpable.dump(this); - } - - @Override - public void dump(Appendable out, String indent) throws IOException - { - Dumpable.dumpObjects(out, indent, this, mappings); - } - - /** - * Get the matching {@link MappedResource} for the provided target. - * - * @param target the target path - * @return the matching resource, or null if no match. - */ - public WebSocketNegotiator getMatchedNegotiator(String target, Consumer pathSpecConsumer) - { - MappedResource mapping = this.mappings.getMatch(target); - if (mapping == null) - return null; - - pathSpecConsumer.accept(mapping.getPathSpec()); - return mapping.getResource(); - } - /** * Parse a PathSpec string into a PathSpec instance. *

@@ -222,6 +132,91 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener throw new IllegalArgumentException("Unrecognized path spec syntax [" + rawSpec + "]"); } + public static final String DEFAULT_KEY = "org.eclipse.jetty.websocket.servlet.WebSocketMapping"; + + private final PathMappings mappings = new PathMappings<>(); + private final WebSocketComponents components; + private final Handshaker handshaker = Handshaker.newInstance(); + + public WebSocketMapping() + { + this(new WebSocketComponents()); + } + + public WebSocketMapping(WebSocketComponents components) + { + this.components = components; + } + + @Override + public void lifeCycleStopping(LifeCycle context) + { + ContextHandler contextHandler = (ContextHandler) context; + WebSocketMapping mapping = contextHandler.getBean(WebSocketMapping.class); + if (mapping == this) + { + contextHandler.removeBean(mapping); + mappings.reset(); + } + } + + @Override + public String dump() + { + return Dumpable.dump(this); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + Dumpable.dumpObjects(out, indent, this, mappings); + } + + /** + * Manually add a WebSocket mapping. + *

+ * If mapping is added before this configuration is started, then it is persisted through + * stop/start of this configuration's lifecycle. Otherwise it will be removed when + * this configuration is stopped. + *

+ * + * @param pathSpec the pathspec to respond on + * @param creator the websocket creator to activate on the provided mapping. + * @param factory the factory to use to create a FrameHandler for the websocket + * @param customizer the customizer to use to customize the WebSocket session. + */ + public void addMapping(PathSpec pathSpec, WebSocketCreator creator, FrameHandlerFactory factory, FrameHandler.Customizer customizer) throws WebSocketException + { + mappings.put(pathSpec, new Negotiator(creator, factory, customizer)); + } + + public WebSocketCreator getMapping(PathSpec pathSpec) + { + Negotiator cn = mappings.get(pathSpec); + return cn == null?null:cn.getWebSocketCreator(); + } + + public boolean removeMapping(PathSpec pathSpec) + { + return mappings.remove(pathSpec); + } + + /** + * Get the matching {@link MappedResource} for the provided target. + * + * @param target the target path + * @return the matching resource, or null if no match. + */ + public WebSocketNegotiator getMatchedNegotiator(String target, Consumer pathSpecConsumer) + { + MappedResource mapping = this.mappings.getMatch(target); + if (mapping == null) + return null; + + pathSpecConsumer.accept(mapping.getPathSpec()); + return mapping.getResource(); + } + public boolean upgrade(HttpServletRequest request, HttpServletResponse response, FrameHandler.Customizer defaultCustomizer) { try @@ -251,9 +246,7 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener } catch (Exception e) { - if (LOG.isDebugEnabled()) - LOG.debug("Unable to upgrade: "+e); - LOG.ignore(e); + LOG.warn("Error during upgrade: ", e); } return false; } diff --git a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServlet.java b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServlet.java index ddd8f85b6e9..3e63669737a 100644 --- a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServlet.java +++ b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServlet.java @@ -189,9 +189,10 @@ public abstract class WebSocketServlet extends HttpServlet @Override public void addMapping(PathSpec pathSpec, WebSocketCreator creator) { - // TODO a bit fragile. This code knows that only the JettyFHF is added directly as a been ServletContext servletContext = getServletContext(); ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "WebSocketServlet"); + + // TODO: a bit fragile, this code knows that only the JettyFHF is added as a bean FrameHandlerFactory frameHandlerFactory = contextHandler.getBean(FrameHandlerFactory.class); if (frameHandlerFactory==null)