GUACAMOLE-422: Merge changes to Guacamole protocol handshake adding flexibility and timezone support.

src/libguac/guacamole/protocol.h

@@ -38,6 +38,20 @@
 #include <cairo/cairo.h>
 #include <stdarg.h>
 
+/**
+ * This defines the overall protocol version that this build of libguac
+ * supports.  The protocol version is used to provide compatibility between
+ * potentially different versions of Guacamole server and clients.  The
+ * version number is a MAJOR_MINOR_PATCH version that matches the versioning
+ * used throughout the components of the Guacamole project.  This version
+ * will not necessarily increment with the other components, unless additional
+ * functionality is introduced that affects compatibility.
+ * 
+ * This version is passed by the __guac_protocol_send_args() function from the
+ * server to the client during the client/server handshake.
+ */
+#define GUACAMOLE_PROTOCOL_VERSION "VERSION_1_1_0"
+
 /* CONTROL INSTRUCTIONS */
 
 /**

src/libguac/guacamole/user.h

@@ -89,6 +89,13 @@ struct guac_user_info {
      */
     int optimal_resolution;
     
+    /**
+     * The timezone of the remote system.  If the client does not provide
+     * a specific timezone then this will be NULL.  The format of the timezone
+     * is the standard tzdata naming convention.
+     */
+    const char* timezone;
+
 };
 
 struct guac_user {
@@ -540,7 +547,7 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout);
 /**
  * Call the appropriate handler defined by the given user for the given
  * instruction. A comparison is made between the instruction opcode and the
- * initial handler lookup table defined in user-handlers.c. The intial handlers
+ * initial handler lookup table defined in user-handlers.c. The initial handlers
  * will in turn call the user's handler (if defined).
  *
  * @param user

src/libguac/protocol.c

@@ -94,6 +94,11 @@ static int __guac_protocol_send_args(guac_socket* socket, const char** args) {
 
     if (guac_socket_write_string(socket, "4.args")) return -1;
     
+    /* Send protocol version ahead of other args. */
+    if (guac_socket_write_string(socket, ",")
+            || __guac_socket_write_length_string(socket, GUACAMOLE_PROTOCOL_VERSION))
+        return -1;
+
     for (i=0; args[i] != NULL; i++) {
 
         if (guac_socket_write_string(socket, ","))

src/libguac/user-handlers.c

@@ -31,6 +31,7 @@
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
+#include <string.h>
 
 /* Guacamole instruction handler map */
 
@@ -53,6 +54,17 @@ __guac_instruction_handler_mapping __guac_instruction_handler_map[] = {
    {NULL,         NULL}
 };
 
+/* Guacamole handshake handler map */
+
+__guac_instruction_handler_mapping __guac_handshake_handler_map[] = {
+    {"size",     __guac_handshake_size_handler},
+    {"audio",    __guac_handshake_audio_handler},
+    {"video",    __guac_handshake_video_handler},
+    {"image",    __guac_handshake_image_handler},
+    {"timezone", __guac_handshake_timezone_handler},
+    {NULL,       NULL}
+};
+
 /**
  * Parses a 64-bit integer from the given string. It is assumed that the string
  * will contain only decimal digits, with an optional leading minus sign.
@@ -581,3 +593,136 @@ int __guac_handle_disconnect(guac_user* user, int argc, char** argv) {
     return 0;
 }
 
+/* Guacamole handshake handler functions. */
+
+int __guac_handshake_size_handler(guac_user* user, int argc, char** argv) {
+    
+    /* Validate size of instruction. */
+    if (argc < 2) {
+        guac_user_log(user, GUAC_LOG_ERROR, "Received \"size\" "
+                "instruction lacked required arguments.");
+        return 1;
+    }
+    
+    /* Parse optimal screen dimensions from size instruction */
+    user->info.optimal_width  = atoi(argv[0]);
+    user->info.optimal_height = atoi(argv[1]);
+
+    /* If DPI given, set the user resolution */
+    if (argc >= 3)
+        user->info.optimal_resolution = atoi(argv[2]);
+
+    /* Otherwise, use a safe default for rough backwards compatibility */
+    else
+        user->info.optimal_resolution = 96;
+    
+    return 0;
+    
+}
+
+int __guac_handshake_audio_handler(guac_user* user, int argc, char** argv) {
+
+    guac_free_mimetypes((char **) user->info.audio_mimetypes);
+    
+    /* Store audio mimetypes */
+    user->info.audio_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
+    
+    return 0;
+    
+}
+
+int __guac_handshake_video_handler(guac_user* user, int argc, char** argv) {
+
+    guac_free_mimetypes((char **) user->info.video_mimetypes);
+    
+    /* Store video mimetypes */
+    user->info.video_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
+    
+    return 0;
+    
+}
+
+int __guac_handshake_image_handler(guac_user* user, int argc, char** argv) {
+    
+    guac_free_mimetypes((char **) user->info.image_mimetypes);
+    
+    /* Store image mimetypes */
+    user->info.image_mimetypes = (const char**) guac_copy_mimetypes(argv, argc);
+    
+    return 0;
+    
+}
+
+int __guac_handshake_timezone_handler(guac_user* user, int argc, char** argv) {
+    
+    /* Free any past value */
+    free((char *) user->info.timezone);
+    
+    /* Store timezone, if present */
+    if (argc > 0 && strcmp(argv[0], ""))
+        user->info.timezone = (const char*) strdup(argv[0]);
+    
+    else
+        user->info.timezone = NULL;
+    
+    return 0;
+    
+}
+
+char** guac_copy_mimetypes(char** mimetypes, int count) {
+
+    int i;
+
+    /* Allocate sufficient space for NULL-terminated array of mimetypes */
+    char** mimetypes_copy = malloc(sizeof(char*) * (count+1));
+
+    /* Copy each provided mimetype */
+    for (i = 0; i < count; i++)
+        mimetypes_copy[i] = strdup(mimetypes[i]);
+
+    /* Terminate with NULL */
+    mimetypes_copy[count] = NULL;
+
+    return mimetypes_copy;
+
+}
+
+void guac_free_mimetypes(char** mimetypes) {
+
+    if (mimetypes == NULL)
+        return;
+    
+    char** current_mimetype = mimetypes;
+
+    /* Free all strings within NULL-terminated mimetype array */
+    while (*current_mimetype != NULL) {
+        free(*current_mimetype);
+        current_mimetype++;
+    }
+
+    /* Free the array itself, now that its contents have been freed */
+    free(mimetypes);
+
+}
+
+int __guac_user_call_opcode_handler(__guac_instruction_handler_mapping* map,
+        guac_user* user, const char* opcode, int argc, char** argv) {
+
+    /* For each defined instruction */
+    __guac_instruction_handler_mapping* current = map;
+    while (current->opcode != NULL) {
+
+        /* If recognized, call handler */
+        if (strcmp(opcode, current->opcode) == 0)
+            return current->handler(user, argc, argv);
+
+        current++;
+    }
+
+    /* If unrecognized, log and ignore */
+    guac_user_log(user, GUAC_LOG_DEBUG, "Handler not found for \"%s\"",
+            opcode);
+    return 0;
+
+}
+

src/libguac/user-handlers.h

@@ -177,6 +177,40 @@ __guac_instruction_handler __guac_handle_size;
  */
 __guac_instruction_handler __guac_handle_disconnect;
 
+/**
+ * Internal handler function that is called when the size instruction is
+ * received during the handshake process.
+ */
+__guac_instruction_handler __guac_handshake_size_handler;
+
+/**
+ * Internal handler function that is called when the audio instruction is
+ * received during the handshake process, specifying the audio mimetypes
+ * available to the client.
+ */
+__guac_instruction_handler __guac_handshake_audio_handler;
+
+/**
+ * Internal handler function that is called when the video instruction is
+ * received during the handshake process, specifying the video mimetypes
+ * available to the client.
+ */
+__guac_instruction_handler __guac_handshake_video_handler;
+
+/**
+ * Internal handler function that is called when the image instruction is
+ * received during the handshake process, specifying the image mimetypes
+ * available to the client.
+ */
+__guac_instruction_handler __guac_handshake_image_handler;
+
+/**
+ * Internal handler function that is called when the timezone instruction is
+ * received during the handshake process, specifying the timezone of the
+ * client.
+ */
+__guac_instruction_handler __guac_handshake_timezone_handler;
+
 /**
  * Instruction handler mapping table. This is a NULL-terminated array of
  * __guac_instruction_handler_mapping structures, each mapping an opcode
@@ -186,4 +220,72 @@ __guac_instruction_handler __guac_handle_disconnect;
  */
 extern __guac_instruction_handler_mapping __guac_instruction_handler_map[];
 
+/**
+ * Handler mapping table for instructions (opcodes) specifically for the
+ * handshake portion of the connection.  Each
+ * __guac_instruction_handler_mapping structure within this NULL-terminated
+ * array maps an opcode to a __guac_instruction_handler.  The end of the array
+ * must be marked with a mapping with the opcode set to NULL.
+ */
+extern __guac_instruction_handler_mapping __guac_handshake_handler_map[];
+
+/**
+ * Frees the given array of mimetypes, including the space allocated to each
+ * mimetype string within the array. The provided array of mimetypes MUST have
+ * been allocated with guac_copy_mimetypes().
+ *
+ * @param mimetypes
+ *     The NULL-terminated array of mimetypes to free. This array MUST have
+ *     been previously allocated with guac_copy_mimetypes().
+ */
+void guac_free_mimetypes(char** mimetypes);
+
+/**
+ * Copies the given array of mimetypes (strings) into a newly-allocated NULL-
+ * terminated array of strings. Both the array and the strings within the array
+ * are newly-allocated and must be later freed via guac_free_mimetypes().
+ *
+ * @param mimetypes
+ *     The array of mimetypes to copy.
+ *
+ * @param count
+ *     The number of mimetypes in the given array.
+ *
+ * @return
+ *     A newly-allocated, NULL-terminated array containing newly-allocated
+ *     copies of each of the mimetypes provided in the original mimetypes
+ *     array.
+ */
+char** guac_copy_mimetypes(char** mimetypes, int count);
+
+/**
+ * Call the appropriate handler defined by the given user for the given
+ * instruction. A comparison is made between the instruction opcode and the
+ * initial handler lookup table defined in the map that is provided to this
+ * function. If an entry for the instruction is found in the provided map,
+ * the handler defined in that map will be called and the value returned.  If
+ * no match is found, it is silently ignored.
+ *
+ * @param map
+ *     The array that holds the opcode to handler mappings.
+ * 
+ * @param user
+ *     The user whose handlers should be called.
+ *
+ * @param opcode
+ *     The opcode of the instruction to pass to the user via the appropriate
+ *     handler.
+ *
+ * @param argc
+ *     The number of arguments which are part of the instruction.
+ *
+ * @param argv
+ *     An array of all arguments which are part of the instruction.
+ *
+ * @return
+ *     Zero if the instruction was handled successfully, or non-zero otherwise.
+ */
+int __guac_user_call_opcode_handler(__guac_instruction_handler_mapping* map,
+        guac_user* user, const char* opcode, int argc, char** argv);
+
 #endif

src/libguac/user-handshake.c

@@ -25,6 +25,7 @@
 #include "guacamole/protocol.h"
 #include "guacamole/socket.h"
 #include "guacamole/user.h"
+#include "user-handlers.h"
 
 #include <pthread.h>
 #include <stdlib.h>
@@ -113,64 +114,6 @@ static void guac_user_log_handshake_failure(guac_user* user) {
 
 }
 
-/**
- * Copies the given array of mimetypes (strings) into a newly-allocated NULL-
- * terminated array of strings. Both the array and the strings within the array
- * are newly-allocated and must be later freed via guac_free_mimetypes().
- *
- * @param mimetypes
- *     The array of mimetypes to copy.
- *
- * @param count
- *     The number of mimetypes in the given array.
- *
- * @return
- *     A newly-allocated, NULL-terminated array containing newly-allocated
- *     copies of each of the mimetypes provided in the original mimetypes
- *     array.
- */
-static char** guac_copy_mimetypes(char** mimetypes, int count) {
-
-    int i;
-
-    /* Allocate sufficient space for NULL-terminated array of mimetypes */
-    char** mimetypes_copy = malloc(sizeof(char*) * (count+1));
-
-    /* Copy each provided mimetype */
-    for (i = 0; i < count; i++)
-        mimetypes_copy[i] = strdup(mimetypes[i]);
-
-    /* Terminate with NULL */
-    mimetypes_copy[count] = NULL;
-
-    return mimetypes_copy;
-
-}
-
-/**
- * Frees the given array of mimetypes, including the space allocated to each
- * mimetype string within the array. The provided array of mimetypes MUST have
- * been allocated with guac_copy_mimetypes().
- *
- * @param mimetypes
- *     The NULL-terminated array of mimetypes to free. This array MUST have
- *     been previously allocated with guac_copy_mimetypes().
- */
-static void guac_free_mimetypes(char** mimetypes) {
-
-    char** current_mimetype = mimetypes;
-
-    /* Free all strings within NULL-terminated mimetype array */
-    while (*current_mimetype != NULL) {
-        free(*current_mimetype);
-        current_mimetype++;
-    }
-
-    /* Free the array itself, now that its contents have been freed */
-    free(mimetypes);
-
-}
-
 /**
  * The thread which handles all user input, calling event handlers for received
  * instructions.
@@ -219,7 +162,8 @@ static void* guac_user_input_thread(void* data) {
         guac_error_message = NULL;
 
         /* Call handler, stop on error */
-        if (guac_user_handle_instruction(user, parser->opcode, parser->argc, parser->argv) < 0) {
+        if (__guac_user_call_opcode_handler(__guac_instruction_handler_map, 
+                user, parser->opcode, parser->argc, parser->argv)) {
 
             /* Log error */
             guac_user_log_guac_error(user, GUAC_LOG_WARNING,
@@ -286,11 +230,78 @@ static int guac_user_start(guac_parser* parser, guac_user* user,
 
 }
 
+/**
+ * This function loops through the received instructions during the handshake
+ * with the client attempting to join the connection, and runs the handlers
+ * for each of the opcodes, ending when the connect instruction is received.
+ * Returns zero if the handshake completes successfully with the connect opcode,
+ * or a non-zero value if an error occurs.
+ * 
+ * @param user
+ *     The guac_user attempting to join the connection.
+ * 
+ * @param parser
+ *     The parser used to examine the received data.
+ * 
+ * @param usec_timeout
+ *     The timeout, in microseconds, for reading the instructions.
+ * 
+ * @return
+ *     Zero if the handshake completes successfully with the connect opcode,
+ *     or non-zero if an error occurs.
+ */
+static int __guac_user_handshake(guac_user* user, guac_parser* parser,
+        int usec_timeout) {
+    
+    guac_socket* socket = user->socket;
+    
+    /* Handle each of the opcodes. */
+    while (guac_parser_read(parser, socket, usec_timeout) == 0) {
+        
+        /* If we receive the connect opcode, we're done. */
+        if (strcmp(parser->opcode, "connect") == 0)
+            return 0;
+        
+        guac_user_log(user, GUAC_LOG_DEBUG, "Processing instruction: %s",
+                parser->opcode);
+        
+        /* Run instruction handler for opcode with arguments. */
+        if (__guac_user_call_opcode_handler(__guac_handshake_handler_map, user,
+                parser->opcode, parser->argc, parser->argv)) {
+            
+            guac_user_log_handshake_failure(user);
+            guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
+                    "Error handling instruction during handshake.");
+            guac_user_log(user, GUAC_LOG_DEBUG, "Failed opcode: %s",
+                    parser->opcode);
+
+            guac_parser_free(parser);
+            return 1;
+            
+        }
+        
+    }
+    
+    /* If we get here it's because we never got the connect instruction. */
+    guac_user_log(user, GUAC_LOG_ERROR,
+            "Handshake failed, \"connect\" instruction was not received.");
+    return 1;
+}
+
 int guac_user_handle_connection(guac_user* user, int usec_timeout) {
 
     guac_socket* socket = user->socket;
     guac_client* client = user->client;
     
+    user->info.audio_mimetypes = NULL;
+    user->info.image_mimetypes = NULL;
+    user->info.video_mimetypes = NULL;
+    user->info.timezone = NULL;
+    
+    /* Count number of arguments. */
+    int num_args;
+    for (num_args = 0; client->args[num_args] != NULL; num_args++);
+    
     /* Send args */
     if (guac_protocol_send_args(socket, client->args)
             || guac_socket_flush(socket)) {
@@ -305,94 +316,8 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) {
 
     guac_parser* parser = guac_parser_alloc();
 
-    /* Get optimal screen size */
+    /* Perform the handshake with the client. */
-    if (guac_parser_expect(parser, socket, usec_timeout, "size")) {
+    if (__guac_user_handshake(user, parser, usec_timeout)) {
-
-        /* Log error */
-        guac_user_log_handshake_failure(user);
-        guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
-                "Error reading \"size\"");
-
-        guac_parser_free(parser);
-        return 1;
-    }
-
-    /* Validate content of size instruction */
-    if (parser->argc < 2) {
-        guac_user_log(user, GUAC_LOG_ERROR, "Received \"size\" "
-                "instruction lacked required arguments.");
-        guac_parser_free(parser);
-        return 1;
-    }
-
-    /* Parse optimal screen dimensions from size instruction */
-    user->info.optimal_width  = atoi(parser->argv[0]);
-    user->info.optimal_height = atoi(parser->argv[1]);
-
-    /* If DPI given, set the user resolution */
-    if (parser->argc >= 3)
-        user->info.optimal_resolution = atoi(parser->argv[2]);
-
-    /* Otherwise, use a safe default for rough backwards compatibility */
-    else
-        user->info.optimal_resolution = 96;
-
-    /* Get supported audio formats */
-    if (guac_parser_expect(parser, socket, usec_timeout, "audio")) {
-
-        /* Log error */
-        guac_user_log_handshake_failure(user);
-        guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
-                "Error reading \"audio\"");
-
-        guac_parser_free(parser);
-        return 1;
-    }
-
-    /* Store audio mimetypes */
-    char** audio_mimetypes = guac_copy_mimetypes(parser->argv, parser->argc);
-    user->info.audio_mimetypes = (const char**) audio_mimetypes;
-
-    /* Get supported video formats */
-    if (guac_parser_expect(parser, socket, usec_timeout, "video")) {
-
-        /* Log error */
-        guac_user_log_handshake_failure(user);
-        guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
-                "Error reading \"video\"");
-
-        guac_parser_free(parser);
-        return 1;
-    }
-
-    /* Store video mimetypes */
-    char** video_mimetypes = guac_copy_mimetypes(parser->argv, parser->argc);
-    user->info.video_mimetypes = (const char**) video_mimetypes;
-
-    /* Get supported image formats */
-    if (guac_parser_expect(parser, socket, usec_timeout, "image")) {
-
-        /* Log error */
-        guac_user_log_handshake_failure(user);
-        guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
-                "Error reading \"image\"");
-
-        guac_parser_free(parser);
-        return 1;
-    }
-
-    /* Store image mimetypes */
-    char** image_mimetypes = guac_copy_mimetypes(parser->argv, parser->argc);
-    user->info.image_mimetypes = (const char**) image_mimetypes;
-
-    /* Get args from connect instruction */
-    if (guac_parser_expect(parser, socket, usec_timeout, "connect")) {
-
-        /* Log error */
-        guac_user_log_handshake_failure(user);
-        guac_user_log_guac_error(user, GUAC_LOG_DEBUG,
-                "Error reading \"connect\"");
-
         guac_parser_free(parser);
         return 1;
     }
@@ -401,8 +326,15 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) {
     guac_protocol_send_ready(socket, client->connection_id);
     guac_socket_flush(socket);
     
-    /* Attempt join */
+    /* Verify argument count. */
-    if (guac_client_add_user(client, user, parser->argc, parser->argv))
+    if (parser->argc != (num_args + 1)) {
+        guac_client_log(client, GUAC_LOG_ERROR, "Client did not return the "
+                "expected number of arguments.");
+        return 1;
+    }
+    
+    /* Attempt to join user to connection. */
+    if (guac_client_add_user(client, user, (parser->argc - 1), parser->argv + 1))
         guac_client_log(client, GUAC_LOG_ERROR, "User \"%s\" could NOT "
                 "join connection \"%s\"", user->user_id, client->connection_id);
 
@@ -412,6 +344,12 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) {
         guac_client_log(client, GUAC_LOG_INFO, "User \"%s\" joined connection "
                 "\"%s\" (%i users now present)", user->user_id,
                 client->connection_id, client->connected_users);
+        if (strcmp(parser->argv[0],"") != 0)
+            guac_client_log(client, GUAC_LOG_DEBUG, "Client is using protocol "
+                    "version \"%s\"", parser->argv[0]);
+        else
+            guac_client_log(client, GUAC_LOG_DEBUG, "Client has not defined "
+                    "its protocol version.");
 
         /* Handle user I/O, wait for connection to terminate */
         guac_user_start(parser, user, usec_timeout);
@@ -423,10 +361,13 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) {
 
     }
     
-    /* Free mimetype lists */
+    /* Free mimetype character arrays. */
-    guac_free_mimetypes(audio_mimetypes);
+    guac_free_mimetypes((char **) user->info.audio_mimetypes);
-    guac_free_mimetypes(video_mimetypes);
+    guac_free_mimetypes((char **) user->info.image_mimetypes);
-    guac_free_mimetypes(image_mimetypes);
+    guac_free_mimetypes((char **) user->info.video_mimetypes);
+    
+    /* Free timezone info. */
+    free((char *) user->info.timezone);
     
     guac_parser_free(parser);
 

src/libguac/user.c

@@ -169,19 +169,8 @@ void guac_user_free_object(guac_user* user, guac_object* object) {
 
 int guac_user_handle_instruction(guac_user* user, const char* opcode, int argc, char** argv) {
 
-    /* For each defined instruction */
+    return __guac_user_call_opcode_handler(__guac_instruction_handler_map,
-    __guac_instruction_handler_mapping* current = __guac_instruction_handler_map;
+            user, opcode, argc, argv);
-    while (current->opcode != NULL) {
-
-        /* If recognized, call handler */
-        if (strcmp(opcode, current->opcode) == 0)
-            return current->handler(user, argc, argv);
-
-        current++;
-    }
-
-    /* If unrecognized, ignore */
-    return 0;
 
 }
 

src/protocols/rdp/rdp_settings.c

@@ -851,10 +851,10 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user,
     if (settings->server_layout == NULL)
         settings->server_layout = guac_rdp_keymap_find(GUAC_DEFAULT_KEYMAP);
 
-    /* Timezone if provied by client */
+    /* Timezone if provided by client, or use handshake version */
     settings->timezone =
         guac_user_parse_args_string(user, GUAC_RDP_CLIENT_ARGS, argv,
-                IDX_TIMEZONE, NULL);
+                IDX_TIMEZONE, user->info.timezone);
 
 #ifdef ENABLE_COMMON_SSH
     /* SFTP enable/disable */

src/protocols/ssh/settings.c

@@ -421,10 +421,10 @@ guac_ssh_settings* guac_ssh_parse_args(guac_user* user,
         guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv,
                 IDX_LOCALE, NULL);
 
-    /* Read the client timezone. */
+    /* Read the timezone parameter, or use client handshake. */
     settings->timezone =
         guac_user_parse_args_string(user, GUAC_SSH_CLIENT_ARGS, argv,
-                IDX_TIMEZONE, NULL);
+                IDX_TIMEZONE, user->info.timezone);
 
     /* Parsing was successful */
     return settings;