From c39201cd8bc6d9f7f0eec7668841879c82ec858b Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 4 Feb 2015 13:08:02 -0800 Subject: [PATCH 1/3] GUAC-803: Write to terminal STDOUT using length-prefixed packets, such that zero-length writes are legal. --- src/terminal/Makefile.am | 2 + src/terminal/common.c | 20 +++++++++ src/terminal/common.h | 21 +++++++++ src/terminal/packet.c | 67 +++++++++++++++++++++++++++++ src/terminal/packet.h | 92 ++++++++++++++++++++++++++++++++++++++++ src/terminal/terminal.c | 26 ++++++++++-- 6 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 src/terminal/packet.c create mode 100644 src/terminal/packet.h diff --git a/src/terminal/Makefile.am b/src/terminal/Makefile.am index 0fb19264..1e7ca72d 100644 --- a/src/terminal/Makefile.am +++ b/src/terminal/Makefile.am @@ -34,6 +34,7 @@ noinst_HEADERS = \ cursor.h \ display.h \ ibar.h \ + packet.h \ pointer.h \ scrollbar.h \ terminal.h \ @@ -48,6 +49,7 @@ libguac_terminal_la_SOURCES = \ cursor.c \ display.c \ ibar.c \ + packet.c \ pointer.c \ scrollbar.c \ terminal.c \ diff --git a/src/terminal/common.c b/src/terminal/common.c index 949f9704..cf69c5e7 100644 --- a/src/terminal/common.c +++ b/src/terminal/common.c @@ -108,3 +108,23 @@ int guac_terminal_write_all(int fd, const char* buffer, int size) { } +int guac_terminal_fill_buffer(int fd, char* buffer, int size) { + + int remaining = size; + while (remaining > 0) { + + /* Attempt to read data */ + int ret_val = read(fd, buffer, remaining); + if (ret_val <= 0) + return -1; + + /* If successful, continue with what space remains (if any) */ + remaining -= ret_val; + buffer += ret_val; + + } + + return size; + +} + diff --git a/src/terminal/common.h b/src/terminal/common.h index dbff68d1..7c9ced75 100644 --- a/src/terminal/common.h +++ b/src/terminal/common.h @@ -52,5 +52,26 @@ bool guac_terminal_has_glyph(int codepoint); */ int guac_terminal_write_all(int fd, const char* buffer, int size); +/** + * Similar to read, but automatically retries the read until an error occurs, + * filling all available space within the buffer. Unless it is known that the + * given amount of space is available on the file descriptor, there is a good + * chance this function will block. + * + * @param fd + * The file descriptor to read data from. + * + * @param buffer + * The buffer to store data within. + * + * @param size + * The number of bytes available within the buffer. + * + * @return + * The number of bytes read if successful, or a negative value if an error + * occurs. + */ +int guac_terminal_fill_buffer(int fd, char* buffer, int size); + #endif diff --git a/src/terminal/packet.c b/src/terminal/packet.c new file mode 100644 index 00000000..9b4ca7fb --- /dev/null +++ b/src/terminal/packet.c @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2015 Glyptodon LLC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "common.h" +#include "packet.h" + +#include + +int guac_terminal_packet_write(int fd, const void* data, int length) { + + guac_terminal_packet out; + + /* Do not attempt to write packets beyond maximum size */ + if (length > GUAC_TERMINAL_PACKET_SIZE) + return -1; + + /* Calculate final packet length */ + int packet_length = sizeof(int) + length; + + /* Copy data into packet */ + out.length = length; + memcpy(out.data, data, length); + + /* Write packet */ + return guac_terminal_write_all(fd, (const char*) &out, packet_length); + +} + +int guac_terminal_packet_read(int fd, void* data, int length) { + + int bytes; + + /* Read buffers MUST be at least GUAC_TERMINAL_PACKET_SIZE */ + if (length < GUAC_TERMINAL_PACKET_SIZE) + return -1; + + /* Read length */ + if (guac_terminal_fill_buffer(fd, (char*) &bytes, sizeof(int)) < 0) + return -1; + + /* Read body */ + if (guac_terminal_fill_buffer(fd, (char*) data, bytes) < 0) + return -1; + + return bytes; + +} + diff --git a/src/terminal/packet.h b/src/terminal/packet.h new file mode 100644 index 00000000..1220e317 --- /dev/null +++ b/src/terminal/packet.h @@ -0,0 +1,92 @@ +/* + * Copyright (C) 2015 Glyptodon LLC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef GUAC_TERMINAL_PACKET_H +#define GUAC_TERMINAL_PACKET_H + +/** + * The maximum size of a packet written or read by the + * guac_terminal_packet_write() or guac_terminal_packet_read() functions. + */ +#define GUAC_TERMINAL_PACKET_SIZE 4096 + +/** + * An arbitrary data packet with minimal framing. + */ +typedef struct guac_terminal_packet { + + /** + * The number of bytes in the data portion of this packet. + */ + int length; + + /** + * Arbitrary data. + */ + char data[GUAC_TERMINAL_PACKET_SIZE]; + +} guac_terminal_packet; + +/** + * Writes a single packet of data to the given file descriptor. The provided + * length MUST be no greater than GUAC_TERMINAL_PACKET_SIZE. Zero-length + * writes are legal and do result in a packet being written to the file + * descriptor. + * + * @param fd + * The file descriptor to write to. + * + * @param data + * A buffer containing the data to write. + * + * @param length + * The number of bytes to write to the file descriptor. + * + * @return + * The number of bytes written on success, which may be zero if the data + * length is zero, or a negative value on error. + */ +int guac_terminal_packet_write(int fd, const void* data, int length); + +/** + * Reads a single packet of data from the given file descriptor. The provided + * length MUST be at least GUAC_TERMINAL_PACKET_SIZE to ensure any packet + * read will fit in the buffer. Zero-length reads are possible if a zero-length + * packet was written. + * + * @param fd + * The file descriptor to read from. + * + * @param data + * The buffer to store data within. + * + * @param length + * The number of bytes available within the buffer. + * + * @return + * The number of bytes read on success, which may be zero if the read + * packet had a length of zero, or a negative value on error. + */ +int guac_terminal_packet_read(int fd, void* data, int length); + +#endif + diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 4f24ec5a..0a4b46c7 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -29,6 +29,7 @@ #include "display.h" #include "ibar.h" #include "guac_clipboard.h" +#include "packet.h" #include "pointer.h" #include "scrollbar.h" #include "terminal.h" @@ -324,7 +325,7 @@ void guac_terminal_free(guac_terminal* term) { int guac_terminal_render_frame(guac_terminal* terminal) { guac_client* client = terminal->client; - char buffer[8192]; + char buffer[GUAC_TERMINAL_PACKET_SIZE]; int ret_val; int fd = terminal->stdout_pipe_fd[0]; @@ -348,7 +349,8 @@ int guac_terminal_render_frame(guac_terminal* terminal) { guac_terminal_lock(terminal); /* Read data, write to terminal */ - if ((bytes_read = read(fd, buffer, sizeof(buffer))) > 0) { + if ((bytes_read = guac_terminal_packet_read(fd, + buffer, sizeof(buffer))) > 0) { if (guac_terminal_write(terminal, buffer, bytes_read)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error writing data"); @@ -382,8 +384,24 @@ int guac_terminal_read_stdin(guac_terminal* terminal, char* c, int size) { return read(stdin_fd, c, size); } -int guac_terminal_write_stdout(guac_terminal* terminal, const char* c, int size) { - return guac_terminal_write_all(terminal->stdout_pipe_fd[1], c, size); +int guac_terminal_write_stdout(guac_terminal* terminal, const char* c, + int size) { + + /* Write maximally-sized packets until only one packet remains */ + while (size > GUAC_TERMINAL_PACKET_SIZE) { + + /* Write maximally-sized packet */ + if (guac_terminal_packet_write(terminal->stdout_pipe_fd[1], c, + GUAC_TERMINAL_PACKET_SIZE) < 0) + return -1; + + /* Advance to next packet */ + c += GUAC_TERMINAL_PACKET_SIZE; + size -= GUAC_TERMINAL_PACKET_SIZE; + + } + + return guac_terminal_packet_write(terminal->stdout_pipe_fd[1], c, size); } int guac_terminal_printf(guac_terminal* terminal, const char* format, ...) { From 61337d56148380f9213539039ae07c14561362c1 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 4 Feb 2015 13:19:40 -0800 Subject: [PATCH 2/3] GUAC-803: Notify terminal of changes instead of explicitly flushing/syncing. --- src/terminal/terminal.c | 38 ++++++++++---------------------------- src/terminal/terminal.h | 13 +++++++++++++ 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 0a4b46c7..096c125b 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -404,6 +404,10 @@ int guac_terminal_write_stdout(guac_terminal* terminal, const char* c, return guac_terminal_packet_write(terminal->stdout_pipe_fd[1], c, size); } +int guac_terminal_notify(guac_terminal* terminal) { + return guac_terminal_packet_write(terminal->stdout_pipe_fd[1], NULL, 0); +} + int guac_terminal_printf(guac_terminal* terminal, const char* format, ...) { int written; @@ -711,12 +715,7 @@ void guac_terminal_scroll_display_down(guac_terminal* terminal, } - guac_terminal_display_flush(terminal->display); - guac_terminal_scrollbar_flush(terminal->scrollbar); - - guac_protocol_send_sync(terminal->client->socket, - terminal->client->last_sent_timestamp); - guac_socket_flush(terminal->client->socket); + guac_terminal_notify(terminal); } @@ -778,12 +777,7 @@ void guac_terminal_scroll_display_up(guac_terminal* terminal, } - guac_terminal_display_flush(terminal->display); - guac_terminal_scrollbar_flush(terminal->scrollbar); - - guac_protocol_send_sync(terminal->client->socket, - terminal->client->last_sent_timestamp); - guac_socket_flush(terminal->client->socket); + guac_terminal_notify(terminal); } @@ -1204,16 +1198,9 @@ int guac_terminal_resize(guac_terminal* terminal, int width, int height) { /* Reset scroll region */ terminal->scroll_end = rows - 1; - guac_terminal_flush(terminal); - } - - /* If terminal size hasn't changed, still need to flush */ - else { - guac_terminal_scrollbar_flush(terminal->scrollbar); - guac_protocol_send_sync(socket, client->last_sent_timestamp); - guac_socket_flush(socket); } + guac_terminal_notify(terminal); return 0; } @@ -1246,7 +1233,7 @@ static int __guac_terminal_send_key(guac_terminal* term, int keysym, int pressed if (term->current_cursor != term->blank_cursor) { term->current_cursor = term->blank_cursor; guac_terminal_set_cursor(term->client, term->blank_cursor); - guac_socket_flush(term->client->socket); + guac_terminal_notify(term); } /* Track modifiers */ @@ -1420,11 +1407,7 @@ static int __guac_terminal_send_mouse(guac_terminal* term, int x, int y, int mas guac_terminal_set_cursor(client, term->pointer_cursor); } - /* Flush scrollbar */ - guac_terminal_scrollbar_flush(term->scrollbar); - guac_protocol_send_sync(socket, client->last_sent_timestamp); - guac_socket_flush(socket); - + guac_terminal_notify(term); return 0; } @@ -1435,8 +1418,7 @@ static int __guac_terminal_send_mouse(guac_terminal* term, int x, int y, int mas if (term->current_cursor != term->ibar_cursor) { term->current_cursor = term->ibar_cursor; guac_terminal_set_cursor(client, term->ibar_cursor); - guac_protocol_send_sync(socket, client->last_sent_timestamp); - guac_socket_flush(socket); + guac_terminal_notify(term); } /* Paste contents of clipboard on right or middle mouse button up */ diff --git a/src/terminal/terminal.h b/src/terminal/terminal.h index 60d959d3..8722e6f1 100644 --- a/src/terminal/terminal.h +++ b/src/terminal/terminal.h @@ -367,6 +367,19 @@ int guac_terminal_read_stdin(guac_terminal* terminal, char* c, int size); */ int guac_terminal_write_stdout(guac_terminal* terminal, const char* c, int size); +/** + * Notifies the terminal that an event has occurred and the terminal should + * flush itself when reasonable. + * + * @param terminal + * The terminal to notify. + * + * @return + * Zero if notification succeeded, non-zero if an error occurred while + * notifying the terminal. + */ +int guac_terminal_notify(guac_terminal* terminal); + /** * Reads a single line from this terminal's STDIN. Input is retrieved in * the same manner as guac_terminal_read_stdin() and the same restrictions From eb9c6fb899eae6dd32b5fbb8b66775bc8ea16b17 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 4 Feb 2015 14:01:25 -0800 Subject: [PATCH 3/3] GUAC-803: Continue reading data until end of frame. --- src/terminal/terminal.c | 106 +++++++++++++++++++++++++++++----------- src/terminal/terminal.h | 11 +++++ 2 files changed, 89 insertions(+), 28 deletions(-) diff --git a/src/terminal/terminal.c b/src/terminal/terminal.c index 096c125b..67224659 100644 --- a/src/terminal/terminal.c +++ b/src/terminal/terminal.c @@ -50,6 +50,7 @@ #include #include #include +#include /** * Sets the given range of columns to the given character. @@ -322,56 +323,105 @@ void guac_terminal_free(guac_terminal* term) { } +/** + * Waits for data to become available on the given file descriptor. + * + * @param fd + * The file descriptor to wait on. + * + * @param msec_timeout + * The maximum amount of time to wait, in milliseconds. + * + * @return + * A positive if data is available, zero if the timeout has elapsed without + * data becoming available, or negative if an error occurred. + */ +static int guac_terminal_wait_for_data(int fd, int msec_timeout) { + + /* Build fd_set */ + fd_set fds; + FD_ZERO(&fds); + FD_SET(fd, &fds); + + /* Split millisecond timeout into seconds and microseconds */ + struct timeval timeout = { + .tv_sec = msec_timeout / 1000, + .tv_usec = (msec_timeout % 1000) * 1000 + }; + + /* Wait for data */ + return select(fd+1, &fds, NULL, NULL, &timeout); + +} + int guac_terminal_render_frame(guac_terminal* terminal) { guac_client* client = terminal->client; char buffer[GUAC_TERMINAL_PACKET_SIZE]; - int ret_val; + int wait_result; int fd = terminal->stdout_pipe_fd[0]; - struct timeval timeout; - fd_set fds; - - /* Build fd_set */ - FD_ZERO(&fds); - FD_SET(fd, &fds); - - /* Time to wait */ - timeout.tv_sec = 1; - timeout.tv_usec = 0; /* Wait for data to be available */ - ret_val = select(fd+1, &fds, NULL, NULL, &timeout); - if (ret_val > 0) { - - int bytes_read = 0; + wait_result = guac_terminal_wait_for_data(fd, 1000); + if (wait_result > 0) { guac_terminal_lock(terminal); + guac_timestamp frame_start = guac_timestamp_current(); - /* Read data, write to terminal */ - if ((bytes_read = guac_terminal_packet_read(fd, - buffer, sizeof(buffer))) > 0) { + do { - if (guac_terminal_write(terminal, buffer, bytes_read)) { - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error writing data"); + guac_timestamp frame_end; + int frame_remaining; + + int bytes_read; + + /* Read data, write to terminal */ + if ((bytes_read = guac_terminal_packet_read(fd, + buffer, sizeof(buffer))) > 0) { + + if (guac_terminal_write(terminal, buffer, bytes_read)) { + guac_client_abort(client, + GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Error writing data"); + guac_terminal_unlock(terminal); + return 1; + } + + } + + /* Notify on error */ + if (bytes_read < 0) { + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Error reading data"); + guac_terminal_unlock(terminal); return 1; } - } + /* Calculate time remaining in frame */ + frame_end = guac_timestamp_current(); + frame_remaining = frame_start + GUAC_TERMINAL_FRAME_DURATION + - frame_end; - /* Notify on error */ - if (bytes_read < 0) { - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error reading data"); - return 1; - } + /* Wait again if frame remaining */ + if (frame_remaining > 0) + wait_result = guac_terminal_wait_for_data(fd, + GUAC_TERMINAL_FRAME_TIMEOUT); + else + break; + + } while (wait_result > 0); /* Flush terminal */ guac_terminal_flush(terminal); guac_terminal_unlock(terminal); } - else if (ret_val < 0) { - guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, "Error waiting for data"); + + /* Notify of any errors */ + if (wait_result < 0) { + guac_client_abort(client, GUAC_PROTOCOL_STATUS_SERVER_ERROR, + "Error waiting for data"); return 1; } diff --git a/src/terminal/terminal.h b/src/terminal/terminal.h index 8722e6f1..7c6b7507 100644 --- a/src/terminal/terminal.h +++ b/src/terminal/terminal.h @@ -39,6 +39,17 @@ #include #include +/** + * The maximum duration of a single frame, in milliseconds. + */ +#define GUAC_TERMINAL_FRAME_DURATION 40 + +/** + * The maximum amount of time to wait for more data before declaring a frame + * complete, in milliseconds. + */ +#define GUAC_TERMINAL_FRAME_TIMEOUT 10 + /** * The maximum number of custom tab stops. */