Skip to content

Commit

Permalink
Merge pull request #5405 from eclipse/jetty-10.0.x-5378-LazyWSUpgrade…
Browse files Browse the repository at this point in the history 
…Filter

Issue #5378 - add test to reproduce issue with WebSocketUpgradeFilter not started
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Oct 8, 2020
2 parents 94ef934 + d2beb28 commit 967749b
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 21 deletions.
93 changes: 89 additions & 4 deletions ...jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketFilterTest.java
View file
Expand Up @@ -21,19 +21,24 @@
import java.net.URI;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import javax.servlet.http.HttpServlet;

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.servlet.ServletHolder;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.client.WebSocketClient;
import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer;
import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer;
import org.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;
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.assertTrue;
Expand All @@ -45,18 +50,29 @@ public class JettyWebSocketFilterTest
private WebSocketClient client;
private ServletContextHandler contextHandler;

@BeforeEach
public void start() throws Exception
public void start(JettyWebSocketServletContainerInitializer.Configurator configurator) throws Exception
{
start(configurator, null);
}

public void start(ServletHolder servletHolder) throws Exception
{
start(null, servletHolder);
}

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, null);
JettyWebSocketServletContainerInitializer.configure(contextHandler, configurator);
server.start();

client = new WebSocketClient();
Expand All @@ -70,9 +86,37 @@ public void stop() throws Exception
server.stop();
}

@Test
public void testWebSocketUpgradeFilter() throws Exception
{
start((context, container) -> container.addMapping("/", EchoSocket.class));

// After mapping is added we have an UpgradeFilter.
assertThat(contextHandler.getServletHandler().getFilters().length, is(1));
FilterHolder filterHolder = contextHandler.getServletHandler().getFilter("WebSocketUpgradeFilter");
assertNotNull(filterHolder);
assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED));
assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class));

// Test we can upgrade to websocket and send a message.
URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath");
EventSocket socket = new EventSocket();
CompletableFuture<Session> connect = client.connect(socket, uri);
try (Session session = connect.get(5, TimeUnit.SECONDS))
{
session.getRemote().sendString("hello world");
}
assertTrue(socket.closeLatch.await(10, TimeUnit.SECONDS));

String msg = socket.textMessages.poll();
assertThat(msg, is("hello world"));
}

@Test
public void testLazyWebSocketUpgradeFilter() throws Exception
{
start(null, null);

// JettyWebSocketServerContainer has already been created.
JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(contextHandler.getServletContext());
assertNotNull(container);
Expand All @@ -83,6 +127,47 @@ 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");
assertNotNull(filterHolder);
assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED));
assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class));

// Test we can upgrade to websocket and send a message.
URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath");
EventSocket socket = new EventSocket();
CompletableFuture<Session> connect = client.connect(socket, uri);
try (Session session = connect.get(5, TimeUnit.SECONDS))
{
session.getRemote().sendString("hello world");
}
assertTrue(socket.closeLatch.await(10, TimeUnit.SECONDS));

String msg = socket.textMessages.poll();
assertThat(msg, is("hello world"));
}

@Test
public void testWebSocketUpgradeFilterWhileStarting() throws Exception
{
start(new ServletHolder(new HttpServlet()
{
@Override
public void init()
{
JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(getServletContext());
if (container == null)
throw new IllegalArgumentException("Missing JettyWebSocketServerContainer");

container.addMapping("/", EchoSocket.class);
}
}));

// After mapping is added we have an UpgradeFilter.
assertThat(contextHandler.getServletHandler().getFilters().length, is(1));
FilterHolder filterHolder = contextHandler.getServletHandler().getFilter("WebSocketUpgradeFilter");
assertNotNull(filterHolder);
assertThat(filterHolder.getState(), is(AbstractLifeCycle.STARTED));
assertThat(filterHolder.getFilter(), instanceOf(WebSocketUpgradeFilter.class));

// Test we can upgrade to websocket and send a message.
URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath");
Expand Down
43 changes: 26 additions & 17 deletions ...-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java
View file
Expand Up @@ -21,6 +21,7 @@
import java.io.IOException;
import java.time.Duration;
import java.util.EnumSet;
import java.util.Objects;
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
Expand All @@ -37,6 +38,7 @@
import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.util.annotation.ManagedObject;
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.core.server.WebSocketServerComponents;
import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping;
Expand Down Expand Up @@ -74,10 +76,12 @@
public class WebSocketUpgradeFilter implements Filter, Dumpable
{
private static final Logger LOG = LoggerFactory.getLogger(WebSocketUpgradeFilter.class);
private static final AutoLock LOCK = new AutoLock();

private static FilterHolder getFilter(ServletContext servletContext)
{
ServletHandler servletHandler = ContextHandler.getContextHandler(servletContext).getChildHandlerByClass(ServletHandler.class);
ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext));
ServletHandler servletHandler = contextHandler.getChildHandlerByClass(ServletHandler.class);

for (FilterHolder holder : servletHandler.getFilters())
{
Expand Down Expand Up @@ -105,22 +109,27 @@ private static FilterHolder getFilter(ServletContext servletContext)
*/
public static FilterHolder ensureFilter(ServletContext servletContext)
{
FilterHolder existingFilter = WebSocketUpgradeFilter.getFilter(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.setAsyncSupported(true);
ServletHandler servletHandler = ContextHandler.getContextHandler(servletContext).getChildHandlerByClass(ServletHandler.class);
servletHandler.addFilterWithMapping(holder, pathSpec, EnumSet.of(DispatcherType.REQUEST));
if (LOG.isDebugEnabled())
LOG.debug("Adding {} mapped to {} in {}", holder, pathSpec, servletContext);
return holder;
// Lock in case two concurrent requests are initializing the filter lazily.
try (AutoLock l = LOCK.lock())
{
FilterHolder existingFilter = WebSocketUpgradeFilter.getFilter(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.setAsyncSupported(true);
ContextHandler contextHandler = Objects.requireNonNull(ContextHandler.getContextHandler(servletContext));
ServletHandler servletHandler = contextHandler.getChildHandlerByClass(ServletHandler.class);
servletHandler.addFilterWithMapping(holder, pathSpec, EnumSet.of(DispatcherType.REQUEST));
if (LOG.isDebugEnabled())
LOG.debug("Adding {} mapped to {} in {}", holder, pathSpec, servletContext);
return holder;
}
}

public static final String MAPPING_ATTRIBUTE_INIT_PARAM = "org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping.key";
Expand Down

0 comments on commit 967749b

Please sign in to comment.