From fdff3e187bc4d3b4f541e75309ca59576d5e2047 Mon Sep 17 00:00:00 2001 From: Tomer Gabel Date: Thu, 23 Apr 2020 17:51:49 +0300 Subject: [PATCH 1/4] GUACAMOLE-1048: Support server control commands during handshake --- ...camoleServerErrorInstructionException.java | 55 +++++++++++++++++++ .../protocol/ConfiguredGuacamoleSocket.java | 34 +++++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java diff --git a/guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java b/guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java new file mode 100644 index 000000000..f6889c8e6 --- /dev/null +++ b/guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java @@ -0,0 +1,55 @@ +/* + * 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; + +import org.apache.guacamole.protocol.GuacamoleStatus; + +/** + * The exception thrown when the Guacamole server explicitly sends an error + * instruction to the client. The error message and status code reflect the arguments + * of the error instruction as determined by the server. + */ +public class GuacamoleServerErrorInstructionException extends GuacamoleServerException { + /** + * The Guacamole protocol status code, as determined by the server; + */ + private final GuacamoleStatus status; + + /** + * Creates a new GuacamoleServerErrorInstructionException with the given + * message and status. + * + * @param message + * A human readable description of the exception that occurred. + * + * @param status + * The status code, as determined by the server from which the + * instruction originated. + */ + public GuacamoleServerErrorInstructionException(String message, GuacamoleStatus status) { + super(message); + this.status = status; + } + + @Override + public GuacamoleStatus getStatus() { + return status; + } +} diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java index 6cf3d7b3c..55d7c9473 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java @@ -21,6 +21,7 @@ package org.apache.guacamole.protocol; import java.util.List; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleServerErrorInstructionException; import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.io.GuacamoleReader; import org.apache.guacamole.io.GuacamoleWriter; @@ -59,12 +60,35 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { */ private GuacamoleProtocolVersion protocolVersion = GuacamoleProtocolVersion.VERSION_1_0_0; - + + /** + * Parses the arguments for the Guacamole "error" server instruction and returns + * the corresponding exception. + * @param args The arguments as provided by the server instruction. + * @return An instance of {@link GuacamoleServerErrorInstructionException} configured + * with the server-provided arguments, or a generic {@link GuacamoleServerException} if + * the specified arguments are invalid. + */ + private static GuacamoleServerException parseServerErrorInstructionArgs(List args) { + try { + if (args.size() >= 2) { + int code = Integer.parseInt(args.get(1)); + GuacamoleStatus status = GuacamoleStatus.fromGuacamoleStatusCode(code); + return new GuacamoleServerErrorInstructionException(args.get(0), status); + } + } catch (NumberFormatException ignored) {} + + return new GuacamoleServerException("Invalid error instruction arguments received: " + args); + } + /** * Waits for the instruction having the given opcode, returning that * instruction once it has been read. If the instruction is never read, * an exception is thrown. - * + * + * Respects server control instructions that are allowed during the handshake + * phase, namely {@code error} and {@code disconnect}. + * * @param reader The reader to read instructions from. * @param opcode The opcode of the instruction we are expecting. * @return The instruction having the given opcode. @@ -79,6 +103,12 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { if (instruction == null) throw new GuacamoleServerException("End of stream while waiting for \"" + opcode + "\"."); + // Handle server control instructions + if ("disconnect".equals(instruction.getOpcode())) + throw new GuacamoleServerException("Server disconnected while waiting for \"" + opcode + "\"."); + if ("error".equals(instruction.getOpcode())) + throw parseServerErrorInstructionArgs(instruction.getArgs()); + // Ensure instruction has expected opcode if (!instruction.getOpcode().equals(opcode)) throw new GuacamoleServerException("Expected \"" + opcode + "\" instruction but instead received \"" + instruction.getOpcode() + "\"."); From e2f2b715a4114b8e5d4c5a3833cadb9592f6a822 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 23 May 2021 20:02:30 -0700 Subject: [PATCH 2/4] GUACAMOLE-1048: Allow direct conversion from GuacamoleStatus to GuacamoleException. --- .../guacamole/protocol/GuacamoleStatus.java | 234 ++++++++++++++++-- .../protocol/GuacamoleStatusTest.java | 78 ++++++ 2 files changed, 291 insertions(+), 21 deletions(-) create mode 100644 guacamole-common/src/test/java/org/apache/guacamole/protocol/GuacamoleStatusTest.java diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleStatus.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleStatus.java index f886d7d43..894dc06b8 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleStatus.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/GuacamoleStatus.java @@ -19,6 +19,28 @@ package org.apache.guacamole.protocol; +import org.apache.guacamole.GuacamoleClientBadTypeException; +import org.apache.guacamole.GuacamoleClientException; +import org.apache.guacamole.GuacamoleClientOverrunException; +import org.apache.guacamole.GuacamoleClientTimeoutException; +import org.apache.guacamole.GuacamoleClientTooManyException; +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.GuacamoleResourceClosedException; +import org.apache.guacamole.GuacamoleResourceConflictException; +import org.apache.guacamole.GuacamoleResourceNotFoundException; +import org.apache.guacamole.GuacamoleSecurityException; +import org.apache.guacamole.GuacamoleServerBusyException; +import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.GuacamoleSessionClosedException; +import org.apache.guacamole.GuacamoleSessionConflictException; +import org.apache.guacamole.GuacamoleSessionTimeoutException; +import org.apache.guacamole.GuacamoleUnauthorizedException; +import org.apache.guacamole.GuacamoleUnsupportedException; +import org.apache.guacamole.GuacamoleUpstreamException; +import org.apache.guacamole.GuacamoleUpstreamNotFoundException; +import org.apache.guacamole.GuacamoleUpstreamTimeoutException; +import org.apache.guacamole.GuacamoleUpstreamUnavailableException; + /** * All possible statuses returned by various Guacamole instructions, each having * a corresponding code. @@ -28,86 +50,193 @@ public enum GuacamoleStatus { /** * The operation succeeded. */ - SUCCESS(200, 1000, 0x0000), + SUCCESS(200, 1000, 0x0000) { + + @Override + public GuacamoleException toException(String message) { + throw new IllegalStateException("The Guacamole protocol SUCCESS " + + "status code cannot be converted into a " + + "GuacamoleException.", new GuacamoleServerException(message)); + } + + }, /** * The requested operation is unsupported. */ - UNSUPPORTED(501, 1011, 0x0100), + UNSUPPORTED(501, 1011, 0x0100) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleUnsupportedException(message); + } + + }, /** * The operation could not be performed due to an internal failure. */ - SERVER_ERROR(500, 1011, 0x0200), + SERVER_ERROR(500, 1011, 0x0200) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleServerException(message); + } + + }, /** * The operation could not be performed as the server is busy. */ - SERVER_BUSY(503, 1008, 0x0201), + SERVER_BUSY(503, 1008, 0x0201) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleServerBusyException(message); + } + + }, /** * The operation could not be performed because the upstream server is not * responding. */ - UPSTREAM_TIMEOUT(504, 1011, 0x0202), + UPSTREAM_TIMEOUT(504, 1011, 0x0202) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleUpstreamTimeoutException(message); + } + + }, /** * The operation was unsuccessful due to an error or otherwise unexpected * condition of the upstream server. */ - UPSTREAM_ERROR(502, 1011, 0x0203), + UPSTREAM_ERROR(502, 1011, 0x0203) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleUpstreamException(message); + } + + }, /** * The operation could not be performed as the requested resource does not * exist. */ - RESOURCE_NOT_FOUND(404, 1002, 0x0204), + RESOURCE_NOT_FOUND(404, 1002, 0x0204) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleResourceNotFoundException(message); + } + + }, /** * The operation could not be performed as the requested resource is already * in use. */ - RESOURCE_CONFLICT(409, 1008, 0x0205), + RESOURCE_CONFLICT(409, 1008, 0x0205) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleResourceConflictException(message); + } + + }, /** * The operation could not be performed as the requested resource is now * closed. */ - RESOURCE_CLOSED(404, 1002, 0x0206), + RESOURCE_CLOSED(404, 1002, 0x0206) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleResourceClosedException(message); + } + + }, /** * The operation could not be performed because the upstream server does * not appear to exist. */ - UPSTREAM_NOT_FOUND(502, 1011, 0x0207), + UPSTREAM_NOT_FOUND(502, 1011, 0x0207) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleUpstreamNotFoundException(message); + } + + }, /** * The operation could not be performed because the upstream server is not * available to service the request. */ - UPSTREAM_UNAVAILABLE(502, 1011, 0x0208), + UPSTREAM_UNAVAILABLE(502, 1011, 0x0208) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleUpstreamUnavailableException(message); + } + + }, /** * The session within the upstream server has ended because it conflicted * with another session. */ - SESSION_CONFLICT(409, 1008, 0x0209), + SESSION_CONFLICT(409, 1008, 0x0209) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleSessionConflictException(message); + } + + }, /** * The session within the upstream server has ended because it appeared to * be inactive. */ - SESSION_TIMEOUT(408, 1002, 0x020A), + SESSION_TIMEOUT(408, 1002, 0x020A) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleSessionTimeoutException(message); + } + + }, /** * The session within the upstream server has been forcibly terminated. */ - SESSION_CLOSED(404, 1002, 0x020B), + SESSION_CLOSED(404, 1002, 0x020B) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleSessionClosedException(message); + } + + }, /** * The operation could not be performed because bad parameters were given. */ - CLIENT_BAD_REQUEST(400, 1002, 0x0300), + CLIENT_BAD_REQUEST(400, 1002, 0x0300) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleClientException(message); + } + + }, /** * Permission was denied to perform the operation, as the user is not yet @@ -115,34 +244,76 @@ public enum GuacamoleStatus { * for HTTP-specific authorization schemes, this status continues to map to * HTTP 403 ("Forbidden"). To do otherwise would risk unintended effects. */ - CLIENT_UNAUTHORIZED(403, 1008, 0x0301), + CLIENT_UNAUTHORIZED(403, 1008, 0x0301) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleUnauthorizedException(message); + } + + }, /** * Permission was denied to perform the operation, and this operation will * not be granted even if the user is authorized. */ - CLIENT_FORBIDDEN(403, 1008, 0x0303), + CLIENT_FORBIDDEN(403, 1008, 0x0303) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleSecurityException(message); + } + + }, /** * The client took too long to respond. */ - CLIENT_TIMEOUT(408, 1002, 0x0308), + CLIENT_TIMEOUT(408, 1002, 0x0308) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleClientTimeoutException(message); + } + + }, /** * The client sent too much data. */ - CLIENT_OVERRUN(413, 1009, 0x030D), + CLIENT_OVERRUN(413, 1009, 0x030D) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleClientOverrunException(message); + } + + }, /** * The client sent data of an unsupported or unexpected type. */ - CLIENT_BAD_TYPE(415, 1003, 0x030F), + CLIENT_BAD_TYPE(415, 1003, 0x030F) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleClientBadTypeException(message); + } + + }, /** * The operation failed because the current client is already using too * many resources. */ - CLIENT_TOO_MANY(429, 1008, 0x031D); + CLIENT_TOO_MANY(429, 1008, 0x031D) { + + @Override + public GuacamoleException toException(String message) { + return new GuacamoleClientTooManyException(message); + } + + }; /** * The most applicable HTTP error code. @@ -226,4 +397,25 @@ public enum GuacamoleStatus { } + /** + * Returns an instance of the {@link GuacamoleException} subclass + * corresponding to this Guacamole protocol status code. All status codes + * have a corresponding GuacamoleException except for {@link SUCCESS}. The + * returned GuacamoleException will have the provided human-readable + * message. + * + * @param message + * A human readable description of the error that occurred. + * + * @return + * An instance of the {@link GuacamoleException} subclass that + * corresponds to this status code and has the provided human-readable + * message. + * + * @throws IllegalStateException + * If invoked on {@link SUCCESS}, which has no corresponding + * GuacamoleException. + */ + public abstract GuacamoleException toException(String message); + } diff --git a/guacamole-common/src/test/java/org/apache/guacamole/protocol/GuacamoleStatusTest.java b/guacamole-common/src/test/java/org/apache/guacamole/protocol/GuacamoleStatusTest.java new file mode 100644 index 000000000..0d38e6e13 --- /dev/null +++ b/guacamole-common/src/test/java/org/apache/guacamole/protocol/GuacamoleStatusTest.java @@ -0,0 +1,78 @@ +/* + * 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 org.apache.guacamole.GuacamoleException; +import org.junit.Assert; +import org.junit.Test; + +/** + * Unit test for GuacamoleStatus that verifies exception translation functions + * as required. + */ +public class GuacamoleStatusTest { + + /** + * Verifies that the {@link SUCCESS} status code does NOT translate to a + * GuacamoleException, but instead throws an IllegalStateException. + */ + @Test + public void testSuccessHasNoException() { + + try { + GuacamoleStatus.SUCCESS.toException("Test message"); + Assert.fail("GuacamoleStatus.SUCCESS must throw " + + "IllegalStateException for toException()."); + } + catch (IllegalStateException e) { + // Expected + } + + } + + /** + * Verifies that each non-success GuacamoleStatus maps to a + * GuacamoleException associated with that GuacamoleStatus. + */ + @Test + public void testStatusExceptionMapping() { + + for (GuacamoleStatus status : GuacamoleStatus.values()) { + + // Ignore SUCCESS status (tested via testSuccessHasNoException()) + if (status == GuacamoleStatus.SUCCESS) + continue; + + String message = "Test message: " + status; + GuacamoleException e = status.toException(message); + + Assert.assertEquals("toException() should return a " + + "GuacamoleException that maps to the same status.", + status, e.getStatus()); + + Assert.assertEquals("toException() should return a " + + "GuacamoleException that uses the provided message.", + message, e.getMessage()); + + } + + } + +} From 0889e4f2d20bbdd134e77de56ab08bd862cd047f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 23 May 2021 20:08:11 -0700 Subject: [PATCH 3/4] GUACAMOLE-1048: Leverage exception conversion provided by GuacamoleStatus for "error" instruction handling. --- ...camoleServerErrorInstructionException.java | 55 ------------- .../protocol/ConfiguredGuacamoleSocket.java | 77 +++++++++++++++---- 2 files changed, 60 insertions(+), 72 deletions(-) delete mode 100644 guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java diff --git a/guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java b/guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java deleted file mode 100644 index f6889c8e6..000000000 --- a/guacamole-common/src/main/java/org/apache/guacamole/GuacamoleServerErrorInstructionException.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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; - -import org.apache.guacamole.protocol.GuacamoleStatus; - -/** - * The exception thrown when the Guacamole server explicitly sends an error - * instruction to the client. The error message and status code reflect the arguments - * of the error instruction as determined by the server. - */ -public class GuacamoleServerErrorInstructionException extends GuacamoleServerException { - /** - * The Guacamole protocol status code, as determined by the server; - */ - private final GuacamoleStatus status; - - /** - * Creates a new GuacamoleServerErrorInstructionException with the given - * message and status. - * - * @param message - * A human readable description of the exception that occurred. - * - * @param status - * The status code, as determined by the server from which the - * instruction originated. - */ - public GuacamoleServerErrorInstructionException(String message, GuacamoleStatus status) { - super(message); - this.status = status; - } - - @Override - public GuacamoleStatus getStatus() { - return status; - } -} diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java index 55d7c9473..541586f05 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java @@ -21,12 +21,13 @@ package org.apache.guacamole.protocol; import java.util.List; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.GuacamoleServerErrorInstructionException; import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.io.GuacamoleReader; import org.apache.guacamole.io.GuacamoleWriter; import org.apache.guacamole.net.DelegatingGuacamoleSocket; import org.apache.guacamole.net.GuacamoleSocket; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A GuacamoleSocket which pre-configures the connection based on a given @@ -40,6 +41,12 @@ import org.apache.guacamole.net.GuacamoleSocket; */ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { + /** + * Logger for this class. + */ + private static final Logger logger = + LoggerFactory.getLogger(ConfiguredGuacamoleSocket.class); + /** * The configuration to use when performing the Guacamole protocol * handshake. @@ -62,23 +69,59 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { GuacamoleProtocolVersion.VERSION_1_0_0; /** - * Parses the arguments for the Guacamole "error" server instruction and returns - * the corresponding exception. - * @param args The arguments as provided by the server instruction. - * @return An instance of {@link GuacamoleServerErrorInstructionException} configured - * with the server-provided arguments, or a generic {@link GuacamoleServerException} if - * the specified arguments are invalid. + * Parses the given "error" instruction, throwing a GuacamoleException that + * corresponds to its status code and message. + * + * @param instruction + * The "error" instruction to parse. + * + * @throws GuacamoleException + * A GuacamoleException that corresponds to the status code and message + * present within the given "error" instruction. */ - private static GuacamoleServerException parseServerErrorInstructionArgs(List args) { - try { - if (args.size() >= 2) { - int code = Integer.parseInt(args.get(1)); - GuacamoleStatus status = GuacamoleStatus.fromGuacamoleStatusCode(code); - return new GuacamoleServerErrorInstructionException(args.get(0), status); - } - } catch (NumberFormatException ignored) {} + private static void handleReceivedError(GuacamoleInstruction instruction) + throws GuacamoleException { + + // Provide reasonable default error message for invalid "error" + // instructions that fail to provide one + String message = "Internal error within guacd / protocol handling."; + + // Consider all error instructions without a corresponding status code + // to be server errors + GuacamoleStatus status = GuacamoleStatus.SERVER_ERROR; + + // Parse human-readable message from "error" instruction, warning if no + // message was given + List args = instruction.getArgs(); + if (args.size() >= 1) + message = args.get(0); + else + logger.debug("Received \"error\" instruction with no corresponding message."); + + // Parse the status code from the received error instruction, warning + // if the status code is missing or invalid + if (args.size() >= 2) { + try { + + // Translate numeric status code into a GuacamoleStatus + int statusCode = Integer.parseInt(args.get(1)); + GuacamoleStatus parsedStatus = GuacamoleStatus.fromGuacamoleStatusCode(statusCode); + if (parsedStatus != null) + status = parsedStatus; + else + logger.debug("Received \"error\" instruction with unknown/invalid status code: {}", statusCode); + + } + catch (NumberFormatException e) { + logger.debug("Received \"error\" instruction with non-numeric status code.", e); + } + } + else + logger.debug("Received \"error\" instruction without status code."); + + // Convert parsed status code and message to a GuacamoleException + throw status.toException(message); - return new GuacamoleServerException("Invalid error instruction arguments received: " + args); } /** @@ -107,7 +150,7 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { if ("disconnect".equals(instruction.getOpcode())) throw new GuacamoleServerException("Server disconnected while waiting for \"" + opcode + "\"."); if ("error".equals(instruction.getOpcode())) - throw parseServerErrorInstructionArgs(instruction.getArgs()); + handleReceivedError(instruction); // Ensure instruction has expected opcode if (!instruction.getOpcode().equals(opcode)) From 307ec9627a2e555094fa5863f2a83e042c7bca22 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 23 May 2021 21:14:54 -0700 Subject: [PATCH 4/4] GUACAMOLE-1048: Use GuacamoleConnectionClosedException to represent explicit connection closure. --- .../protocol/ConfiguredGuacamoleSocket.java | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java b/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java index 541586f05..1b34ef84a 100644 --- a/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java +++ b/guacamole-common/src/main/java/org/apache/guacamole/protocol/ConfiguredGuacamoleSocket.java @@ -20,6 +20,7 @@ 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; @@ -132,11 +133,18 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { * Respects server control instructions that are allowed during the handshake * phase, namely {@code error} and {@code disconnect}. * - * @param reader The reader to read instructions from. - * @param opcode The opcode of the instruction we are expecting. - * @return The instruction having the given opcode. - * @throws GuacamoleException If an error occurs while reading, or if - * the expected instruction is not read. + * @param reader + * The reader to read instructions from. + * + * @param opcode + * The opcode of the instruction we are expecting. + * + * @return + * The instruction having the given opcode. + * + * @throws GuacamoleException + * If an error occurs while reading, or if the expected instruction is + * not read. */ private GuacamoleInstruction expect(GuacamoleReader reader, String opcode) throws GuacamoleException { @@ -146,9 +154,11 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket { if (instruction == null) throw new GuacamoleServerException("End of stream while waiting for \"" + opcode + "\"."); - // Handle server control instructions + // Report connection closure if server explicitly disconnects if ("disconnect".equals(instruction.getOpcode())) - throw new GuacamoleServerException("Server disconnected while waiting for \"" + opcode + "\"."); + throw new GuacamoleConnectionClosedException("Server disconnected while waiting for \"" + opcode + "\"."); + + // Pass through any received errors as GuacamoleExceptions if ("error".equals(instruction.getOpcode())) handleReceivedError(instruction);