[Spice-devel] [PATCH spice-server 22/23] websocket: Handle continuation and 0-size frames
Frediano Ziglio
fziglio at redhat.com
Tue Jun 25 16:11:46 UTC 2019
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 at redhat.com>
---
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
More information about the Spice-devel
mailing list