The WebSocket protocol allows 0-size frames so a returned lenth of 0 does not only mean an issue but it's perfectly expected. This is also required by WebSocket specification. Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/tests/test-websocket.c | 25 +++++++++++------ server/websocket.c | 53 ++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/server/tests/test-websocket.c b/server/tests/test-websocket.c index 120a522f1..dc7b7d342 100644 --- a/server/tests/test-websocket.c +++ b/server/tests/test-websocket.c @@ -219,21 +219,28 @@ handle_client(int new_sock) assert(ws); char buffer[4096]; + bool got_message = false; size_t to_send = 0; unsigned ws_flags = WEBSOCKET_BINARY_FINAL; while (!got_term) { int events = 0; - if (sizeof(buffer) > to_send) { + if (sizeof(buffer) > to_send && + (!got_message || (ws_flags & WEBSOCKET_FINAL) == 0)) { events |= POLLIN; } - if (to_send) { + assert(!to_send || got_message); + if (got_message) { events |= POLLOUT; } events = wait_for(new_sock, events); if (events & POLLIN) { assert(sizeof(buffer) > to_send); + unsigned flags; int size = websocket_read(ws, (void *) (buffer + to_send), sizeof(buffer) - to_send, - &ws_flags); + &flags); + if (flags) { + ws_flags = flags; + } if (size < 0) { if (errno == EIO) { @@ -245,14 +252,15 @@ handle_client(int new_sock) err(1, "recv"); } - if (size == 0) { + if (size == 0 && flags == 0) { break; } if (debug) { - printf("received %d bytes of data\n", size); + printf("received %d bytes of data flags %x\n", size, flags); } to_send += size; + got_message = true; } if (events & POLLOUT) { @@ -275,12 +283,11 @@ handle_client(int new_sock) printf("sent %d bytes of data\n", size); } - if (size == 0) { - errx(1, "Unexpected short write\n"); - } - to_send -= size; memmove(buffer, buffer + size, to_send); + if (!to_send) { + got_message = false; + } } } diff --git a/server/websocket.c b/server/websocket.c index e076645d3..f5df63f8b 100644 --- a/server/websocket.c +++ b/server/websocket.c @@ -83,7 +83,7 @@ typedef struct { } WebSocketControl; typedef struct { - uint8_t type; + uint8_t type, fin, unfinished; uint8_t header[WEBSOCKET_MAX_HEADER_SIZE]; int header_pos; bool frame_ready:1; @@ -100,6 +100,7 @@ struct RedsWebSocket { uint64_t write_remainder; uint8_t write_header[WEBSOCKET_MAX_HEADER_SIZE]; uint8_t write_header_pos, write_header_len; + bool send_unfinished; bool close_pending; WebSocketControl pong; WebSocketControl pending_pong; @@ -247,7 +248,9 @@ static char *generate_reply_key(char *buf) static void websocket_clear_frame(websocket_frame_t *frame) { + uint8_t unfinished = frame->unfinished; memset(frame, 0, sizeof(*frame)); + frame->unfinished = unfinished; } /* Extract a frame header of data from a set of data transmitted by @@ -261,7 +264,7 @@ static bool websocket_get_frame_header(websocket_frame_t *frame) return true; } - fin = frame->header[0] & FIN_FLAG; + fin = frame->fin = frame->header[0] & FIN_FLAG; frame->type = frame->header[0] & TYPE_MASK; used++; @@ -282,8 +285,16 @@ static bool websocket_get_frame_header(websocket_frame_t *frame) /* This is a Spice specific optimization. We don't really care about assembling frames fully, so we treat a frame in process as a finished frame and pass it along. */ - if (!fin && frame->type == CONTINUATION_FRAME) { - frame->type = BINARY_FRAME; + if ((frame->type & CONTROL_FRAME_MASK) == 0) { + if (frame->type == CONTINUATION_FRAME) { + if (!frame->unfinished) { + return false; + } + frame->type = frame->unfinished; + } else if (frame->unfinished) { + return false; + } + frame->unfinished = fin ? 0 : frame->type; } frame->expected_len = extract_length(frame->header + used, &used); @@ -358,18 +369,21 @@ int websocket_read(RedsWebSocket *ws, uint8_t *buf, size_t size, unsigned *flags send_pending_data(ws); return 0; } else if (frame->type == BINARY_FRAME || frame->type == TEXT_FRAME) { - rc = ws->raw_read(ws->raw_stream, buf, - MIN(size, frame->expected_len - frame->relayed)); - if (rc <= 0) { - goto read_error; + rc = 0; + if (frame->expected_len > frame->relayed) { + rc = ws->raw_read(ws->raw_stream, buf, + MIN(size, frame->expected_len - frame->relayed)); + if (rc <= 0) { + goto read_error; + } + + relay_data(buf, rc, frame); + n += rc; + buf += rc; + size -= rc; } *flags = frame->type; - - relay_data(buf, rc, frame); - n += rc; - buf += rc; - size -= rc; } else if (frame->type == PING_FRAME) { spice_assert(ws->pong.data_len == frame->expected_len); rc = 0; @@ -409,8 +423,11 @@ int websocket_read(RedsWebSocket *ws, uint8_t *buf, size_t size, unsigned *flags } frame->relayed += rc; if (frame->relayed >= frame->expected_len) { + if (*flags) { + *flags |= frame->fin; + } websocket_clear_frame(frame); - if (n) { + if (*flags) { break; } } @@ -433,8 +450,8 @@ static int fill_header(uint8_t *header, uint64_t len, uint8_t type) int used = 0; int i; - type &= TYPE_MASK; - header[0] = FIN_FLAG | (type ? type : BINARY_FRAME); + type &= FIN_FLAG|TYPE_MASK; + header[0] = type; used++; header[1] = 0; @@ -512,7 +529,11 @@ static int send_data_header(RedsWebSocket *ws, uint64_t len, uint8_t type) /* fill a new header */ ws->write_header_pos = 0; + if (ws->send_unfinished) { + type &= FIN_FLAG; + } ws->write_header_len = fill_header(ws->write_header, len, type); + ws->send_unfinished = (type & FIN_FLAG) == 0; return send_data_header_left(ws); } -- 2.20.1 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel