[PATCH spice-server 22/23] websocket: Handle continuation and 0-size frames

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]