From 35b0c8b5772e5a3e3721d589d0ef67d6ca3adbaa Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 9 Dec 2016 14:58:18 +0100 Subject: [PATCH] Update websocket support for Jetty 9.3+ Due to a few changes in `WebSocketServerFactory` and `Session` API, our `JettyRequestUpgradeStrategy` and `JettyWebSocketSession` needed to adapt. As of 9.3.15+ and 9.4.0+, some reflection is required to support previous versions. Spring 4.3 websocket officially supports Jetty 9.1 to 9.3. Issue: SPR-14940 --- build.gradle | 7 +- .../adapter/jetty/JettyWebSocketSession.java | 85 +++++++++- .../jetty/JettyRequestUpgradeStrategy.java | 150 ++++++++++++------ 3 files changed, 187 insertions(+), 55 deletions(-) diff --git a/build.gradle b/build.gradle index 4992b87791..3c11f0839e 100644 --- a/build.gradle +++ b/build.gradle @@ -56,6 +56,7 @@ configure(allprojects) { project -> ext.jasperreportsVersion = "6.2.1" // our tests fail with JR-internal NPEs against 6.2.2 and higher ext.javamailVersion = "1.5.6" ext.jettyVersion = "9.3.13.v20161014" + ext.jetty94Version = "9.4.0.v20161208" // for spring-websocket support ext.jodaVersion = "2.9.6" ext.jrubyVersion = "1.7.26" // JRuby 9000 only supported through JSR-223 (StandardScriptFactory) ext.jtaVersion = "1.2" @@ -949,11 +950,11 @@ project("spring-websocket") { optional("org.eclipse.jetty:jetty-webapp:${jettyVersion}") { exclude group: "javax.servlet", module: "javax.servlet" } - optional("org.eclipse.jetty.websocket:websocket-server:${jettyVersion}") { + optional("org.eclipse.jetty.websocket:websocket-server:${jetty94Version}") { exclude group: "javax.servlet", module: "javax.servlet" } - optional("org.eclipse.jetty.websocket:websocket-client:${jettyVersion}") - optional("org.eclipse.jetty:jetty-client:${jettyVersion}") + optional("org.eclipse.jetty.websocket:websocket-client:${jetty94Version}") + optional("org.eclipse.jetty:jetty-client:${jetty94Version}") optional("io.undertow:undertow-core:${undertowVersion}") optional("io.undertow:undertow-servlet:${undertowVersion}") { exclude group: "org.jboss.spec.javax.servlet", module: "jboss-servlet-api_3.1_spec" diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketSession.java index 2825a48243..5ae271d3f6 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketSession.java @@ -17,6 +17,7 @@ package org.springframework.web.socket.adapter.jetty; import java.io.IOException; +import java.lang.reflect.Method; import java.net.InetSocketAddress; import java.net.URI; import java.security.Principal; @@ -26,11 +27,14 @@ import java.util.Map; import org.eclipse.jetty.websocket.api.RemoteEndpoint; import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.UpgradeRequest; +import org.eclipse.jetty.websocket.api.UpgradeResponse; import org.eclipse.jetty.websocket.api.WebSocketException; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.springframework.http.HttpHeaders; import org.springframework.util.ObjectUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.web.socket.BinaryMessage; import org.springframework.web.socket.CloseStatus; import org.springframework.web.socket.PingMessage; @@ -45,10 +49,22 @@ import org.springframework.web.socket.adapter.AbstractWebSocketSession; * * @author Phillip Webb * @author Rossen Stoyanchev + * @author Brian Clozel * @since 4.0 */ public class JettyWebSocketSession extends AbstractWebSocketSession { + // As of Jetty 9.4, UpgradeRequest and UpgradeResponse are interfaces instead of classes + private static final boolean isJetty94; + + private static Method getUpgradeRequest; + private static Method getUpgradeResponse; + private static Method getRequestURI; + private static Method getHeaders; + private static Method getAcceptedSubProtocol; + private static Method getExtensions; + private static Method getUserPrincipal; + private String id; private URI uri; @@ -61,6 +77,23 @@ public class JettyWebSocketSession extends AbstractWebSocketSession { private Principal user; + static { + isJetty94 = UpgradeRequest.class.isInterface(); + if (!isJetty94) { + try { + getUpgradeRequest = Session.class.getMethod("getUpgradeRequest"); + getUpgradeResponse = Session.class.getMethod("getUpgradeResponse"); + getRequestURI = UpgradeRequest.class.getMethod("getRequestURI"); + getHeaders = UpgradeRequest.class.getMethod("getHeaders"); + getAcceptedSubProtocol = UpgradeResponse.class.getMethod("getAcceptedSubProtocol"); + getExtensions = UpgradeResponse.class.getMethod("getExtensions"); + getUserPrincipal = UpgradeRequest.class.getMethod("getUserPrincipal"); + } + catch (NoSuchMethodException ex) { + throw new IllegalStateException("Incompatible Jetty API", ex); + } + } + } /** * Create a new {@link JettyWebSocketSession} instance. @@ -164,20 +197,64 @@ public class JettyWebSocketSession extends AbstractWebSocketSession { @Override public void initializeNativeSession(Session session) { super.initializeNativeSession(session); + if (isJetty94) { + initializeJetty94Session(session); + } + else { + initializeJettySession(session); + } + } + @SuppressWarnings("unchecked") + private void initializeJettySession(Session session) { + + Object request = ReflectionUtils.invokeMethod(getUpgradeRequest, session); + Object response = ReflectionUtils.invokeMethod(getUpgradeResponse, session); + + this.id = ObjectUtils.getIdentityHexString(getNativeSession()); + this.uri = (URI) ReflectionUtils.invokeMethod(getRequestURI, request); + + this.headers = new HttpHeaders(); + this.headers.putAll((Map>) ReflectionUtils.invokeMethod(getHeaders, request)); + this.headers = HttpHeaders.readOnlyHttpHeaders(headers); + + this.acceptedProtocol = (String) ReflectionUtils.invokeMethod(getAcceptedSubProtocol, response); + + List source = (List) ReflectionUtils.invokeMethod(getExtensions, response); + if (source != null) { + this.extensions = new ArrayList(source.size()); + for (ExtensionConfig ec : source) { + this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters())); + } + } + else { + this.extensions = new ArrayList(0); + } + + if (this.user == null) { + this.user = (Principal) ReflectionUtils.invokeMethod(getUserPrincipal, request); + } + } + + private void initializeJetty94Session(Session session) { this.id = ObjectUtils.getIdentityHexString(getNativeSession()); this.uri = session.getUpgradeRequest().getRequestURI(); this.headers = new HttpHeaders(); - this.headers.putAll(getNativeSession().getUpgradeRequest().getHeaders()); + this.headers.putAll(session.getUpgradeRequest().getHeaders()); this.headers = HttpHeaders.readOnlyHttpHeaders(headers); this.acceptedProtocol = session.getUpgradeResponse().getAcceptedSubProtocol(); List source = getNativeSession().getUpgradeResponse().getExtensions(); - this.extensions = new ArrayList(source.size()); - for (ExtensionConfig ec : source) { - this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters())); + if (source != null) { + this.extensions = new ArrayList(source.size()); + for (ExtensionConfig ec : source) { + this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters())); + } + } + else { + this.extensions = new ArrayList(0); } if (this.user == null) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java index 51c5aaeac0..73ee31622d 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java @@ -17,7 +17,6 @@ package org.springframework.web.socket.server.jetty; import java.io.IOException; -import java.lang.reflect.Method; import java.security.Principal; import java.util.ArrayList; import java.util.List; @@ -27,8 +26,7 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.websocket.api.UpgradeRequest; -import org.eclipse.jetty.websocket.api.UpgradeResponse; +import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.eclipse.jetty.websocket.server.HandshakeRFC6455; @@ -56,69 +54,54 @@ import org.springframework.web.socket.server.HandshakeFailureException; import org.springframework.web.socket.server.RequestUpgradeStrategy; /** - * A {@link RequestUpgradeStrategy} for use with Jetty 9.0-9.3. Based on Jetty's + * A {@link RequestUpgradeStrategy} for use with Jetty 9.1-9.3. Based on Jetty's * internal {@code org.eclipse.jetty.websocket.server.WebSocketHandler} class. * * @author Phillip Webb * @author Rossen Stoyanchev + * @author Brian Clozel * @since 4.0 */ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Lifecycle, ServletContextAware { - // Pre-Jetty 9.3 init method without ServletContext - private static final Method webSocketFactoryInitMethod = - ClassUtils.getMethodIfAvailable(WebSocketServerFactory.class, "init"); - private static final ThreadLocal wsContainerHolder = new NamedThreadLocal("WebSocket Handler Container"); + // Actually 9.3.15+ + private static boolean isJetty94 = ClassUtils.hasConstructor(WebSocketServerFactory.class, ServletContext.class); - private final WebSocketServerFactory factory; + private static boolean isJetty91 = ClassUtils.hasMethod(WebSocketServerFactory.class, "init"); + + private WebSocketServerFactoryAdapter factoryAdapter; private volatile List supportedExtensions; - private ServletContext servletContext; + protected ServletContext servletContext; private volatile boolean running = false; - /** * Default constructor that creates {@link WebSocketServerFactory} through * its default constructor thus using a default {@link WebSocketPolicy}. */ public JettyRequestUpgradeStrategy() { - this(new WebSocketServerFactory()); + this(WebSocketPolicy.newServerPolicy()); } /** - * A constructor accepting a {@link WebSocketServerFactory}. - * This may be useful for modifying the factory's {@link WebSocketPolicy} - * via {@link WebSocketServerFactory#getPolicy()}. + * A constructor accepting a {@link WebSocketPolicy} + * to be used when creating the {@link WebSocketServerFactory} instance. + * @since 4.3 */ - public JettyRequestUpgradeStrategy(WebSocketServerFactory factory) { - Assert.notNull(factory, "WebSocketServerFactory must not be null"); - this.factory = factory; - this.factory.setCreator(new WebSocketCreator() { - @Override - public Object createWebSocket(ServletUpgradeRequest request, ServletUpgradeResponse response) { - // Cast to avoid infinite recursion - return createWebSocket((UpgradeRequest) request, (UpgradeResponse) response); - } - // For Jetty 9.0.x - public Object createWebSocket(UpgradeRequest request, UpgradeResponse response) { - WebSocketHandlerContainer container = wsContainerHolder.get(); - Assert.state(container != null, "Expected WebSocketHandlerContainer"); - response.setAcceptedSubProtocol(container.getSelectedProtocol()); - response.setExtensions(container.getExtensionConfigs()); - return container.getHandler(); - } - }); + public JettyRequestUpgradeStrategy(WebSocketPolicy webSocketPolicy) { + this.factoryAdapter = isJetty94 ? new Jetty94WebSocketServerFactoryAdapter() + : new JettyWebSocketServerFactoryAdapter(); + this.factoryAdapter.setWebSocketPolicy(webSocketPolicy); } - @Override public String[] getSupportedVersions() { - return new String[] { String.valueOf(HandshakeRFC6455.VERSION) }; + return new String[] {String.valueOf(HandshakeRFC6455.VERSION)}; } @Override @@ -131,7 +114,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life private List getWebSocketExtensions() { List result = new ArrayList(); - for (String name : this.factory.getExtensionFactory().getExtensionNames()) { + for (String name : this.factoryAdapter.getFactory().getExtensionFactory().getExtensionNames()) { result.add(new WebSocketExtension(name)); } return result; @@ -153,15 +136,10 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life if (!isRunning()) { this.running = true; try { - if (webSocketFactoryInitMethod != null) { - webSocketFactoryInitMethod.invoke(this.factory); - } - else { - this.factory.init(this.servletContext); - } + this.factoryAdapter.start(); } catch (Exception ex) { - throw new IllegalStateException("Unable to initialize Jetty WebSocketServerFactory", ex); + throw new IllegalStateException("Unable to start Jetty WebSocketServerFactory", ex); } } } @@ -169,8 +147,13 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life @Override public void stop() { if (isRunning()) { - this.running = false; - this.factory.cleanup(); + try { + this.running = false; + factoryAdapter.stop(); + } + catch (Exception ex) { + throw new IllegalStateException("Unable to stop Jetty WebSocketServerFactory", ex); + } } } @@ -185,7 +168,8 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life Assert.isInstanceOf(ServletServerHttpResponse.class, response); HttpServletResponse servletResponse = ((ServletServerHttpResponse) response).getServletResponse(); - Assert.isTrue(this.factory.isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake"); + Assert.isTrue(this.factoryAdapter.getFactory() + .isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake"); JettyWebSocketSession session = new JettyWebSocketSession(attributes, user); JettyWebSocketHandlerAdapter handlerAdapter = new JettyWebSocketHandlerAdapter(wsHandler, session); @@ -195,7 +179,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life try { wsContainerHolder.set(container); - this.factory.acceptWebSocket(servletRequest, servletResponse); + this.factoryAdapter.getFactory().acceptWebSocket(servletRequest, servletResponse); } catch (IOException ex) { throw new HandshakeFailureException( @@ -219,7 +203,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life this.handler = handler; this.selectedProtocol = protocol; if (CollectionUtils.isEmpty(extensions)) { - this.extensionConfigs = null; + this.extensionConfigs = new ArrayList(); } else { this.extensionConfigs = new ArrayList(); @@ -242,4 +226,74 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life } } + private static abstract class WebSocketServerFactoryAdapter { + + protected WebSocketServerFactory factory; + + protected WebSocketPolicy webSocketPolicy; + + public WebSocketServerFactory getFactory() { + return factory; + } + + public void setWebSocketPolicy(WebSocketPolicy webSocketPolicy) { + this.webSocketPolicy = webSocketPolicy; + } + + protected void configureFactory() { + this.factory.setCreator(new WebSocketCreator() { + @Override + public Object createWebSocket(ServletUpgradeRequest request, ServletUpgradeResponse response) { + WebSocketHandlerContainer container = wsContainerHolder.get(); + Assert.state(container != null, "Expected WebSocketHandlerContainer"); + response.setAcceptedSubProtocol(container.getSelectedProtocol()); + response.setExtensions(container.getExtensionConfigs()); + return container.getHandler(); + } + }); + } + + abstract void start() throws Exception; + + abstract void stop() throws Exception; + } + + private class JettyWebSocketServerFactoryAdapter extends WebSocketServerFactoryAdapter { + + @Override + void start() throws Exception { + this.factory = WebSocketServerFactory.class.getConstructor(WebSocketPolicy.class) + .newInstance(this.webSocketPolicy); + configureFactory(); + if(isJetty91) { + WebSocketServerFactory.class.getMethod("init").invoke(this.factory); + } + else { + WebSocketServerFactory.class.getMethod("init", ServletContext.class) + .invoke(this.factory, servletContext); + } + } + + @Override + void stop() throws Exception { + WebSocketServerFactory.class.getMethod("cleanup").invoke(this.factory); + } + } + + private class Jetty94WebSocketServerFactoryAdapter extends WebSocketServerFactoryAdapter { + + @Override + void start() throws Exception { + servletContext.setAttribute(DecoratedObjectFactory.ATTR, new DecoratedObjectFactory()); + this.factory = new WebSocketServerFactory(servletContext, this.webSocketPolicy); + configureFactory(); + this.factory.start(); + } + + @Override + void stop() throws Exception { + this.factory.stop(); + } + } + }