From 9a3a1bdcde4fc3ffa21c87ab7cb9232e44cc93d5 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 24 Sep 2015 12:21:15 -0700 Subject: [PATCH 1/2] GUAC-1305: Mimetype pointers need not be const. --- src/libguac/client.c | 2 +- src/libguac/guacamole/client.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libguac/client.c b/src/libguac/client.c index fa75249d..949bc26f 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -451,7 +451,7 @@ void guac_client_stream_webp(guac_client* client, guac_socket* socket, int guac_client_supports_webp(guac_client* client) { #ifdef ENABLE_WEBP - const char** mimetype = client->info.image_mimetypes; + char** mimetype = client->info.image_mimetypes; /* Search for WebP mimetype in list of supported image mimetypes */ while (*mimetype != NULL) { diff --git a/src/libguac/guacamole/client.h b/src/libguac/guacamole/client.h index d385976b..be7842ef 100644 --- a/src/libguac/guacamole/client.h +++ b/src/libguac/guacamole/client.h @@ -65,20 +65,20 @@ struct guac_client_info { * NULL-terminated array of client-supported audio mimetypes. If the client * does not support audio at all, this will be NULL. */ - const char** audio_mimetypes; + char** audio_mimetypes; /** * NULL-terminated array of client-supported video mimetypes. If the client * does not support video at all, this will be NULL. */ - const char** video_mimetypes; + char** video_mimetypes; /** * NULL-terminated array of client-supported image mimetypes. Though all * supported image mimetypes will be listed here, it can be safely assumed * that all clients will support at least "image/png" and "image/jpeg". */ - const char** image_mimetypes; + char** image_mimetypes; /** * The DPI of the physical remote display if configured for the optimal From 653751e2f55e203b8e939abe6d27f3421366497a Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 24 Sep 2015 12:30:56 -0700 Subject: [PATCH 2/2] 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);