From 653751e2f55e203b8e939abe6d27f3421366497a Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 24 Sep 2015 12:30:56 -0700 Subject: [PATCH] GUAC-1305: Copy mimetypes into client structure. Instruction contents CANNOT be relied upon after new instruction data is read (the argv, etc. buffers are reused and shared). --- src/guacd/daemon.c | 153 +++++++++++++++++++++++++++++++-------------- 1 file changed, 106 insertions(+), 47 deletions(-) diff --git a/src/guacd/daemon.c b/src/guacd/daemon.c index 89c607f9..cb791122 100644 --- a/src/guacd/daemon.c +++ b/src/guacd/daemon.c @@ -79,6 +79,64 @@ static void guacd_log_handshake_failure() { } +/** + * 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 guacd_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** guacd_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 guacd_copy_mimetypes(). + * + * @param mimetypes + * The NULL-terminated array of mimetypes to free. This array MUST have + * been previously allocated with guacd_copy_mimetypes(). + */ +static void guacd_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); + +} + /** * Creates a new guac_client for the connection on the given socket, adding * it to the client map based on its ID. @@ -164,6 +222,14 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) return; } + /* Get client */ + client = guac_client_alloc(); + if (client == NULL) { + guacd_log_guac_error(GUAC_LOG_ERROR, "Unable to create client"); + guac_socket_free(socket); + return; + } + /* Get optimal screen size */ size = guac_instruction_expect( socket, GUACD_USEC_TIMEOUT, "size"); @@ -174,10 +240,25 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) guacd_log_guac_error(GUAC_LOG_DEBUG, "Error reading \"size\""); /* Free resources */ + guac_client_free(client); guac_socket_free(socket); return; } + /* Parse optimal screen dimensions from size instruction */ + client->info.optimal_width = atoi(size->argv[0]); + client->info.optimal_height = atoi(size->argv[1]); + + /* If DPI given, set the client resolution */ + if (size->argc >= 3) + client->info.optimal_resolution = atoi(size->argv[2]); + + /* Otherwise, use a safe default for rough backwards compatibility */ + else + client->info.optimal_resolution = 96; + + guac_instruction_free(size); + /* Get supported audio formats */ audio = guac_instruction_expect( socket, GUACD_USEC_TIMEOUT, "audio"); @@ -188,10 +269,17 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) guacd_log_guac_error(GUAC_LOG_DEBUG, "Error reading \"audio\""); /* Free resources */ + guac_client_free(client); guac_socket_free(socket); return; } + /* Store audio mimetypes */ + client->info.audio_mimetypes = + guacd_copy_mimetypes(audio->argv, audio->argc); + + guac_instruction_free(audio); + /* Get supported video formats */ video = guac_instruction_expect( socket, GUACD_USEC_TIMEOUT, "video"); @@ -202,10 +290,17 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) guacd_log_guac_error(GUAC_LOG_DEBUG, "Error reading \"video\""); /* Free resources */ + guac_client_free(client); guac_socket_free(socket); return; } + /* Store video mimetypes */ + client->info.video_mimetypes = + guacd_copy_mimetypes(video->argv, video->argc); + + guac_instruction_free(video); + /* Get supported image formats */ image = guac_instruction_expect( socket, GUACD_USEC_TIMEOUT, "image"); @@ -216,10 +311,17 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) guacd_log_guac_error(GUAC_LOG_DEBUG, "Error reading \"image\""); /* Free resources */ + guac_client_free(client); guac_socket_free(socket); return; } + /* Store image mimetypes */ + client->info.image_mimetypes = + guacd_copy_mimetypes(image->argv, image->argc); + + guac_instruction_free(image); + /* Get args from connect instruction */ connect = guac_instruction_expect( socket, GUACD_USEC_TIMEOUT, "connect"); @@ -233,14 +335,7 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) guacd_log_guac_error(GUAC_LOG_WARNING, "Unable to close client plugin"); - guac_socket_free(socket); - return; - } - - /* Get client */ - client = guac_client_alloc(); - if (client == NULL) { - guacd_log_guac_error(GUAC_LOG_ERROR, "Unable to create client"); + guac_client_free(client); guac_socket_free(socket); return; } @@ -248,36 +343,6 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) client->socket = socket; client->log_handler = guacd_client_log; - /* Parse optimal screen dimensions from size instruction */ - client->info.optimal_width = atoi(size->argv[0]); - client->info.optimal_height = atoi(size->argv[1]); - - /* If DPI given, set the client resolution */ - if (size->argc >= 3) - client->info.optimal_resolution = atoi(size->argv[2]); - - /* Otherwise, use a safe default for rough backwards compatibility */ - else - client->info.optimal_resolution = 96; - - /* Store audio mimetypes */ - client->info.audio_mimetypes = malloc(sizeof(char*) * (audio->argc+1)); - memcpy(client->info.audio_mimetypes, audio->argv, - sizeof(char*) * audio->argc); - client->info.audio_mimetypes[audio->argc] = NULL; - - /* Store video mimetypes */ - client->info.video_mimetypes = malloc(sizeof(char*) * (video->argc+1)); - memcpy(client->info.video_mimetypes, video->argv, - sizeof(char*) * video->argc); - client->info.video_mimetypes[video->argc] = NULL; - - /* Store image mimetypes */ - client->info.image_mimetypes = malloc(sizeof(char*) * (image->argc+1)); - memcpy(client->info.image_mimetypes, image->argv, - sizeof(char*) * image->argc); - client->info.image_mimetypes[image->argc] = NULL; - /* Store client */ if (guacd_client_map_add(map, client)) guacd_log(GUAC_LOG_ERROR, "Unable to add client. Internal client storage has failed"); @@ -319,15 +384,9 @@ static void guacd_handle_connection(guacd_client_map* map, guac_socket* socket) guacd_log(GUAC_LOG_ERROR, "Unable to remove client. Internal client storage has failed"); /* Free mimetype lists */ - free(client->info.audio_mimetypes); - free(client->info.video_mimetypes); - free(client->info.image_mimetypes); - - /* Free remaining instructions */ - guac_instruction_free(audio); - guac_instruction_free(video); - guac_instruction_free(image); - guac_instruction_free(size); + guacd_free_mimetypes(client->info.audio_mimetypes); + guacd_free_mimetypes(client->info.video_mimetypes); + guacd_free_mimetypes(client->info.image_mimetypes); /* Clean up */ guac_client_free(client);