GUACAMOLE-1048: Merge support server control commands during handshake

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

@@ -20,12 +20,15 @@
 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.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
@@ -39,6 +42,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.
@@ -60,16 +69,82 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket {
     private GuacamoleProtocolVersion protocolVersion =
             GuacamoleProtocolVersion.VERSION_1_0_0;
 
+    /**
+     * 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 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<String> 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);
+
+    }
+
     /**
      * 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.
      *
-     * @param reader The reader to read instructions from.
+     * Respects server control instructions that are allowed during the handshake
-     * @param opcode The opcode of the instruction we are expecting.
+     * phase, namely {@code error} and {@code disconnect}.
-     * @return The instruction having the given opcode.
+     *
-     * @throws GuacamoleException If an error occurs while reading, or if
+     * @param reader
-     *                            the expected instruction is not read.
+     *     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 {
@@ -79,6 +154,14 @@ public class ConfiguredGuacamoleSocket extends DelegatingGuacamoleSocket {
         if (instruction == null)
             throw new GuacamoleServerException("End of stream while waiting for \"" + opcode + "\".");
 
+        // Report connection closure if server explicitly disconnects
+        if ("disconnect".equals(instruction.getOpcode()))
+            throw new GuacamoleConnectionClosedException("Server disconnected while waiting for \"" + opcode + "\".");
+
+        // Pass through any received errors as GuacamoleExceptions
+        if ("error".equals(instruction.getOpcode()))
+            handleReceivedError(instruction);
+
         // Ensure instruction has expected opcode
         if (!instruction.getOpcode().equals(opcode))
             throw new GuacamoleServerException("Expected \"" + opcode + "\" instruction but instead received \"" + instruction.getOpcode() + "\".");

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);
+
 }

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());
+
+        }
+
+    }
+
+}