Merge pull request #306 from glyptodon/stuck-websocket-tunnels

GUAC-1427: Delay creation of tunnel until upgrade handshake is complete

guacamole-common/src/main/java/org/glyptodon/guacamole/websocket/GuacamoleWebSocketTunnelEndpoint.java

@@ -207,6 +207,10 @@ public abstract class GuacamoleWebSocketTunnelEndpoint extends Endpoint {
     @OnMessage
     public void onMessage(String message) {
 
+        // Ignore inbound messages if there is no associated tunnel
+        if (tunnel == null)
+            return;
+
         GuacamoleWriter writer = tunnel.acquireWriter();
 
         try {

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/HTTPTunnelRequest.java

@@ -22,8 +22,11 @@
 
 package org.glyptodon.guacamole.net.basic;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 
 /**
@@ -34,33 +37,54 @@ import javax.servlet.http.HttpServletRequest;
 public class HTTPTunnelRequest extends TunnelRequest {
 
     /**
-     * The wrapped HttpServletRequest.
+     * A copy of the parameters obtained from the HttpServletRequest used to
+     * construct the HTTPTunnelRequest.
      */
-    private final HttpServletRequest request;
+    private final Map<String, List<String>> parameterMap =
+            new HashMap<String, List<String>>();
 
     /**
-     * Creates a TunnelRequest implementation which delegates parameter and
+     * Creates a HTTPTunnelRequest which copies and exposes the parameters
-     * session retrieval to the given HttpServletRequest.
+     * from the given HttpServletRequest.
      *
-     * @param request The HttpServletRequest to wrap.
+     * @param request
+     *     The HttpServletRequest to copy parameter values from.
      */
+    @SuppressWarnings("unchecked") // getParameterMap() is defined as returning Map<String, String[]>
     public HTTPTunnelRequest(HttpServletRequest request) {
-        this.request = request;
+
+        // For each parameter
+        for (Map.Entry<String, String[]> mapEntry : ((Map<String, String[]>)
+                request.getParameterMap()).entrySet()) {
+
+            // Get parameter name and corresponding values
+            String parameterName = mapEntry.getKey();
+            List<String> parameterValues = Arrays.asList(mapEntry.getValue());
+
+            // Store copy of all values in our own map
+            parameterMap.put(
+                parameterName,
+                new ArrayList<String>(parameterValues)
+            );
+
+        }
+
     }
 
     @Override
     public String getParameter(String name) {
-        return request.getParameter(name);
+        List<String> values = getParameterValues(name);
+
+        // Return the first value from the list if available
+        if (values != null && !values.isEmpty())
+            return values.get(0);
+
+        return null;
     }
 
     @Override
     public List<String> getParameterValues(String name) {
-
+        return parameterMap.get(name);
-        String[] values = request.getParameterValues(name);
-        if (values == null)
-            return null;
-        
-        return Arrays.asList(values);
     }
     
 }

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/BasicGuacamoleWebSocketTunnelEndpoint.java

@@ -31,6 +31,7 @@ import javax.websocket.server.HandshakeRequest;
 import javax.websocket.server.ServerEndpointConfig;
 import org.glyptodon.guacamole.GuacamoleException;
 import org.glyptodon.guacamole.net.GuacamoleTunnel;
+import org.glyptodon.guacamole.net.basic.TunnelRequest;
 import org.glyptodon.guacamole.net.basic.TunnelRequestService;
 import org.glyptodon.guacamole.websocket.GuacamoleWebSocketTunnelEndpoint;
 
@@ -41,16 +42,16 @@ import org.glyptodon.guacamole.websocket.GuacamoleWebSocketTunnelEndpoint;
 public class BasicGuacamoleWebSocketTunnelEndpoint extends GuacamoleWebSocketTunnelEndpoint {
 
     /**
-     * Unique string which shall be used to store the GuacamoleTunnel
+     * Unique string which shall be used to store the TunnelRequest
      * associated with a WebSocket connection.
      */
-    private static final String TUNNEL_USER_PROPERTY = "WS_GUAC_TUNNEL";
+    private static final String TUNNEL_REQUEST_PROPERTY = "WS_GUAC_TUNNEL_REQUEST";
 
     /**
-     * Unique string which shall be used to store any GuacamoleException that
+     * Unique string which shall be used to store the TunnelRequestService to
-     * occurs while retrieving the tunnel during the handshake.
+     * be used for processing TunnelRequests.
      */
-    private static final String ERROR_USER_PROPERTY = "WS_GUAC_TUNNEL_ERROR";
+    private static final String TUNNEL_REQUEST_SERVICE_PROPERTY = "WS_GUAC_TUNNEL_REQUEST_SERVICE";
 
     /**
      * Configurator implementation which stores the requested GuacamoleTunnel
@@ -70,52 +71,49 @@ public class BasicGuacamoleWebSocketTunnelEndpoint extends GuacamoleWebSocketTun
          * service provider to retrieve the necessary service to handle new
          * connections requests.
          * 
-         * @param tunnelRequestServiceProvider The tunnel request service
+         * @param tunnelRequestServiceProvider
-         *                                     provider to use for all new
+         *     The tunnel request service provider to use for all new
-         *                                     connections.
+         *     connections.
          */
         public Configurator(Provider<TunnelRequestService> tunnelRequestServiceProvider) {
             this.tunnelRequestServiceProvider = tunnelRequestServiceProvider;
         }
         
         @Override
-        public void modifyHandshake(ServerEndpointConfig config, HandshakeRequest request, HandshakeResponse response) {
+        public void modifyHandshake(ServerEndpointConfig config,
+                HandshakeRequest request, HandshakeResponse response) {
 
             super.modifyHandshake(config, request, response);
             
-            // Attempt tunnel creation
+            // Store tunnel request and tunnel request service for retrieval
+            // upon WebSocket open
             Map<String, Object> userProperties = config.getUserProperties();
             userProperties.clear();
-            try {
+            userProperties.put(TUNNEL_REQUEST_PROPERTY, new WebSocketTunnelRequest(request));
-
+            userProperties.put(TUNNEL_REQUEST_SERVICE_PROPERTY, tunnelRequestServiceProvider.get());
-                // Get tunnel request service
-                TunnelRequestService tunnelRequestService = tunnelRequestServiceProvider.get();
-                
-                // Store new tunnel within user properties
-                GuacamoleTunnel tunnel = tunnelRequestService.createTunnel(new WebSocketTunnelRequest(request));
-                if (tunnel != null)
-                    userProperties.put(TUNNEL_USER_PROPERTY, tunnel);
-
-            }
-            catch (GuacamoleException e) {
-                userProperties.put(ERROR_USER_PROPERTY, e);
-            }
 
         }
         
     }
     
     @Override
-    protected GuacamoleTunnel createTunnel(Session session, EndpointConfig config) throws GuacamoleException {
+    protected GuacamoleTunnel createTunnel(Session session,
+            EndpointConfig config) throws GuacamoleException {
 
-        // Throw any error that occurred during tunnel creation
         Map<String, Object> userProperties = config.getUserProperties();
-        GuacamoleException tunnelError = (GuacamoleException) userProperties.get(ERROR_USER_PROPERTY);
-        if (tunnelError != null)
-            throw tunnelError;
 
-        // Return created tunnel, if any
+        // Get original tunnel request
-        return (GuacamoleTunnel) userProperties.get(TUNNEL_USER_PROPERTY);
+        TunnelRequest tunnelRequest = (TunnelRequest) userProperties.get(TUNNEL_REQUEST_PROPERTY);
+        if (tunnelRequest == null)
+            return null;
+
+        // Get tunnel request service
+        TunnelRequestService tunnelRequestService = (TunnelRequestService) userProperties.get(TUNNEL_REQUEST_SERVICE_PROPERTY);
+        if (tunnelRequestService == null)
+            return null;
+
+        // Create and return tunnel
+        return tunnelRequestService.createTunnel(tunnelRequest);
 
     }
 

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty8/BasicGuacamoleWebSocketTunnelServlet.java

@@ -24,11 +24,10 @@ package org.glyptodon.guacamole.net.basic.websocket.jetty8;
 
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
-import javax.servlet.http.HttpServletRequest;
 import org.glyptodon.guacamole.GuacamoleException;
 import org.glyptodon.guacamole.net.GuacamoleTunnel;
 import org.glyptodon.guacamole.net.basic.TunnelRequestService;
-import org.glyptodon.guacamole.net.basic.HTTPTunnelRequest;
+import org.glyptodon.guacamole.net.basic.TunnelRequest;
 
 /**
  * Tunnel servlet implementation which uses WebSocket as a tunnel backend,
@@ -45,9 +44,9 @@ public class BasicGuacamoleWebSocketTunnelServlet extends GuacamoleWebSocketTunn
     private TunnelRequestService tunnelRequestService;
  
     @Override
-    protected GuacamoleTunnel doConnect(HttpServletRequest request)
+    protected GuacamoleTunnel doConnect(TunnelRequest request)
             throws GuacamoleException {
-        return tunnelRequestService.createTunnel(new HTTPTunnelRequest(request));
+        return tunnelRequestService.createTunnel(request);
     }
 
 }

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty8/GuacamoleWebSocketTunnelServlet.java

@@ -33,6 +33,8 @@ import org.eclipse.jetty.websocket.WebSocket.Connection;
 import org.eclipse.jetty.websocket.WebSocketServlet;
 import org.glyptodon.guacamole.GuacamoleClientException;
 import org.glyptodon.guacamole.GuacamoleConnectionClosedException;
+import org.glyptodon.guacamole.net.basic.HTTPTunnelRequest;
+import org.glyptodon.guacamole.net.basic.TunnelRequest;
 import org.glyptodon.guacamole.protocol.GuacamoleStatus;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -72,23 +74,24 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet {
     @Override
     public WebSocket doWebSocketConnect(HttpServletRequest request, String protocol) {
 
-        // Get tunnel
+        final TunnelRequest tunnelRequest = new HTTPTunnelRequest(request);
-        final GuacamoleTunnel tunnel;
-
-        try {
-            tunnel = doConnect(request);
-        }
-        catch (GuacamoleException e) {
-            logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage());
-            logger.debug("Error connecting WebSocket tunnel.", e);
-            return null;
-        }
 
         // Return new WebSocket which communicates through tunnel
         return new WebSocket.OnTextMessage() {
 
+            /**
+             * The GuacamoleTunnel associated with the connected WebSocket. If
+             * the WebSocket has not yet been connected, this will be null.
+             */
+            private GuacamoleTunnel tunnel = null;
+
             @Override
             public void onMessage(String string) {
+
+                // Ignore inbound messages if there is no associated tunnel
+                if (tunnel == null)
+                    return;
+
                 GuacamoleWriter writer = tunnel.acquireWriter();
 
                 // Write message received
@@ -103,11 +106,22 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet {
                 }
 
                 tunnel.releaseWriter();
+
             }
 
             @Override
             public void onOpen(final Connection connection) {
 
+                try {
+                    tunnel = doConnect(tunnelRequest);
+                }
+                catch (GuacamoleException e) {
+                    logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage());
+                    logger.debug("Error connecting WebSocket tunnel.", e);
+                    closeConnection(connection, e.getStatus());
+                    return;
+                }
+
                 // Do not start connection if tunnel does not exist
                 if (tunnel == null) {
                     closeConnection(connection, GuacamoleStatus.RESOURCE_NOT_FOUND);
@@ -199,16 +213,19 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet {
      * result of this connection request (whether some sort of credentials must
      * be specified, for example).
      *
-     * @param request The HttpServletRequest associated with the connection
+     * @param request
-     *                request received. Any parameters specified along with
+     *     The TunnelRequest associated with the connection request received.
-     *                the connection request can be read from this object.
+     *     Any parameters specified along with the connection request can be
-     * @return A newly constructed GuacamoleTunnel if successful,
+     *     read from this object.
-     *         null otherwise.
+     *
-     * @throws GuacamoleException If an error occurs while constructing the
+     * @return
-     *                            GuacamoleTunnel, or if the conditions
+     *     A newly constructed GuacamoleTunnel if successful, null otherwise.
-     *                            required for connection are not met.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs while constructing the GuacamoleTunnel, or if the
+     *     conditions required for connection are not met.
      */
-    protected abstract GuacamoleTunnel doConnect(HttpServletRequest request)
+    protected abstract GuacamoleTunnel doConnect(TunnelRequest request)
             throws GuacamoleException;
 
 }

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty9/GuacamoleWebSocketTunnelListener.java

@@ -185,6 +185,10 @@ public abstract class GuacamoleWebSocketTunnelListener implements WebSocketListe
     @Override
     public void onWebSocketText(String message) {
 
+        // Ignore inbound messages if there is no associated tunnel
+        if (tunnel == null)
+            return;
+
         GuacamoleWriter writer = tunnel.acquireWriter();
 
         try {

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/tomcat/BasicGuacamoleWebSocketTunnelServlet.java

@@ -24,11 +24,10 @@ package org.glyptodon.guacamole.net.basic.websocket.tomcat;
 
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
-import javax.servlet.http.HttpServletRequest;
 import org.glyptodon.guacamole.GuacamoleException;
 import org.glyptodon.guacamole.net.GuacamoleTunnel;
 import org.glyptodon.guacamole.net.basic.TunnelRequestService;
-import org.glyptodon.guacamole.net.basic.HTTPTunnelRequest;
+import org.glyptodon.guacamole.net.basic.TunnelRequest;
 
 /**
  * Tunnel servlet implementation which uses WebSocket as a tunnel backend,
@@ -45,9 +44,9 @@ public class BasicGuacamoleWebSocketTunnelServlet extends GuacamoleWebSocketTunn
     private TunnelRequestService tunnelRequestService;
  
     @Override
-    protected GuacamoleTunnel doConnect(HttpServletRequest request)
+    protected GuacamoleTunnel doConnect(TunnelRequest request)
             throws GuacamoleException {
-        return tunnelRequestService.createTunnel(new HTTPTunnelRequest(request));
+        return tunnelRequestService.createTunnel(request);
     };
 
 }

guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java

@@ -38,6 +38,8 @@ import org.apache.catalina.websocket.WebSocketServlet;
 import org.apache.catalina.websocket.WsOutbound;
 import org.glyptodon.guacamole.GuacamoleClientException;
 import org.glyptodon.guacamole.GuacamoleConnectionClosedException;
+import org.glyptodon.guacamole.net.basic.HTTPTunnelRequest;
+import org.glyptodon.guacamole.net.basic.TunnelRequest;
 import org.glyptodon.guacamole.protocol.GuacamoleStatus;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -92,26 +94,27 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet {
     }
 
     @Override
-    public StreamInbound createWebSocketInbound(String protocol, HttpServletRequest request) {
+    public StreamInbound createWebSocketInbound(String protocol,
+            HttpServletRequest request) {
 
-        // Get tunnel
+        final TunnelRequest tunnelRequest = new HTTPTunnelRequest(request);
-        final GuacamoleTunnel tunnel;
-
-        try {
-            tunnel = doConnect(request);
-        }
-        catch (GuacamoleException e) {
-            logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage());
-            logger.debug("Error connecting WebSocket tunnel.", e);
-            return null;
-        }
 
         // Return new WebSocket which communicates through tunnel
         return new StreamInbound() {
 
+            /**
+             * The GuacamoleTunnel associated with the connected WebSocket. If
+             * the WebSocket has not yet been connected, this will be null.
+             */
+            private GuacamoleTunnel tunnel = null;
+
             @Override
             protected void onTextData(Reader reader) throws IOException {
 
+                // Ignore inbound messages if there is no associated tunnel
+                if (tunnel == null)
+                    return;
+
                 GuacamoleWriter writer = tunnel.acquireWriter();
 
                 // Write all available data
@@ -137,6 +140,16 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet {
             @Override
             public void onOpen(final WsOutbound outbound) {
 
+                try {
+                    tunnel = doConnect(tunnelRequest);
+                }
+                catch (GuacamoleException e) {
+                    logger.error("Creation of WebSocket tunnel to guacd failed: {}", e.getMessage());
+                    logger.debug("Error connecting WebSocket tunnel.", e);
+                    closeConnection(outbound, e.getStatus());
+                    return;
+                }
+
                 // Do not start connection if tunnel does not exist
                 if (tunnel == null) {
                     closeConnection(outbound, GuacamoleStatus.RESOURCE_NOT_FOUND);
@@ -233,16 +246,20 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet {
      * result of this connection request (whether some sort of credentials must
      * be specified, for example).
      *
-     * @param request The HttpServletRequest associated with the connection
+     * @param request
-     *                request received. Any parameters specified along with
+     *     The TunnelRequest associated with the connection request received.
-     *                the connection request can be read from this object.
+     *     Any parameters specified along with the connection request can be
-     * @return A newly constructed GuacamoleTunnel if successful,
+     *     read from this object.
-     *         null otherwise.
+     *
-     * @throws GuacamoleException If an error occurs while constructing the
+     * @return
-     *                            GuacamoleTunnel, or if the conditions
+     *     A newly constructed GuacamoleTunnel if successful, null otherwise.
-     *                            required for connection are not met.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs while constructing the GuacamoleTunnel, or if the
+     *     conditions required for connection are not met.
      */
-    protected abstract GuacamoleTunnel doConnect(HttpServletRequest request) throws GuacamoleException;
+    protected abstract GuacamoleTunnel doConnect(TunnelRequest request)
+            throws GuacamoleException;
 
 }