From 05f397caa43ea11d5bd05ff8495ac538a16fa1df Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 14 Dec 2016 13:20:28 -0700 Subject: [PATCH 1/3] Issue #1114 - Adding initial stop/start test logic --- .../server/WebSocketUpgradeFilterTest.java | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java index fabffcc3405..b5474308cb4 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java @@ -291,7 +291,7 @@ public class WebSocketUpgradeFilterTest } @Test - public void testConfiguration() throws Exception + public void testNormalConfiguration() throws Exception { URI destUri = serverUri.resolve("/info/"); @@ -311,4 +311,45 @@ public class WebSocketUpgradeFilterTest assertThat("payload", payload, containsString("session.maxTextMessageSize=" + (10 * 1024 * 1024))); } } + + @Test + public void testStopStartOfHandler() throws Exception + { + URI destUri = serverUri.resolve("/info/"); + + try (BlockheadClient client = new BlockheadClient(destUri)) + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + client.write(new TextFrame().setPayload("hello 1")); + + EventQueue frames = client.readFrames(1, 1000, TimeUnit.MILLISECONDS); + String payload = frames.poll().getPayloadAsUTF8(); + + // If we can connect and send a text message, we know that the endpoint was + // added properly, and the response will help us verify the policy configuration too + assertThat("payload", payload, containsString("session.maxTextMessageSize=" + (10 * 1024 * 1024))); + } + + server.getHandler().stop(); + server.getHandler().start(); + + try (BlockheadClient client = new BlockheadClient(destUri)) + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + client.write(new TextFrame().setPayload("hello 2")); + + EventQueue frames = client.readFrames(1, 1000, TimeUnit.MILLISECONDS); + String payload = frames.poll().getPayloadAsUTF8(); + + // If we can connect and send a text message, we know that the endpoint was + // added properly, and the response will help us verify the policy configuration too + assertThat("payload", payload, containsString("session.maxTextMessageSize=" + (10 * 1024 * 1024))); + } + } } From bd104d59f9a7e46e0e4042132887477255ef5ff8 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 14 Dec 2016 15:31:23 -0700 Subject: [PATCH 2/3] Issue #1114 - persisting NativeWebSocketConfiguration mappings + Mappings are persisted if added to the NativeWebSocketConfiguration before that configuration is started. Otherwise they are cleared out on configuration.stop() lifecycle (like before) --- .../jetty/http/pathmap/PathMappings.java | 6 ++++ .../server/NativeWebSocketConfiguration.java | 36 +++++++++++++++++-- .../server/WebSocketUpgradeFilter.java | 4 ++- .../server/WebSocketUpgradeFilterTest.java | 35 ++++++++++++++++-- 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java index d1c36cfbd69..e618213e758 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.function.Predicate; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; @@ -69,6 +70,11 @@ public class PathMappings implements Iterable>, Dumpable mappings.clear(); } + public void removeIf(Predicate> predicate) + { + mappings.removeIf(predicate); + } + /** * Return a list of MappedResource matches for the specified path. * diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java index aac64ec26ba..beea529e118 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/NativeWebSocketConfiguration.java @@ -60,7 +60,7 @@ public class NativeWebSocketConfiguration extends ContainerLifeCycle implements @Override public void doStop() throws Exception { - mappings.reset(); + mappings.removeIf((mapped) -> !(mapped.getResource() instanceof PersistedWebSocketCreator)); super.doStop(); } @@ -111,13 +111,23 @@ public class NativeWebSocketConfiguration extends ContainerLifeCycle implements /** * 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. */ public void addMapping(PathSpec pathSpec, WebSocketCreator creator) { - mappings.put(pathSpec, creator); + WebSocketCreator wsCreator = creator; + if (!isRunning()) + { + wsCreator = new PersistedWebSocketCreator(creator); + } + mappings.put(pathSpec, wsCreator); } /** @@ -170,4 +180,26 @@ public class NativeWebSocketConfiguration extends ContainerLifeCycle implements } }); } + + private class PersistedWebSocketCreator implements WebSocketCreator + { + private final WebSocketCreator delegate; + + public PersistedWebSocketCreator(WebSocketCreator delegate) + { + this.delegate = delegate; + } + + @Override + public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse resp) + { + return delegate.createWebSocket(req, resp); + } + + @Override + public String toString() + { + return "Persisted[" + super.toString() + "]"; + } + } } diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java index 85f97b3b52e..42044bc2c0d 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java @@ -110,6 +110,7 @@ public class WebSocketUpgradeFilter implements Filter, MappedWebSocketCreator, D } private NativeWebSocketConfiguration configuration; + private boolean exportConfiguration = true; private boolean localConfiguration = false; private boolean alreadySetToAttribute = false; @@ -126,6 +127,7 @@ public class WebSocketUpgradeFilter implements Filter, MappedWebSocketCreator, D public WebSocketUpgradeFilter(NativeWebSocketConfiguration configuration) { this.configuration = configuration; + this.exportConfiguration = false; } @Override @@ -304,7 +306,7 @@ public class WebSocketUpgradeFilter implements Filter, MappedWebSocketCreator, D NativeWebSocketConfiguration.class.getName() + " at ServletContext attribute '" + configurationKey + "'"); } } - else + else if (exportConfiguration) { // We have a NativeWebSocketConfiguration already present, make sure it exists on the ServletContext if (config.getServletContext().getAttribute(configurationKey) == null) diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java index b5474308cb4..131b289a9b6 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilterTest.java @@ -34,6 +34,7 @@ import javax.servlet.DispatcherType; import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.toolchain.test.EventQueue; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; @@ -166,14 +167,44 @@ public class WebSocketUpgradeFilterTest NativeWebSocketConfiguration configuration = new NativeWebSocketConfiguration(context.getServletContext()); configuration.getFactory().getPolicy().setMaxTextMessageSize(10 * 1024 * 1024); configuration.addMapping(new ServletPathSpec("/info/*"), infoCreator); - context.getServletContext().setAttribute(NativeWebSocketConfiguration.class.getName(), configuration); + context.setAttribute(NativeWebSocketConfiguration.class.getName(), configuration); server.start(); return server; } }}); - + + // Embedded WSUF, added as filter, apply app-ws configuration via wsuf constructor + + cases.add(new Object[]{"wsuf/addFilter/WSUF Constructor configure", new ServerProvider() + { + @Override + public Server newServer() throws Exception + { + Server server = new Server(); + ServerConnector connector = new ServerConnector(server); + connector.setPort(0); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + server.setHandler(context); + + NativeWebSocketConfiguration configuration = new NativeWebSocketConfiguration(context.getServletContext()); + configuration.getFactory().getPolicy().setMaxTextMessageSize(10 * 1024 * 1024); + configuration.addMapping(new ServletPathSpec("/info/*"), infoCreator); + context.addBean(configuration, true); + + FilterHolder wsufHolder = new FilterHolder(new WebSocketUpgradeFilter(configuration)); + context.addFilter(wsufHolder, "/*", EnumSet.of(DispatcherType.REQUEST)); + + server.start(); + + return server; + } + }}); + // Embedded WSUF, added as filter, apply app-ws configuration via ServletContextListener cases.add(new Object[]{"wsuf.configureContext/ServletContextListener configure", new ServerProvider() From c698c95e9bfb97648b2b3c8edb7c206af4f2084a Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 15 Dec 2016 10:44:34 -0700 Subject: [PATCH 3/3] Issue #1114 - removing redundant exportConfiguration --- .../jetty/websocket/server/WebSocketUpgradeFilter.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java index 42044bc2c0d..85f97b3b52e 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java @@ -110,7 +110,6 @@ public class WebSocketUpgradeFilter implements Filter, MappedWebSocketCreator, D } private NativeWebSocketConfiguration configuration; - private boolean exportConfiguration = true; private boolean localConfiguration = false; private boolean alreadySetToAttribute = false; @@ -127,7 +126,6 @@ public class WebSocketUpgradeFilter implements Filter, MappedWebSocketCreator, D public WebSocketUpgradeFilter(NativeWebSocketConfiguration configuration) { this.configuration = configuration; - this.exportConfiguration = false; } @Override @@ -306,7 +304,7 @@ public class WebSocketUpgradeFilter implements Filter, MappedWebSocketCreator, D NativeWebSocketConfiguration.class.getName() + " at ServletContext attribute '" + configurationKey + "'"); } } - else if (exportConfiguration) + else { // We have a NativeWebSocketConfiguration already present, make sure it exists on the ServletContext if (config.getServletContext().getAttribute(configurationKey) == null)