From 0889e4f2d20bbdd134e77de56ab08bd862cd047f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 23 May 2021 20:08:11 -0700 Subject: [PATCH] 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))