Revert "Update websocket support for Jetty 9.3+"

This reverts commit 305cea2.

Author: bclozel

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"

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 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 @@
  *
  * @author Phillip Webb
  * @author Rossen Stoyanchev
- * @author Brian Clozel
  * @since 4.0
  */
 public class JettyWebSocketSession extends AbstractWebSocketSession<Session> {
 
-	// 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<Session> {
 
 	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 boolean isOpen() {
 	@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<String, List<String>>) ReflectionUtils.invokeMethod(getHeaders, request));
-		this.headers = HttpHeaders.readOnlyHttpHeaders(headers);
-
-		this.acceptedProtocol = (String) ReflectionUtils.invokeMethod(getAcceptedSubProtocol, response);
-
-		List<ExtensionConfig> source = (List<ExtensionConfig>) ReflectionUtils.invokeMethod(getExtensions, response);
-		if (source != null) {
-			this.extensions = new ArrayList<WebSocketExtension>(source.size());
-			for (ExtensionConfig ec : source) {
-				this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters()));
-			}
-		}
-		else {
-			this.extensions = new ArrayList<WebSocketExtension>(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<ExtensionConfig> source = getNativeSession().getUpgradeResponse().getExtensions();
-		if (source != null) {
-			this.extensions = new ArrayList<WebSocketExtension>(source.size());
-			for (ExtensionConfig ec : source) {
-				this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters()));
-			}
-		}
-		else {
-				this.extensions = new ArrayList<WebSocketExtension>(0);
+		this.extensions = new ArrayList<WebSocketExtension>(source.size());
+		for (ExtensionConfig ec : source) {
+			this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters()));
 		}
 
 		if (this.user == null) {

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.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.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<WebSocketHandlerContainer> wsContainerHolder =
 			new NamedThreadLocal<WebSocketHandlerContainer>("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<WebSocketExtension> 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 List<WebSocketExtension> getSupportedExtensions(ServerHttpRequest request
 
 	private List<WebSocketExtension> getWebSocketExtensions() {
 		List<WebSocketExtension> result = new ArrayList<WebSocketExtension>();
-		for (String name : this.factoryAdapter.getFactory().getExtensionFactory().getExtensionNames()) {
+		for (String name : this.factory.getExtensionFactory().getExtensionNames()) {
 			result.add(new WebSocketExtension(name));
 		}
 		return result;
@@ -136,24 +153,24 @@ public void start() {
 		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);
 			}
 		}
 	}
 
 	@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 void upgrade(ServerHttpRequest request, ServerHttpResponse response,
 		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 void upgrade(ServerHttpRequest request, ServerHttpResponse response,
 
 		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 WebSocketHandlerContainer(JettyWebSocketHandlerAdapter handler, String pr
 			this.handler = handler;
 			this.selectedProtocol = protocol;
 			if (CollectionUtils.isEmpty(extensions)) {
-				this.extensionConfigs = new ArrayList<ExtensionConfig>();
+				this.extensionConfigs = null;
 			}
 			else {
 				this.extensionConfigs = new ArrayList<ExtensionConfig>();
@@ -226,74 +242,4 @@ public List<ExtensionConfig> getExtensionConfigs() {
 		}
 	}
 
-	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();
-		}
-	}
-
 }