GUACAMOLE-267: Narrow concerns of FailoverGuacamoleSocket to a single connection. Throw exceptions directly from constructor if upstream errors are encountered.

guacamole-common/src/main/java/org/apache/guacamole/protocol/FailoverGuacamoleSocket.java

@@ -19,10 +19,14 @@
 
 package org.apache.guacamole.protocol;
 
+import java.util.LinkedList;
 import java.util.List;
-import org.apache.guacamole.GuacamoleConnectionClosedException;
+import java.util.Queue;
 import org.apache.guacamole.GuacamoleException;
-import org.apache.guacamole.GuacamoleServerException;
+import org.apache.guacamole.GuacamoleUpstreamException;
+import org.apache.guacamole.GuacamoleUpstreamNotFoundException;
+import org.apache.guacamole.GuacamoleUpstreamTimeoutException;
+import org.apache.guacamole.GuacamoleUpstreamUnavailableException;
 import org.apache.guacamole.io.GuacamoleReader;
 import org.apache.guacamole.io.GuacamoleWriter;
 import org.apache.guacamole.net.GuacamoleSocket;
@@ -30,318 +34,220 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * GuacamoleSocket implementation which transparently switches between an
+ * GuacamoleSocket which intercepts errors received early in the Guacamole
- * underlying queue of available sockets when errors are encountered during
+ * session. Upstream errors which are intercepted early enough result in
- * the initial part of a connection (prior to first "sync").
+ * exceptions thrown immediately within the FailoverGuacamoleSocket's
+ * constructor, allowing a different socket to be substituted prior to
+ * fulfilling the connection.
  */
 public class FailoverGuacamoleSocket implements GuacamoleSocket {
 
     /**
      * Logger for this class.
      */
-    private Logger logger = LoggerFactory.getLogger(FailoverGuacamoleSocket.class);
+    private static final Logger logger =
+            LoggerFactory.getLogger(FailoverGuacamoleSocket.class);
 
     /**
-     * A queue of available sockets. The full set of available sockets need not
+     * The maximum number of characters of Guacamole instruction data to store
-     * be known ahead of time.
+     * within the instruction queue while searching for errors. Once this limit
+     * is exceeded, the connection is assumed to be successful.
      */
-    public static interface SocketQueue {
+    private static final int INSTRUCTION_QUEUE_LIMIT = 2048;
 
     /**
-         * Returns the next available socket in the queue, or null if no
+     * The wrapped socket being used.
-         * further sockets are available. This function will be invoked when an
+     */
-         * error occurs within the current socket, and will be invoked again if
+    private final GuacamoleSocket socket;
-         * a GuacamoleException is thrown. Such repeated calls to this function
+
-         * will occur until errors cease or null is returned.
+    /**
+     * Queue of all instructions read while this FailoverGuacamoleSocket was
+     * being constructed.
+     */
+    private final Queue<GuacamoleInstruction> instructionQueue =
+            new LinkedList<GuacamoleInstruction>();
+
+    /**
+     * Parses the given "error" instruction, throwing an exception if the
+     * instruction represents an error from the upstream remote desktop.
      *
-         * @return
+     * @param instruction
-         *     The next available socket in the queue, or null if no further
+     *     The "error" instruction to parse.
-         *     sockets are available.
      *
-         * @throws GuacamoleException
+     * @throws GuacamoleUpstreamException
-         *     If an error occurs preventing the next available socket from
+     *     If the "error" instruction represents an error from the upstream
-         *     being used.
+     *     remote desktop.
      */
-        GuacamoleSocket nextSocket() throws GuacamoleException;
+    private static void handleUpstreamErrors(GuacamoleInstruction instruction)
-
+            throws GuacamoleUpstreamException {
-    }
-
-    /**
-     * The queue of available sockets provided when this FailoverGuacamoleSocket
-     * was created.
-     */
-    private final SocketQueue sockets;
-
-    /**
-     * The current socket being used, or null if no socket is available.
-     */
-    private GuacamoleSocket socket;
-
-    /**
-     * Creates a new FailoverGuacamoleSocket which pulls its sockets from the
-     * given SocketQueue. If an error occurs during the Guacamole connection,
-     * other sockets from the SocketQueue are tried, in order, until no error
-     * occurs.
-     *
-     * @param sockets
-     *     A SocketQueue which returns the sockets which should be used, in
-     *     order.
-     *
-     * @throws GuacamoleException
-     *     If errors prevent use of the sockets defined by the SocketQueue, and
-     *     no further sockets remain to be tried.
-     */
-    public FailoverGuacamoleSocket(SocketQueue sockets)
-            throws GuacamoleException {
-        this.sockets = sockets;
-        selectNextSocket();
-    }
-
-    private GuacamoleException tryNextSocket() {
-
-        try {
-            if (socket != null)
-                socket.close();
-        }
-        catch (GuacamoleException e) {
-            if (socket.isOpen())
-                logger.debug("Previous failed socket could not be closed.", e);
-        }
-
-        try {
-
-            socket = sockets.nextSocket();
-            if (socket == null)
-                return new GuacamoleServerException("No remaining sockets to try.");
-
-        }
-
-        catch (GuacamoleException e) {
-            if (tryNextSocket() != null)
-                return e;
-        }
-
-        return null;
-
-    }
-
-    private void selectNextSocket() throws GuacamoleException {
-        synchronized (sockets) {
-            GuacamoleException failure = tryNextSocket();
-            if (failure != null)
-                throw failure;
-        }
-    }
-
-    private class ErrorFilter implements GuacamoleFilter {
-
-        /**
-         * Whether a "sync" instruction has been read.
-         */
-        private boolean receivedSync = false;
-
-        @Override
-        public GuacamoleInstruction filter(GuacamoleInstruction instruction)
-                throws GuacamoleException {
-
-            // Ignore instructions after first sync is received
-            if (receivedSync)
-                return instruction;
-
-            String opcode = instruction.getOpcode();
-
-            if (opcode.equals("sync")) {
-                receivedSync = true;
-                return instruction;
-            }
-
-            if (opcode.equals("error"))
-                return handleError(instruction);
-
-            return instruction;
-
-        }
-
-        private GuacamoleInstruction handleError(GuacamoleInstruction instruction) {
 
         // Ignore error instructions which are missing the status code
         List<String> args = instruction.getArgs();
         if (args.size() < 2) {
-                logger.debug("Ignoring \"error\" instruction without status code.");
+            logger.debug("Received \"error\" instruction without status code.");
-                return instruction;
+            return;
         }
 
+        // Parse the status code from the received error instruction
         int statusCode;
         try {
             statusCode = Integer.parseInt(args.get(1));
         }
         catch (NumberFormatException e) {
-                logger.debug("Ignoring \"error\" instruction with non-numeric status code.", e);
+            logger.debug("Received \"error\" instruction with non-numeric status code.", e);
-                return instruction;
+            return;
         }
 
-            // Invalid status code
+        // Translate numeric status code into a GuacamoleStatus
         GuacamoleStatus status = GuacamoleStatus.fromGuacamoleStatusCode(statusCode);
         if (status == null) {
-                logger.debug("Ignoring \"error\" instruction with unknown/invalid status code: {}", statusCode);
+            logger.debug("Received \"error\" instruction with unknown/invalid status code: {}", statusCode);
-                return instruction;
+            return;
         }
 
         // Only handle error instructions related to the upstream remote desktop
         switch (status) {
 
-                // Transparently connect to a different connection if upstream fails
+            // Generic upstream error
             case UPSTREAM_ERROR:
+                throw new GuacamoleUpstreamException(args.get(0));
+
+            // Upstream is unreachable
             case UPSTREAM_NOT_FOUND:
+                throw new GuacamoleUpstreamNotFoundException(args.get(0));
+
+            // Upstream did not respond
             case UPSTREAM_TIMEOUT:
+                throw new GuacamoleUpstreamTimeoutException(args.get(0));
+
+            // Upstream is refusing the connection
             case UPSTREAM_UNAVAILABLE:
-                    break;
+                throw new GuacamoleUpstreamUnavailableException(args.get(0));
-
-                // Allow error through otherwise
-                default:
-                    return instruction;
-
-            }
-
-            logger.debug("Overriding {} \"error\" instruction. Failing over to next connection...", status);
-
-            // Advance through remaining sockets until another functional socket
-            // is retrieved, or no more sockets remain
-            try {
-                selectNextSocket();
-                return null;
-            }
-
-            catch (GuacamoleException e) {
-                logger.debug("No sockets remain to be tried - giving up on failover.");
-            }
-
-            // Allow error through if not intercepting
-            return instruction;
 
         }
 
     }
 
     /**
-     * GuacamoleReader which filters "error" instructions, transparently failing
+     * Creates a new FailoverGuacamoleSocket which reads Guacamole instructions
-     * over to the next socket in the queue if an error is encountered. Read
+     * from the given socket, searching for errors from the upstream remote
-     * attempts are delegated to the GuacamoleReader of the current socket.
+     * desktop. If an upstream error is encountered, it is thrown as a
+     * GuacamoleUpstreamException. This constructor will block until an error
+     * is encountered, or until the connection appears to have been successful.
+     * Once the FailoverGuacamoleSocket has been created, all reads, writes,
+     * etc. will be delegated to the provided socket.
+     *
+     * @param socket
+     *     The GuacamoleSocket of the Guacamole connection this
+     *     FailoverGuacamoleSocket should handle.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs while reading data from the provided socket.
+     *
+     * @throws GuacamoleUpstreamException
+     *     If the connection to guacd succeeded, but an error occurred while
+     *     connecting to the remote desktop.
      */
-    private final GuacamoleReader reader = new FilteredGuacamoleReader(new GuacamoleReader() {
+    public FailoverGuacamoleSocket(GuacamoleSocket socket)
+            throws GuacamoleException, GuacamoleUpstreamException {
+
+        int totalQueueSize = 0;
+
+        GuacamoleInstruction instruction;
+        GuacamoleReader reader = socket.getReader();
+
+        // Continuously read instructions, searching for errors
+        while ((instruction = reader.readInstruction()) != null) {
+
+            // Add instruction to tail of instruction queue
+            instructionQueue.add(instruction);
+
+            // If instruction is a "sync" instruction, stop reading
+            String opcode = instruction.getOpcode();
+            if (opcode.equals("sync"))
+                break;
+
+            // If instruction is an "error" instruction, parse its contents and
+            // stop reading
+            if (opcode.equals("error")) {
+                handleUpstreamErrors(instruction);
+                break;
+            }
+
+            // Otherwise, track total data parsed, and assume connection is
+            // successful if no error encountered within reasonable space
+            totalQueueSize += instruction.toString().length();
+            if (totalQueueSize >= INSTRUCTION_QUEUE_LIMIT)
+                break;
+
+        }
+
+        this.socket = socket;
+
+    }
+
+    /**
+     * GuacamoleReader which reads instructions from the queue populated when
+     * the FailoverGuacamoleSocket was constructed. Once the queue has been
+     * emptied, reads are delegated directly to the reader of the wrapped
+     * socket.
+     */
+    private final GuacamoleReader queuedReader = new GuacamoleReader() {
 
         @Override
         public boolean available() throws GuacamoleException {
-            synchronized (sockets) {
+            return !instructionQueue.isEmpty() || socket.getReader().available();
-
-                if (socket == null)
-                    return false;
-
-                return socket.getReader().available();
-
-            }
         }
 
         @Override
         public char[] read() throws GuacamoleException {
-            synchronized (sockets) {
 
-                if (socket == null)
+            // Read instructions from queue before finally delegating to
-                    return null;
+            // underlying reader (received when FailoverGuacamoleSocket was
+            // being constructed)
+            if (!instructionQueue.isEmpty()) {
+                GuacamoleInstruction instruction = instructionQueue.remove();
+                return instruction.toString().toCharArray();
+            }
 
             return socket.getReader().read();
 
         }
-        }
 
         @Override
         public GuacamoleInstruction readInstruction()
                 throws GuacamoleException {
-            synchronized (sockets) {
 
-                if (socket == null)
+            // Read instructions from queue before finally delegating to
-                    return null;
+            // underlying reader (received when FailoverGuacamoleSocket was
+            // being constructed)
+            if (!instructionQueue.isEmpty())
+                return instructionQueue.remove();
 
             return socket.getReader().readInstruction();
 
         }
-        }
-
-    }, new ErrorFilter());
-
-    /**
-     * GuacamoleWriter which delegates all write attempts to the GuacamoleWriter
-     * of the current socket.
-     */
-    private final GuacamoleWriter writer = new GuacamoleWriter() {
-
-        @Override
-        public void write(char[] chunk, int off, int len)
-                throws GuacamoleException {
-            synchronized (sockets) {
-
-                if (socket == null)
-                    throw new GuacamoleConnectionClosedException("No further sockets remaining in SocketQueue.");
-
-                socket.getWriter().write(chunk, off, len);
-
-            }
-        }
-
-        @Override
-        public void write(char[] chunk) throws GuacamoleException {
-            synchronized (sockets) {
-
-                if (socket == null)
-                    throw new GuacamoleConnectionClosedException("No further sockets remaining in SocketQueue.");
-
-                socket.getWriter().write(chunk);
-
-            }
-        }
-
-        @Override
-        public void writeInstruction(GuacamoleInstruction instruction)
-                throws GuacamoleException {
-            synchronized (sockets) {
-
-                if (socket == null)
-                    throw new GuacamoleConnectionClosedException("No further sockets remaining in SocketQueue.");
-
-                socket.getWriter().writeInstruction(instruction);
-
-            }
-        }
 
     };
 
     @Override
     public GuacamoleReader getReader() {
-        return reader;
+        return queuedReader;
     }
 
     @Override
     public GuacamoleWriter getWriter() {
-        return writer;
+        return socket.getWriter();
     }
 
     @Override
     public void close() throws GuacamoleException {
-        synchronized (sockets) {
         socket.close();
     }
-    }
 
     @Override
     public boolean isOpen() {
-        synchronized (sockets) {
-
-            if (socket == null)
-                return false;
-
         return socket.isOpen();
-
-        }
     }
     
 }