From 798f06ee0a4e3e72ba303f970d9dded7aa5e29a8 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 4 Apr 2017 12:28:05 -0700 Subject: [PATCH 1/5] GUACAMOLE-267: Implement FailoverGuacamoleSocket. --- .../protocol/FailoverGuacamoleSocket.java | 347 ++++++++++++++++++ 1 file changed, 347 insertions(+) create mode 100644 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 new file mode 100644 index 000000000..fcef9a59a --- /dev/null +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/FailoverGuacamoleSocket.java @@ -0,0 +1,347 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.guacamole.protocol; + +import java.util.List; +import org.apache.guacamole.GuacamoleConnectionClosedException; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.io.GuacamoleReader; +import org.apache.guacamole.io.GuacamoleWriter; +import org.apache.guacamole.net.GuacamoleSocket; +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"). + */ +public class FailoverGuacamoleSocket implements GuacamoleSocket { + + /** + * Logger for this class. + */ + private Logger logger = LoggerFactory.getLogger(FailoverGuacamoleSocket.class); + + /** + * A queue of available sockets. The full set of available sockets need not + * be known ahead of time. + */ + 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; + + } + + /** + * 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 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; + + // 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 + * over to the next socket in the queue if an error is encountered. Read + * attempts are delegated to the GuacamoleReader of the current socket. + */ + private final GuacamoleReader reader = new FilteredGuacamoleReader(new GuacamoleReader() { + + @Override + public boolean available() throws GuacamoleException { + synchronized (sockets) { + + if (socket == null) + return false; + + return 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); + + } + } + + @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; + } + + @Override + public GuacamoleWriter getWriter() { + return writer; + } + + @Override + public void close() throws GuacamoleException { + synchronized (sockets) { + socket.close(); + } + } + + @Override + public boolean isOpen() { + synchronized (sockets) { + + if (socket == null) + return false; + + return socket.isOpen(); + + } + } + +} From 3f38880a1205ea57ecb29e360f8fdf6a06f1e8dd Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 16 Apr 2017 21:02:00 -0700 Subject: [PATCH 2/5] GUACAMOLE-267: Avoid unnecessarily rebuilding the Guacamole protocol form of a GuacamoleInstruction. --- .../protocol/GuacamoleInstruction.java | 52 ++++++++++++------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java index 072c3f172..c3abd4610 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleInstruction.java @@ -33,12 +33,18 @@ public class GuacamoleInstruction { /** * The opcode of this instruction. */ - private String opcode; + private final String opcode; /** * All arguments of this instruction, in order. */ - private List args; + private final List args; + + /** + * The cached result of converting this GuacamoleInstruction to the format + * used by the Guacamole protocol. + */ + private String protocolForm = null; /** * Creates a new GuacamoleInstruction having the given Operation and @@ -91,31 +97,41 @@ public class GuacamoleInstruction { * Returns this GuacamoleInstruction in the form it would be sent over the * Guacamole protocol. * - * @return This GuacamoleInstruction in the form it would be sent over the - * Guacamole protocol. + * @return + * This GuacamoleInstruction in the form it would be sent over the + * Guacamole protocol. */ @Override public String toString() { - StringBuilder buff = new StringBuilder(); + // Avoid rebuilding Guacamole protocol form of instruction if already + // known + if (protocolForm == null) { - // Write opcode - buff.append(opcode.length()); - buff.append('.'); - buff.append(opcode); + StringBuilder buff = new StringBuilder(); - // Write argument values - for (String value : args) { - buff.append(','); - buff.append(value.length()); + // Write opcode + buff.append(opcode.length()); buff.append('.'); - buff.append(value); + buff.append(opcode); + + // Write argument values + for (String value : args) { + buff.append(','); + buff.append(value.length()); + buff.append('.'); + buff.append(value); + } + + // Write terminator + buff.append(';'); + + // Cache result for future calls + protocolForm = buff.toString(); + } - // Write terminator - buff.append(';'); - - return buff.toString(); + return protocolForm; } From ad3fcb59ca1f51de630ff864f8624e916b75efd9 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 16 Apr 2017 21:02:59 -0700 Subject: [PATCH 3/5] GUACAMOLE-267: Narrow concerns of FailoverGuacamoleSocket to a single connection. Throw exceptions directly from constructor if upstream errors are encountered. --- .../protocol/FailoverGuacamoleSocket.java | 394 +++++++----------- 1 file changed, 150 insertions(+), 244 deletions(-) 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 fcef9a59a..627c9fbad 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 { + private static final int INSTRUCTION_QUEUE_LIMIT = 2048; - /** - * 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; + /** + * The wrapped socket being used. + */ + private final GuacamoleSocket socket; + + /** + * Queue of all instructions read while this FailoverGuacamoleSocket was + * being constructed. + */ + private final Queue instructionQueue = + new LinkedList(); + + /** + * Parses the given "error" instruction, throwing an exception if the + * instruction represents an error from the upstream remote desktop. + * + * @param instruction + * The "error" instruction to parse. + * + * @throws GuacamoleUpstreamException + * If the "error" instruction represents an error from the upstream + * remote desktop. + */ + private static void handleUpstreamErrors(GuacamoleInstruction instruction) + throws GuacamoleUpstreamException { + + // Ignore error instructions which are missing the status code + List 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 { + statusCode = Integer.parseInt(args.get(1)); + } + catch (NumberFormatException e) { + logger.debug("Received \"error\" instruction with non-numeric status code.", e); + return; + } + + // 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; + } + + // Only handle error instructions related to the upstream remote desktop + switch (status) { + + // 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: + throw new GuacamoleUpstreamUnavailableException(args.get(0)); + + } } /** - * 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. + * 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 sockets - * A SocketQueue which returns the sockets which should be used, in - * order. + * @param socket + * The GuacamoleSocket of the Guacamole connection this + * FailoverGuacamoleSocket should handle. * * @throws GuacamoleException - * If errors prevent use of the sockets defined by the SocketQueue, and - * no further sockets remain to be tried. + * 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(SocketQueue sockets) - throws GuacamoleException { - this.sockets = sockets; - selectNextSocket(); - } + public FailoverGuacamoleSocket(GuacamoleSocket socket) + throws GuacamoleException, GuacamoleUpstreamException { - private GuacamoleException tryNextSocket() { + int totalQueueSize = 0; - try { - if (socket != null) - socket.close(); - } - catch (GuacamoleException e) { - if (socket.isOpen()) - logger.debug("Previous failed socket could not be closed.", e); - } + GuacamoleInstruction instruction; + GuacamoleReader reader = socket.getReader(); - try { + // Continuously read instructions, searching for errors + while ((instruction = reader.readInstruction()) != null) { - 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; + // 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 (opcode.equals("sync")) { - receivedSync = true; - return instruction; + // If instruction is an "error" instruction, parse its contents and + // stop reading + if (opcode.equals("error")) { + handleUpstreamErrors(instruction); + break; } - if (opcode.equals("error")) - return handleError(instruction); - - 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; } - private GuacamoleInstruction handleError(GuacamoleInstruction instruction) { - - // Ignore error instructions which are missing the status code - List 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; - - // 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; - - } + 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(); + // 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(); } + + return socket.getReader().read(); + } @Override public GuacamoleInstruction readInstruction() throws GuacamoleException { - synchronized (sockets) { - if (socket == null) - return null; + // Read instructions from queue before finally delegating to + // underlying reader (received when FailoverGuacamoleSocket was + // being constructed) + if (!instructionQueue.isEmpty()) + return instructionQueue.remove(); - return socket.getReader().readInstruction(); + 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(); - } + socket.close(); } @Override public boolean isOpen() { - synchronized (sockets) { - - if (socket == null) - return false; - - return socket.isOpen(); - - } + return socket.isOpen(); } } From 8b9b4881b71f60db8fd44c38a96b3c9fa6503909 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 16 Apr 2017 21:10:25 -0700 Subject: [PATCH 4/5] GUACAMOLE-267: Do not require ConfiguredGuacamoleSocket for all active connections. --- .../tunnel/AbstractGuacamoleTunnelService.java | 2 +- .../auth/jdbc/tunnel/ActiveConnectionRecord.java | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java index 2e2c947e4..1ddd7f9d5 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java @@ -488,7 +488,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS getUnconfiguredGuacamoleSocket(cleanupTask), config, info); // Assign and return new tunnel - return activeConnection.assignGuacamoleTunnel(socket); + return activeConnection.assignGuacamoleTunnel(socket, socket.getConnectionID()); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java index 1fe4143f2..3a4b1484d 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/ActiveConnectionRecord.java @@ -366,13 +366,17 @@ public class ActiveConnectionRecord implements ConnectionRecord { * given socket. * * @param socket - * The ConfiguredGuacamoleSocket to use to create the tunnel associated - * with this connection record. - * + * The GuacamoleSocket to use to create the tunnel associated with this + * connection record. + * + * @param connectionID + * The connection ID assigned to this connection by guacd. + * * @return * The newly-created tunnel associated with this connection record. */ - public GuacamoleTunnel assignGuacamoleTunnel(final ConfiguredGuacamoleSocket socket) { + public GuacamoleTunnel assignGuacamoleTunnel(final GuacamoleSocket socket, + String connectionID) { // Create tunnel with given socket this.tunnel = new AbstractGuacamoleTunnel() { @@ -391,7 +395,7 @@ public class ActiveConnectionRecord implements ConnectionRecord { // Store connection ID of the primary connection only if (isPrimaryConnection()) - this.connectionID = socket.getConnectionID(); + this.connectionID = connectionID; // Return newly-created tunnel return this.tunnel; From c9b88e2ba927fd6b92a788e67fd936107027919a Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 16 Apr 2017 21:41:56 -0700 Subject: [PATCH 5/5] GUACAMOLE-267: Failover to other connections within same group if upstream remote desktop errors are detected. --- .../AbstractGuacamoleTunnelService.java | 86 +++++++++++++------ 1 file changed, 62 insertions(+), 24 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java index 1ddd7f9d5..043029c2b 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/tunnel/AbstractGuacamoleTunnelService.java @@ -39,8 +39,10 @@ import org.apache.guacamole.auth.jdbc.connection.ConnectionModel; import org.apache.guacamole.auth.jdbc.connection.ConnectionRecordModel; import org.apache.guacamole.auth.jdbc.connection.ConnectionParameterModel; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleResourceConflictException; import org.apache.guacamole.GuacamoleResourceNotFoundException; import org.apache.guacamole.GuacamoleSecurityException; +import org.apache.guacamole.GuacamoleUpstreamException; import org.apache.guacamole.auth.jdbc.JDBCEnvironment; import org.apache.guacamole.auth.jdbc.connection.ConnectionMapper; import org.apache.guacamole.environment.Environment; @@ -60,6 +62,9 @@ import org.apache.guacamole.auth.jdbc.sharingprofile.ModeledSharingProfile; import org.apache.guacamole.auth.jdbc.sharingprofile.SharingProfileParameterMapper; import org.apache.guacamole.auth.jdbc.sharingprofile.SharingProfileParameterModel; import org.apache.guacamole.auth.jdbc.user.RemoteAuthenticatedUser; +import org.apache.guacamole.protocol.FailoverGuacamoleSocket; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -69,6 +74,11 @@ import org.apache.guacamole.auth.jdbc.user.RemoteAuthenticatedUser; */ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelService { + /** + * Logger for this class. + */ + private final Logger logger = LoggerFactory.getLogger(AbstractGuacamoleTunnelService.class); + /** * The environment of the Guacamole server. */ @@ -439,6 +449,10 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS * Information describing the Guacamole client connecting to the given * connection. * + * @param interceptErrors + * Whether errors from the upstream remote desktop should be + * intercepted and rethrown as GuacamoleUpstreamExceptions. + * * @return * A new GuacamoleTunnel which is configured and connected to the given * connection. @@ -448,7 +462,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS * while connection configuration information is being retrieved. */ private GuacamoleTunnel assignGuacamoleTunnel(ActiveConnectionRecord activeConnection, - GuacamoleClientInformation info) throws GuacamoleException { + GuacamoleClientInformation info, boolean interceptErrors) throws GuacamoleException { // Record new active connection Runnable cleanupTask = new ConnectionCleanupTask(activeConnection); @@ -487,8 +501,11 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS ConfiguredGuacamoleSocket socket = new ConfiguredGuacamoleSocket( getUnconfiguredGuacamoleSocket(cleanupTask), config, info); - // Assign and return new tunnel - return activeConnection.assignGuacamoleTunnel(socket, socket.getConnectionID()); + // Assign and return new tunnel + if (interceptErrors) + return activeConnection.assignGuacamoleTunnel(new FailoverGuacamoleSocket(socket), socket.getConnectionID()); + else + return activeConnection.assignGuacamoleTunnel(socket, socket.getConnectionID()); } @@ -633,7 +650,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS // Connect only if the connection was successfully acquired ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); connectionRecord.init(user, connection); - return assignGuacamoleTunnel(connectionRecord, info); + return assignGuacamoleTunnel(connectionRecord, info, false); } @@ -653,29 +670,50 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS if (connections.isEmpty()) throw new GuacamoleSecurityException("Permission denied."); - // Acquire group - acquire(user, connectionGroup); + do { - // Attempt to acquire to any child - ModeledConnection connection; - try { - connection = acquire(user, connections); - } + // Acquire group + acquire(user, connectionGroup); - // Ensure connection group is always released if child acquire fails - catch (GuacamoleException e) { - release(user, connectionGroup); - throw e; - } + // Attempt to acquire to any child + ModeledConnection connection; + try { + connection = acquire(user, connections); + } - // If session affinity is enabled, prefer this connection going forward - if (connectionGroup.isSessionAffinityEnabled()) - user.preferConnection(connection.getIdentifier()); + // Ensure connection group is always released if child acquire fails + catch (GuacamoleException e) { + release(user, connectionGroup); + throw e; + } - // Connect to acquired child - ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); - connectionRecord.init(user, connectionGroup, connection); - return assignGuacamoleTunnel(connectionRecord, info); + try { + + // Connect to acquired child + ActiveConnectionRecord connectionRecord = activeConnectionRecordProvider.get(); + connectionRecord.init(user, connectionGroup, connection); + GuacamoleTunnel tunnel = assignGuacamoleTunnel(connectionRecord, info, connections.size() > 1); + + // If session affinity is enabled, prefer this connection going forward + if (connectionGroup.isSessionAffinityEnabled()) + user.preferConnection(connection.getIdentifier()); + + return tunnel; + + } + + // If connection failed due to an upstream error, retry other + // connections + catch (GuacamoleUpstreamException e) { + logger.info("Upstream error intercepted for connection \"{}\". Failing over to next connection in group...", connection.getIdentifier()); + logger.debug("Upstream remote desktop reported an error during connection.", e); + connections.remove(connection); + } + + } while (!connections.isEmpty()); + + // All connection possibilities have been exhausted + throw new GuacamoleResourceConflictException("Cannot connect. All upstream connections are unavailable."); } @@ -703,7 +741,7 @@ public abstract class AbstractGuacamoleTunnelService implements GuacamoleTunnelS definition.getSharingProfile()); // Connect to shared connection described by the created record - GuacamoleTunnel tunnel = assignGuacamoleTunnel(connectionRecord, info); + GuacamoleTunnel tunnel = assignGuacamoleTunnel(connectionRecord, info, false); // Register tunnel, such that it is closed when the // SharedConnectionDefinition is invalidated