From 2f57564f5d2ed7e8a41179000178bcf6ec4e5487 Mon Sep 17 00:00:00 2001 From: Nick Couchman Date: Tue, 2 Apr 2019 21:38:28 -0400 Subject: [PATCH] GUACAMOLE-422: Remove duplicate code and migrate handshake to user handlers. --- src/libguac/guacamole/user.h | 99 ------------------- src/libguac/user-handlers.c | 131 ++++++++++++++++++++++++++ src/libguac/user-handlers.h | 101 ++++++++++++++++++++ src/libguac/user-handshake.c | 178 +++-------------------------------- src/libguac/user.c | 18 ---- 5 files changed, 245 insertions(+), 282 deletions(-) diff --git a/src/libguac/guacamole/user.h b/src/libguac/guacamole/user.h index 137553a7..67ca7c88 100644 --- a/src/libguac/guacamole/user.h +++ b/src/libguac/guacamole/user.h @@ -505,79 +505,6 @@ struct guac_user { }; -/** - * Handler for Guacamole protocol opcode specific to the handshake that - * happens between client and server at the beginning of the connection. The - * handler will be invoked when the matching opcode is received during the - * handshake process. - * - * @param user - * The user that initiated the handshake. - * - * @param parser - * The parser allocated for parsing the data provided by the client. - * - * @param timeout - * The timeout, in microseconds, for parsing the value. - * - * @return - * Zero if the handshake instruction is successfully parsed; otherwise - * false. - */ -typedef int __guac_handshake_handler(guac_user* user, int argc, char** argv); - -/** - * Structure that maps opcodes received during the handshake phase of the - * connection to callback functions used when those opcodes are received. - */ -typedef struct __guac_handshake_mapping { - - /** - * The instruction opcode which maps to the handler. - */ - char* opcode; - - /** - * The handler function used when specified opcode is received. - */ - __guac_handshake_handler* handler; - -} __guac_handshake_mapping; - -/** - * Internal handler function that is called when the size instruction is - * received during the handshake process. - */ -__guac_handshake_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_handshake_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_handshake_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_handshake_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_handshake_handler __guac_handshake_timezone_handler; - /** * Allocates a new, blank user, not associated with any specific client or * socket. @@ -617,32 +544,6 @@ void guac_user_free(guac_user* user); */ 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 - * will in turn call the user's handler (if defined). - * - * @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 - * Non-negative if the instruction was handled successfully, or negative - * if an error occurred. - */ -int guac_user_handle_instruction(guac_user* user, const char* opcode, - int argc, char** argv); - /** * Allocates a new stream. An arbitrary index is automatically assigned * if no previously-allocated stream is available for use. diff --git a/src/libguac/user-handlers.c b/src/libguac/user-handlers.c index 6be20e2e..737e6f30 100644 --- a/src/libguac/user-handlers.c +++ b/src/libguac/user-handlers.c @@ -31,6 +31,7 @@ #include #include #include +#include /* 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,122 @@ 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) { + + /* 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) { + + /* 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) { + + /* 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]); + + 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) { + + 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_handle_instruction(__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, ignore */ + return 0; + +} + diff --git a/src/libguac/user-handlers.h b/src/libguac/user-handlers.h index 5d7c6eae..1a6477ed 100644 --- a/src/libguac/user-handlers.h +++ b/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,71 @@ __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 user-handlers.c. The intial handlers + * will in turn call the user's handler (if defined). + * + * @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 + * Non-negative if the instruction was handled successfully, or negative + * if an error occurred. + */ +int guac_user_handle_instruction(__guac_instruction_handler_mapping* map, + guac_user* user, const char* opcode, int argc, char** argv); + #endif diff --git a/src/libguac/user-handshake.c b/src/libguac/user-handshake.c index e9ccbacd..9cccec4c 100644 --- a/src/libguac/user-handshake.c +++ b/src/libguac/user-handshake.c @@ -113,141 +113,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); - -} - -/* 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) { - - /* 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) { - - /* 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) { - - /* 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]); - - return 0; - -} - -/* Guacamole handshake handler mappings. */ -__guac_handshake_mapping __guac_handshake_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} -}; - /** * The thread which handles all user input, calling event handlers for received * instructions. @@ -296,7 +161,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_handle_instruction(__guac_instruction_handler_map, + user, parser->opcode, parser->argc, parser->argv) < 0) { /* Log error */ guac_user_log_guac_error(user, GUAC_LOG_WARNING, @@ -399,39 +265,21 @@ int guac_user_handle_connection(guac_user* user, int usec_timeout) { guac_user_log(user, GUAC_LOG_DEBUG, "Processing instruction: %s", parser->opcode); - /* Loop available opcodes and run handler if/when match found. */ - __guac_handshake_mapping* current = __guac_handshake_map; - while (current->opcode != NULL) { + /* Run instruction handler for opcode with arguments. */ + if (guac_user_handle_instruction(__guac_handshake_handler_map, user, + parser->opcode, parser->argc, parser->argv)) { - /* Check if loop opcode matches parsed opcode. */ - if (strcmp(parser->opcode, current->opcode) == 0) { - - /* If calling the handler fails, log it and return. */ - if (current->handler(user, parser->argc, parser->argv)) { - - guac_user_log_handshake_failure(user); - guac_user_log_guac_error(user, GUAC_LOG_DEBUG, - "Error handling handling opcode during handshake."); - guac_user_log(user, GUAC_LOG_DEBUG, "Failed opcode: %s", - current->opcode); + guac_user_log_handshake_failure(user); + guac_user_log_guac_error(user, GUAC_LOG_DEBUG, + "Error handling handling opcode during handshake."); + guac_user_log(user, GUAC_LOG_DEBUG, "Failed opcode: %s", + parser->opcode); - guac_parser_free(parser); - return 1; - } - - /* If calling the handler has succeeded, log it and break. */ - else { - guac_user_log(user, GUAC_LOG_DEBUG, - "Successfully processed instruction: \"%s\"", - current->opcode); - break; - } - - } + guac_parser_free(parser); + return 1; - /* Move to next opcode. */ - current++; } + } /* Acknowledge connection availability */ diff --git a/src/libguac/user.c b/src/libguac/user.c index 16590060..64c2a763 100644 --- a/src/libguac/user.c +++ b/src/libguac/user.c @@ -167,24 +167,6 @@ 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 */ - __guac_instruction_handler_mapping* current = __guac_instruction_handler_map; - 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; - -} - void guac_user_stop(guac_user* user) { user->active = 0; }