GUACAMOLE-267: Narrow concerns of FailoverGuacamoleSocket to a single connection. Throw exceptions directly from constructor if upstream errors are encountered.
Project: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/commit/ad3fcb59 Tree: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/tree/ad3fcb59 Diff: http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/diff/ad3fcb59 Branch: refs/heads/master Commit: ad3fcb59ca1f51de630ff864f8624e916b75efd9 Parents: 3f38880 Author: Michael Jumper <[email protected]> Authored: Sun Apr 16 21:02:59 2017 -0700 Committer: Michael Jumper <[email protected]> Committed: Thu Apr 20 22:37:08 2017 -0700 ---------------------------------------------------------------------- .../protocol/FailoverGuacamoleSocket.java | 362 +++++++------------ 1 file changed, 134 insertions(+), 228 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-guacamole-client/blob/ad3fcb59/guacamole-common/src/main/java/org/apache/guacamole/protocol/FailoverGuacamoleSocket.java ---------------------------------------------------------------------- diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/FailoverGuacamoleSocket.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/FailoverGuacamoleSocket.java index fcef9a5..627c9fb 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/FailoverGuacamoleSocket.java +++ b/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 - * underlying queue of available sockets when errors are encountered during - * the initial part of a connection (prior to first "sync"). + * GuacamoleSocket which intercepts errors received early in the Guacamole + * session. Upstream errors which are intercepted early enough result in + * 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 - * be known ahead of time. + * The maximum number of characters of Guacamole instruction data to store + * within the instruction queue while searching for errors. Once this limit + * is exceeded, the connection is assumed to be successful. */ - public static interface SocketQueue { - - /** - * Returns the next available socket in the queue, or null if no - * further sockets are available. This function will be invoked when an - * error occurs within the current socket, and will be invoked again if - * a GuacamoleException is thrown. Such repeated calls to this function - * will occur until errors cease or null is returned. - * - * @return - * The next available socket in the queue, or null if no further - * sockets are available. - * - * @throws GuacamoleException - * If an error occurs preventing the next available socket from - * being used. - */ - GuacamoleSocket nextSocket() throws GuacamoleException; - - } + private static final int INSTRUCTION_QUEUE_LIMIT = 2048; /** - * The queue of available sockets provided when this FailoverGuacamoleSocket - * was created. + * The wrapped socket being used. */ - private final SocketQueue sockets; + private final GuacamoleSocket socket; /** - * The current socket being used, or null if no socket is available. + * Queue of all instructions read while this FailoverGuacamoleSocket was + * being constructed. */ - private GuacamoleSocket socket; + private final Queue<GuacamoleInstruction> instructionQueue = + new LinkedList<GuacamoleInstruction>(); /** - * 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. + * Parses the given "error" instruction, throwing an exception if the + * instruction represents an error from the upstream remote desktop. * - * @param sockets - * A SocketQueue which returns the sockets which should be used, in - * order. + * @param instruction + * The "error" instruction to parse. * - * @throws GuacamoleException - * If errors prevent use of the sockets defined by the SocketQueue, and - * no further sockets remain to be tried. + * @throws GuacamoleUpstreamException + * If the "error" instruction represents an error from the upstream + * remote desktop. */ - 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); + private static void handleUpstreamErrors(GuacamoleInstruction instruction) + throws GuacamoleUpstreamException { + + // Ignore error instructions which are missing the status code + List<String> args = instruction.getArgs(); + if (args.size() < 2) { + logger.debug("Received \"error\" instruction without status code."); + return; } + // Parse the status code from the received error instruction + int statusCode; try { - - socket = sockets.nextSocket(); - if (socket == null) - return new GuacamoleServerException("No remaining sockets to try."); - + statusCode = Integer.parseInt(args.get(1)); } - - catch (GuacamoleException e) { - if (tryNextSocket() != null) - return e; + catch (NumberFormatException e) { + logger.debug("Received \"error\" instruction with non-numeric status code.", e); + return; } - return null; - - } - - private void selectNextSocket() throws GuacamoleException { - synchronized (sockets) { - GuacamoleException failure = tryNextSocket(); - if (failure != null) - throw failure; + // Translate numeric status code into a GuacamoleStatus + GuacamoleStatus status = GuacamoleStatus.fromGuacamoleStatusCode(statusCode); + if (status == null) { + logger.debug("Received \"error\" instruction with unknown/invalid status code: {}", statusCode); + return; } - } - - 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; + // Only handle error instructions related to the upstream remote desktop + switch (status) { - String opcode = instruction.getOpcode(); + // Generic upstream error + case UPSTREAM_ERROR: + throw new GuacamoleUpstreamException(args.get(0)); - if (opcode.equals("sync")) { - receivedSync = true; - return instruction; - } + // Upstream is unreachable + case UPSTREAM_NOT_FOUND: + throw new GuacamoleUpstreamNotFoundException(args.get(0)); - if (opcode.equals("error")) - return handleError(instruction); + // Upstream did not respond + case UPSTREAM_TIMEOUT: + throw new GuacamoleUpstreamTimeoutException(args.get(0)); - return instruction; + // Upstream is refusing the connection + case UPSTREAM_UNAVAILABLE: + throw new GuacamoleUpstreamUnavailableException(args.get(0)); } - 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."); - return instruction; - } - - int statusCode; - try { - statusCode = Integer.parseInt(args.get(1)); - } - catch (NumberFormatException e) { - logger.debug("Ignoring \"error\" instruction with non-numeric status code.", e); - return instruction; - } - - // Invalid status code - GuacamoleStatus status = GuacamoleStatus.fromGuacamoleStatusCode(statusCode); - if (status == null) { - logger.debug("Ignoring \"error\" instruction with unknown/invalid status code: {}", statusCode); - return instruction; - } - - // Only handle error instructions related to the upstream remote desktop - switch (status) { + } - // Transparently connect to a different connection if upstream fails - case UPSTREAM_ERROR: - case UPSTREAM_NOT_FOUND: - case UPSTREAM_TIMEOUT: - case UPSTREAM_UNAVAILABLE: - break; + /** + * Creates a new FailoverGuacamoleSocket which reads Guacamole instructions + * from the given socket, searching for errors from the upstream remote + * 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. + */ + public FailoverGuacamoleSocket(GuacamoleSocket socket) + throws GuacamoleException, GuacamoleUpstreamException { - // Allow error through otherwise - default: - return instruction; + int totalQueueSize = 0; - } + GuacamoleInstruction instruction; + GuacamoleReader reader = socket.getReader(); - logger.debug("Overriding {} \"error\" instruction. Failing over to next connection...", status); + // Continuously read instructions, searching for errors + while ((instruction = reader.readInstruction()) != null) { - // Advance through remaining sockets until another functional socket - // is retrieved, or no more sockets remain - try { - selectNextSocket(); - return null; - } + // Add instruction to tail of instruction queue + instructionQueue.add(instruction); - catch (GuacamoleException e) { - logger.debug("No sockets remain to be tried - giving up on failover."); + // 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; } - // Allow error through if not intercepting - return instruction; + // 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 filters "error" instructions, transparently failing - * over to the next socket in the queue if an error is encountered. Read - * attempts are delegated to the GuacamoleReader of the current 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 reader = new FilteredGuacamoleReader(new GuacamoleReader() { + private final GuacamoleReader queuedReader = new GuacamoleReader() { @Override public boolean available() throws GuacamoleException { - synchronized (sockets) { - - if (socket == null) - return false; - - return socket.getReader().available(); - - } + return !instructionQueue.isEmpty() || socket.getReader().available(); } @Override public char[] read() throws GuacamoleException { - synchronized (sockets) { - - if (socket == null) - return null; - - return socket.getReader().read(); - - } - } - - @Override - public GuacamoleInstruction readInstruction() - throws GuacamoleException { - synchronized (sockets) { - - if (socket == null) - return null; - - 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); + // Read instructions from queue before finally delegating to + // underlying reader (received when FailoverGuacamoleSocket was + // being constructed) + if (!instructionQueue.isEmpty()) { + GuacamoleInstruction instruction = instructionQueue.remove(); + return instruction.toString().toCharArray(); } - } - - @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); + return socket.getReader().read(); - } } @Override - public void writeInstruction(GuacamoleInstruction instruction) + public GuacamoleInstruction readInstruction() throws GuacamoleException { - synchronized (sockets) { - if (socket == null) - throw new GuacamoleConnectionClosedException("No further sockets remaining in SocketQueue."); + // Read instructions from queue before finally delegating to + // underlying reader (received when FailoverGuacamoleSocket was + // being constructed) + if (!instructionQueue.isEmpty()) + return instructionQueue.remove(); - socket.getWriter().writeInstruction(instruction); + return socket.getReader().readInstruction(); - } } }; @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(); - } + socket.close(); } @Override public boolean isOpen() { - synchronized (sockets) { - - if (socket == null) - return false; - - return socket.isOpen(); - - } + return socket.isOpen(); } }
