Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simplify the usage of WebSocketUpgradeFilter in jetty 10 #5413

Merged
merged 6 commits into from
Nov 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.IOException;
import java.net.URI;
import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
Expand All @@ -28,10 +29,13 @@
import org.eclipse.jetty.client.HttpResponse;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.ExtensionConfig;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.FrameHandler;
import org.eclipse.jetty.websocket.core.OpCode;
import org.eclipse.jetty.websocket.core.TestFrameHandler;
import org.eclipse.jetty.websocket.core.WebSocketServer;
import org.eclipse.jetty.websocket.core.client.CoreClientUpgradeRequest;
Expand Down Expand Up @@ -291,7 +295,8 @@ public void onHandshakeResponse(HttpRequest request, HttpResponse response)

// We should now only be able to send this message in multiple frames as it exceeds deflate_buffer_size.
String message = "0123456789";
clientHandler.sendText(message);
ByteBuffer payload = toBuffer(message, true);
clientHandler.getCoreSession().sendFrame(new Frame(OpCode.TEXT, payload), Callback.NOOP, false);

// Verify the frame has been fragmented into multiple parts.
int numFrames = 0;
Expand All @@ -315,4 +320,18 @@ public void onHandshakeResponse(HttpRequest request, HttpResponse response)
assertNull(serverHandler.getError());
assertNull(clientHandler.getError());
}

public ByteBuffer toBuffer(String string, boolean direct)
{
ByteBuffer buffer = BufferUtil.allocate(string.length(), direct);
BufferUtil.clearToFill(buffer);
BufferUtil.put(BufferUtil.toBuffer(string), buffer);
BufferUtil.flipToFlush(buffer, 0);

// Sanity checks.
assertThat(buffer.hasArray(), is(!direct));
assertThat(BufferUtil.toString(buffer), is(string));

return buffer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
<filter>
<filter-name>wsuf-test</filter-name>
<filter-class>org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter</filter-class>
<init-param>
<param-name>jetty.websocket.WebSocketMapping</param-name>
<param-value>jetty.websocket.defaultMapping</param-value>
</init-param>
</filter>

<filter-mapping>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl

private final ServletContextHandler contextHandler;
private final WebSocketMapping webSocketMapping;
private final WebSocketComponents components;
private final FrameHandlerFactory frameHandlerFactory;
private final Executor executor;
private final Configuration.ConfigurationCustomizer customizer = new Configuration.ConfigurationCustomizer();
Expand All @@ -102,14 +103,15 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl
* 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 components the {@link WebSocketComponents} instance to use
* @param executor the {@link Executor} to use
*/
JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMapping webSocketMapping, WebSocketComponents webSocketComponents, Executor executor)
JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMapping webSocketMapping, WebSocketComponents components, Executor executor)
{
this.contextHandler = contextHandler;
this.webSocketMapping = webSocketMapping;
this.executor = executor;
this.components = components;

// Ensure there is a FrameHandlerFactory
JettyServerFrameHandlerFactory factory = contextHandler.getBean(JettyServerFrameHandlerFactory.class);
Expand Down Expand Up @@ -155,6 +157,11 @@ public void addMapping(String pathSpec, final Class<?> endpointClass)
});
}

public WebSocketComponents getWebSocketComponents()
{
return components;
}

@Override
public Executor getExecutor()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,9 @@ public static void configure(ServletContextHandler context, Configurator configu
private static JettyWebSocketServerContainer initialize(ServletContextHandler context)
{
WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServer(), context.getServletContext());
WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY);
JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(context.getServletContext());

if (LOG.isDebugEnabled())
LOG.debug("configureContext {} {} {}", container, mapping, components);
LOG.debug("initialize {} {}", container, components);

return container;
}
Expand All @@ -105,6 +103,8 @@ private static JettyWebSocketServerContainer initialize(ServletContextHandler co
public void onStartup(Set<Class<?>> c, ServletContext context)
{
ServletContextHandler contextHandler = ServletContextHandler.getServletContextHandler(context, "Jetty WebSocket SCI");
JettyWebSocketServletContainerInitializer.initialize(contextHandler);
JettyWebSocketServerContainer container = JettyWebSocketServletContainerInitializer.initialize(contextHandler);
if (LOG.isDebugEnabled())
LOG.debug("onStartup {}", container);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@
import java.net.URI;
import java.time.Duration;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import javax.servlet.DispatcherType;

import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.Server;
Expand All @@ -39,7 +37,6 @@
import org.eclipse.jetty.websocket.common.WebSocketSession;
import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession;
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;
Expand All @@ -54,22 +51,20 @@

public class ErrorCloseTest
{
private Server server = new Server();
private WebSocketClient client = new WebSocketClient();
private ThrowingSocket serverSocket = new ThrowingSocket();
private CountDownLatch serverCloseListener = new CountDownLatch(1);
private ServerConnector connector;
private final Server server = new Server();
private final WebSocketClient client = new WebSocketClient();
private final ThrowingSocket serverSocket = new ThrowingSocket();
private final CountDownLatch serverCloseListener = new CountDownLatch(1);
private URI serverUri;

@BeforeEach
public void start() throws Exception
{
connector = new ServerConnector(server);
ServerConnector connector = new ServerConnector(server);
server.addConnector(connector);

ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
contextHandler.setContextPath("/");
contextHandler.addFilter(WebSocketUpgradeFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) ->
{
container.addMapping("/", (req, resp) -> serverSocket);
Expand Down Expand Up @@ -140,7 +135,7 @@ public void testOnOpenThrows() throws Exception
serverSocket.methodsToThrow.add("onOpen");
EventSocket clientSocket = new EventSocket();

try (StacklessLogging stacklessLogging = new StacklessLogging(WebSocketCoreSession.class))
try (StacklessLogging ignored = new StacklessLogging(WebSocketCoreSession.class))
{
client.connect(clientSocket, serverUri).get(5, TimeUnit.SECONDS);
assertTrue(serverSocket.closeLatch.await(5, TimeUnit.SECONDS));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@
import java.net.SocketTimeoutException;
import java.net.URI;
import java.time.Duration;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import javax.servlet.DispatcherType;

import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
Expand All @@ -49,7 +47,6 @@
import org.eclipse.jetty.websocket.tests.EchoSocket;
import org.eclipse.jetty.websocket.tests.GetAuthHeaderEndpoint;
import org.eclipse.jetty.websocket.tests.SimpleStatusServlet;
import org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter;
import org.hamcrest.Matcher;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -73,7 +70,7 @@ public class ClientConnectTest
{
private Server server;
private WebSocketClient client;
private CountDownLatch serverLatch = new CountDownLatch(1);
private final CountDownLatch serverLatch = new CountDownLatch(1);

@SuppressWarnings("unchecked")
private <E extends Throwable> E assertExpectedError(ExecutionException e, CloseTrackingEndpoint wsocket, Matcher<Throwable> errorMatcher)
Expand Down Expand Up @@ -143,8 +140,6 @@ public void startServer() throws Exception
});
});

context.addFilter(WebSocketUpgradeFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));

context.addServlet(new ServletHolder(new SimpleStatusServlet(404)), "/bogus");
context.addServlet(new ServletHolder(new SimpleStatusServlet(200)), "/a-okay");
context.addServlet(new ServletHolder(new InvalidUpgradeServlet()), "/invalid-upgrade/*");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,10 @@
import java.time.Duration;
import java.util.Arrays;
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import javax.servlet.DispatcherType;

import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
Expand All @@ -50,7 +48,6 @@
import org.eclipse.jetty.websocket.tests.EchoSocket;
import org.eclipse.jetty.websocket.tests.ParamsEndpoint;
import org.eclipse.jetty.websocket.tests.util.FutureWriteCallback;
import org.eclipse.jetty.websocket.util.server.WebSocketUpgradeFilter;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -104,10 +101,7 @@ public void startServer() throws Exception
configuration.addMapping("/get-params", (req, resp) -> new ParamsEndpoint());
});

context.addFilter(WebSocketUpgradeFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));

server.setHandler(context);

server.start();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
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;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -78,34 +77,40 @@ 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}.
*
* @param servletContext the {@link ServletContext} to use.
* @return the configured default {@link WebSocketUpgradeFilter} instance.
*/
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;
}

/**
* Configure the default WebSocketUpgradeFilter.
*
* <p>
* This will return the default {@link WebSocketUpgradeFilter} on the
* provided {@link ServletContext}, creating the filter if necessary.
* Ensure a {@link WebSocketUpgradeFilter} is available on the provided {@link ServletContext},
* a new filter will added if one does not already exist.
* </p>
* <p>
* The default {@link WebSocketUpgradeFilter} is also available via
* the {@link ServletContext} attribute named {@code org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter}
* </p>
*
* @param servletContext the {@link ServletContext} to use
* @return the configured default {@link WebSocketUpgradeFilter} instance
* @param servletContext the {@link ServletContext} to use.
* @return the configured default {@link WebSocketUpgradeFilter} instance.
*/
public static FilterHolder ensureFilter(ServletContext servletContext)
{
Expand All @@ -132,8 +137,6 @@ public static FilterHolder ensureFilter(ServletContext servletContext)
}
}

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

private final Configuration.ConfigurationCustomizer defaultCustomizer = new Configuration.ConfigurationCustomizer();
private WebSocketMapping mapping;

Expand Down Expand Up @@ -174,10 +177,9 @@ public void init(FilterConfig config) throws ServletException
final ServletContext context = config.getServletContext();

String mappingKey = config.getInitParameter(MAPPING_ATTRIBUTE_INIT_PARAM);
if (mappingKey != null)
mapping = WebSocketMapping.ensureMapping(context, mappingKey);
else
mapping = new WebSocketMapping(WebSocketServerComponents.getWebSocketComponents(context));
if (mappingKey == null)
throw new ServletException("the WebSocketMapping init param must be set");
mapping = WebSocketMapping.ensureMapping(context, mappingKey);

String max = config.getInitParameter("idleTimeout");
if (max == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public class WebSocketMapping implements Dumpable, LifeCycle.Listener
public static WebSocketMapping getMapping(ServletContext servletContext, String mappingKey)
{
Object mappingObject = servletContext.getAttribute(mappingKey);

if (mappingObject != null)
{
if (mappingObject instanceof WebSocketMapping)
Expand All @@ -86,7 +85,6 @@ public WebSocketCreator getMapping(PathSpec pathSpec)
public static WebSocketMapping ensureMapping(ServletContext servletContext, String mappingKey)
{
WebSocketMapping mapping = getMapping(servletContext, mappingKey);

if (mapping == null)
{
mapping = new WebSocketMapping(WebSocketServerComponents.getWebSocketComponents(servletContext));
Expand Down Expand Up @@ -135,7 +133,7 @@ else if (rawSpec.startsWith("uri-template|"))
throw new IllegalArgumentException("Unrecognized path spec syntax [" + rawSpec + "]");
}

public static final String DEFAULT_KEY = "org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping";
public static final String DEFAULT_KEY = "jetty.websocket.defaultMapping";

private final PathMappings<Negotiator> mappings = new PathMappings<>();
private final WebSocketComponents components;
Expand Down