From 75034c3710c999c687183cb112f8d1b8eca0c2c2 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 8 Dec 2016 22:48:06 +0100 Subject: [PATCH] Revert "Update websocket support for Jetty 9.3+" This reverts commit 305cea28c5ddf9c87d616c4b370248809b849823. --- build.gradle | 2 +- .../adapter/jetty/JettyWebSocketSession.java | 85 +--------- .../jetty/JettyRequestUpgradeStrategy.java | 150 ++++++------------ 3 files changed, 53 insertions(+), 184 deletions(-) diff --git a/build.gradle b/build.gradle index 01ab2fa331..4992b87791 100644 --- a/build.gradle +++ b/build.gradle @@ -55,7 +55,7 @@ configure(allprojects) { project -> ext.jackson2Version = "2.8.4" 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.4.0.RC3" + ext.jettyVersion = "9.3.13.v20161014" ext.jodaVersion = "2.9.6" ext.jrubyVersion = "1.7.26" // JRuby 9000 only supported through JSR-223 (StandardScriptFactory) ext.jtaVersion = "1.2" 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 5ae271d3f6..2825a48243 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,7 +17,6 @@ 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; @@ -27,14 +26,11 @@ 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; @@ -49,22 +45,10 @@ 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; @@ -77,23 +61,6 @@ 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. @@ -197,64 +164,20 @@ 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(session.getUpgradeRequest().getHeaders()); + this.headers.putAll(getNativeSession().getUpgradeRequest().getHeaders()); this.headers = HttpHeaders.readOnlyHttpHeaders(headers); this.acceptedProtocol = session.getUpgradeResponse().getAcceptedSubProtocol(); List source = getNativeSession().getUpgradeResponse().getExtensions(); - 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); + this.extensions = new ArrayList(source.size()); + for (ExtensionConfig ec : source) { + this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters())); } 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 73ee31622d..51c5aaeac0 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,6 +17,7 @@ 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; @@ -26,7 +27,8 @@ import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.util.DecoratedObjectFactory; +import org.eclipse.jetty.websocket.api.UpgradeRequest; +import org.eclipse.jetty.websocket.api.UpgradeResponse; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; import org.eclipse.jetty.websocket.server.HandshakeRFC6455; @@ -54,54 +56,69 @@ import org.springframework.web.socket.server.HandshakeFailureException; import org.springframework.web.socket.server.RequestUpgradeStrategy; /** - * A {@link RequestUpgradeStrategy} for use with Jetty 9.1-9.3. Based on Jetty's + * A {@link RequestUpgradeStrategy} for use with Jetty 9.0-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 static boolean isJetty91 = ClassUtils.hasMethod(WebSocketServerFactory.class, "init"); - - private WebSocketServerFactoryAdapter factoryAdapter; + private final WebSocketServerFactory factory; private volatile List supportedExtensions; - protected ServletContext servletContext; + private 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(WebSocketPolicy.newServerPolicy()); + this(new WebSocketServerFactory()); } /** - * A constructor accepting a {@link WebSocketPolicy} - * to be used when creating the {@link WebSocketServerFactory} instance. - * @since 4.3 + * A constructor accepting a {@link WebSocketServerFactory}. + * This may be useful for modifying the factory's {@link WebSocketPolicy} + * via {@link WebSocketServerFactory#getPolicy()}. */ - public JettyRequestUpgradeStrategy(WebSocketPolicy webSocketPolicy) { - this.factoryAdapter = isJetty94 ? new Jetty94WebSocketServerFactoryAdapter() - : new JettyWebSocketServerFactoryAdapter(); - this.factoryAdapter.setWebSocketPolicy(webSocketPolicy); + 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(); + } + }); } + @Override public String[] getSupportedVersions() { - return new String[] {String.valueOf(HandshakeRFC6455.VERSION)}; + return new String[] { String.valueOf(HandshakeRFC6455.VERSION) }; } @Override @@ -114,7 +131,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life private List getWebSocketExtensions() { List result = new ArrayList(); - for (String name : this.factoryAdapter.getFactory().getExtensionFactory().getExtensionNames()) { + for (String name : this.factory.getExtensionFactory().getExtensionNames()) { result.add(new WebSocketExtension(name)); } return result; @@ -136,10 +153,15 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life if (!isRunning()) { this.running = true; try { - this.factoryAdapter.start(); + if (webSocketFactoryInitMethod != null) { + webSocketFactoryInitMethod.invoke(this.factory); + } + else { + this.factory.init(this.servletContext); + } } catch (Exception ex) { - throw new IllegalStateException("Unable to start Jetty WebSocketServerFactory", ex); + throw new IllegalStateException("Unable to initialize Jetty WebSocketServerFactory", ex); } } } @@ -147,13 +169,8 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life @Override public void stop() { if (isRunning()) { - try { - this.running = false; - factoryAdapter.stop(); - } - catch (Exception ex) { - throw new IllegalStateException("Unable to stop Jetty WebSocketServerFactory", ex); - } + this.running = false; + this.factory.cleanup(); } } @@ -168,8 +185,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life Assert.isInstanceOf(ServletServerHttpResponse.class, response); HttpServletResponse servletResponse = ((ServletServerHttpResponse) response).getServletResponse(); - Assert.isTrue(this.factoryAdapter.getFactory() - .isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake"); + Assert.isTrue(this.factory.isUpgradeRequest(servletRequest, servletResponse), "Not a WebSocket handshake"); JettyWebSocketSession session = new JettyWebSocketSession(attributes, user); JettyWebSocketHandlerAdapter handlerAdapter = new JettyWebSocketHandlerAdapter(wsHandler, session); @@ -179,7 +195,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life try { wsContainerHolder.set(container); - this.factoryAdapter.getFactory().acceptWebSocket(servletRequest, servletResponse); + this.factory.acceptWebSocket(servletRequest, servletResponse); } catch (IOException ex) { throw new HandshakeFailureException( @@ -203,7 +219,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Life this.handler = handler; this.selectedProtocol = protocol; if (CollectionUtils.isEmpty(extensions)) { - this.extensionConfigs = new ArrayList(); + this.extensionConfigs = null; } else { this.extensionConfigs = new ArrayList(); @@ -226,74 +242,4 @@ 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(); - } - } - }