]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
ws: fix and extend CURLWS_CONT handling
authorCalvin Ruocco <calvin.ruocco@vector.com>
Mon, 10 Mar 2025 16:19:58 +0000 (17:19 +0100)
committerDaniel Stenberg <daniel@haxx.se>
Fri, 14 Mar 2025 10:46:36 +0000 (11:46 +0100)
Follow-up to fa3d1e7d43bf0e4f589aeae737

Add test 2311 to verify

Closes #16687

docs/libcurl/curl_ws_recv.md
docs/libcurl/curl_ws_send.md
lib/ws.c
lib/ws.h
tests/data/test2311
tests/libtest/lib2311.c

index b126305833c030f194fe05618cbca73a0bdb0bf8..2409335f0d87033de671137338f682bc9b5e0f2d 100644 (file)
@@ -52,9 +52,13 @@ pointer is permitted in this case. Note that frames without payload are consumed
 by this action.
 
 If the received message consists of multiple fragments, the *CURLWS_CONT* bit
-is set in all frames except the final one. The application is responsible for
-reassembling fragmented messages. See curl_ws_meta(3) for more details on
-*CURLWS_CONT*.
+is set in all frames except the final one. The appropriate *CURLWS_TEXT* or
+*CURLWS_BINARY* flag is set in every frame, regardless whether it is the first
+fragment, an intermediate fragment or the final fragment. The application is
+responsible for reassembling fragmented messages. Special care must be taken
+to correctly handle control frames (i.e. CLOSE, PING and PONG) arriving in
+between consecutive fragments of a fragmented TEXT or BINARY message. See
+curl_ws_meta(3) for more details on *CURLWS_CONT*.
 
 # %PROTOCOLS%
 
index 852cbf3d2a30eb99f8f1a670e340ec98bfaaa56f..757059fbd48a1ad93c54086ce026a922e8b5579b 100644 (file)
@@ -49,7 +49,10 @@ in the section on *CURLWS_OFFSET* below.
 
 *flags* must contain at least one flag indicating the type of the message.
 To send a fragmented message consisting of multiple frames, additionally set
-the *CURLWS_CONT* bit in all frames except the final one.
+the *CURLWS_CONT* bit in all frames except the final one. The appropriate
+message type bit should be set in every frame of a fragmented message without
+exemption. Omitting the message type for continuation frames of a fragmented
+message is only supported for backwards compatibility and highly discouraged.
 
 For more details on the supported flags see below and in curl_ws_meta(3).
 
index 372338fcbdd65b0f15cdcaab172540c6b1f12e00..d6ba74a440961fefbc0c1508e555a430f2357553 100644 (file)
--- a/lib/ws.c
+++ b/lib/ws.c
 #include "memdebug.h"
 
 
-#define WSBIT_FIN 0x80
-#define WSBIT_RSV1 0x40
-#define WSBIT_RSV2 0x20
-#define WSBIT_RSV3 0x10
+/***
+    RFC 6455 Section 5.2
+
+      0 1 2 3 4 5 6 7
+     +-+-+-+-+-------+
+     |F|R|R|R| opcode|
+     |I|S|S|S|  (4)  |
+     |N|V|V|V|       |
+     | |1|2|3|       |
+*/
+#define WSBIT_FIN  (0x80)
+#define WSBIT_RSV1 (0x40)
+#define WSBIT_RSV2 (0x20)
+#define WSBIT_RSV3 (0x10)
 #define WSBIT_RSV_MASK (WSBIT_RSV1 | WSBIT_RSV2 | WSBIT_RSV3)
-#define WSBIT_OPCODE_CONT  0
-#define WSBIT_OPCODE_TEXT  (1)
-#define WSBIT_OPCODE_BIN   (2)
-#define WSBIT_OPCODE_CLOSE (8)
-#define WSBIT_OPCODE_PING  (9)
+#define WSBIT_OPCODE_CONT  (0x0)
+#define WSBIT_OPCODE_TEXT  (0x1)
+#define WSBIT_OPCODE_BIN   (0x2)
+#define WSBIT_OPCODE_CLOSE (0x8)
+#define WSBIT_OPCODE_PING  (0x9)
 #define WSBIT_OPCODE_PONG  (0xa)
 #define WSBIT_OPCODE_MASK  (0xf)
 
 #define WS_CHUNK_SIZE 65535
 #define WS_CHUNK_COUNT 2
 
-struct ws_frame_meta {
-  char proto_opcode;
-  int flags;
-  const char *name;
-};
-
-static struct ws_frame_meta WS_FRAMES[] = {
-  { WSBIT_OPCODE_CONT,  CURLWS_CONT,   "CONT" },
-  { WSBIT_OPCODE_TEXT,  CURLWS_TEXT,   "TEXT" },
-  { WSBIT_OPCODE_BIN,   CURLWS_BINARY, "BIN" },
-  { WSBIT_OPCODE_CLOSE, CURLWS_CLOSE,  "CLOSE" },
-  { WSBIT_OPCODE_PING,  CURLWS_PING,   "PING" },
-  { WSBIT_OPCODE_PONG,  CURLWS_PONG,   "PONG" },
-};
-
-static const char *ws_frame_name_of_op(unsigned char proto_opcode)
+static const char *ws_frame_name_of_op(unsigned char firstbyte)
 {
-  unsigned char opcode = proto_opcode & WSBIT_OPCODE_MASK;
-  size_t i;
-  for(i = 0; i < CURL_ARRAYSIZE(WS_FRAMES); ++i) {
-    if(WS_FRAMES[i].proto_opcode == opcode)
-      return WS_FRAMES[i].name;
+  switch(firstbyte & WSBIT_OPCODE_MASK) {
+    case WSBIT_OPCODE_CONT:
+      return "CONT";
+    case WSBIT_OPCODE_TEXT:
+      return "TEXT";
+    case WSBIT_OPCODE_BIN:
+      return "BIN";
+    case WSBIT_OPCODE_CLOSE:
+      return "CLOSE";
+    case WSBIT_OPCODE_PING:
+      return "PING";
+    case WSBIT_OPCODE_PONG:
+      return "PONG";
+    default:
+      return "???";
   }
-  return "???";
 }
 
-static int ws_frame_op2flags(unsigned char proto_opcode)
+static int ws_frame_firstbyte2flags(struct Curl_easy *data,
+                                    unsigned char firstbyte, int cont_flags)
 {
-  unsigned char opcode = proto_opcode & WSBIT_OPCODE_MASK;
-  size_t i;
-  for(i = 0; i < CURL_ARRAYSIZE(WS_FRAMES); ++i) {
-    if(WS_FRAMES[i].proto_opcode == opcode)
-      return WS_FRAMES[i].flags;
+  switch(firstbyte) {
+    case WSBIT_OPCODE_CONT:
+      /* continuation of a previous fragment: restore stored flags */
+      return cont_flags | CURLWS_CONT;
+    case (WSBIT_OPCODE_CONT | WSBIT_FIN):
+      /* continuation of a previous fragment: restore stored flags */
+      return cont_flags & ~CURLWS_CONT;
+    case WSBIT_OPCODE_TEXT:
+      return CURLWS_TEXT | CURLWS_CONT;
+    case (WSBIT_OPCODE_TEXT | WSBIT_FIN):
+      return CURLWS_TEXT;
+    case WSBIT_OPCODE_BIN:
+      return CURLWS_BINARY | CURLWS_CONT;
+    case (WSBIT_OPCODE_BIN | WSBIT_FIN):
+      return CURLWS_BINARY;
+    case (WSBIT_OPCODE_CLOSE | WSBIT_FIN):
+      return CURLWS_CLOSE;
+    case (WSBIT_OPCODE_PING | WSBIT_FIN):
+      return CURLWS_PING;
+    case (WSBIT_OPCODE_PONG | WSBIT_FIN):
+      return CURLWS_PONG;
+    default:
+      if(firstbyte & WSBIT_RSV_MASK) {
+        failf(data, "WS: unknown reserved bit: %x",
+              firstbyte & WSBIT_RSV_MASK);
+      }
+      else {
+        failf(data, "WS: unknown opcode: %x",
+              firstbyte & WSBIT_OPCODE_MASK);
+      }
+      return 0;
   }
-  return 0;
 }
 
-static unsigned char ws_frame_flags2op(int flags)
+static unsigned char ws_frame_flags2firstbyte(struct Curl_easy *data,
+                                              unsigned int flags,
+                                              bool contfragment,
+                                              CURLcode *err)
 {
-  size_t i;
-  for(i = 0; i < CURL_ARRAYSIZE(WS_FRAMES); ++i) {
-    if(WS_FRAMES[i].flags & flags)
-      return (unsigned char)WS_FRAMES[i].proto_opcode;
+  switch(flags & ~CURLWS_OFFSET) {
+    case 0:
+      if(contfragment) {
+        infof(data, "WS: no flags given; interpreting as continuation "
+                    "fragment for compatibility");
+        return (WSBIT_OPCODE_CONT | WSBIT_FIN);
+      }
+      failf(data, "WS: no flags given");
+      *err = CURLE_BAD_FUNCTION_ARGUMENT;
+      return 0xff;
+    case CURLWS_CONT:
+      if(contfragment) {
+        infof(data, "WS: setting CURLWS_CONT flag without message type is "
+                    "supported for compatibility but highly discouraged");
+        return WSBIT_OPCODE_CONT;
+      }
+      failf(data, "WS: No ongoing fragmented message to continue");
+      *err = CURLE_BAD_FUNCTION_ARGUMENT;
+      return 0xff;
+    case CURLWS_TEXT:
+      return contfragment ? (WSBIT_OPCODE_CONT | WSBIT_FIN)
+                          : (WSBIT_OPCODE_TEXT | WSBIT_FIN);
+    case (CURLWS_TEXT | CURLWS_CONT):
+      return contfragment ? WSBIT_OPCODE_CONT : WSBIT_OPCODE_TEXT;
+    case CURLWS_BINARY:
+      return contfragment ? (WSBIT_OPCODE_CONT | WSBIT_FIN)
+                          : (WSBIT_OPCODE_BIN | WSBIT_FIN);
+    case (CURLWS_BINARY | CURLWS_CONT):
+      return contfragment ? WSBIT_OPCODE_CONT : WSBIT_OPCODE_BIN;
+    case CURLWS_CLOSE:
+      return WSBIT_OPCODE_CLOSE | WSBIT_FIN;
+    case (CURLWS_CLOSE | CURLWS_CONT):
+      failf(data, "WS: CLOSE frame must not be fragmented");
+      *err = CURLE_BAD_FUNCTION_ARGUMENT;
+      return 0xff;
+    case CURLWS_PING:
+      return WSBIT_OPCODE_PING | WSBIT_FIN;
+    case (CURLWS_PING | CURLWS_CONT):
+      failf(data, "WS: PING frame must not be fragmented");
+      *err = CURLE_BAD_FUNCTION_ARGUMENT;
+      return 0xff;
+    case CURLWS_PONG:
+      return WSBIT_OPCODE_PONG | WSBIT_FIN;
+    case (CURLWS_PONG | CURLWS_CONT):
+      failf(data, "WS: PONG frame must not be fragmented");
+      *err = CURLE_BAD_FUNCTION_ARGUMENT;
+      return 0xff;
+    default:
+      failf(data, "WS: unknown flags: %x", flags);
+      *err = CURLE_SEND_ERROR;
+      return 0xff;
   }
-  return 0;
-}
-
-/* No extensions are supported. If any of the RSV bits are set, we must fail */
-static bool ws_frame_rsv_supported(int flags)
-{
-  unsigned char reserved_bits = flags & WSBIT_RSV_MASK;
-  return reserved_bits == 0;
 }
 
 static void ws_dec_info(struct ws_decoder *dec, struct Curl_easy *data,
@@ -159,6 +231,16 @@ typedef ssize_t ws_write_payload(const unsigned char *buf, size_t buflen,
                                  void *userp,
                                  CURLcode *err);
 
+static void ws_dec_next_frame(struct ws_decoder *dec)
+{
+  dec->frame_age = 0;
+  dec->frame_flags = 0;
+  dec->payload_offset = 0;
+  dec->payload_len = 0;
+  dec->head_len = dec->head_total = 0;
+  dec->state = WS_DEC_INIT;
+  /* dec->cont_flags must be carried over to next frame */
+}
 
 static void ws_dec_reset(struct ws_decoder *dec)
 {
@@ -168,7 +250,7 @@ static void ws_dec_reset(struct ws_decoder *dec)
   dec->payload_len = 0;
   dec->head_len = dec->head_total = 0;
   dec->state = WS_DEC_INIT;
-  dec->fin_bit = TRUE;
+  dec->cont_flags = 0;
 }
 
 static void ws_dec_init(struct ws_decoder *dec)
@@ -188,22 +270,21 @@ static CURLcode ws_dec_read_head(struct ws_decoder *dec,
       dec->head[0] = *inbuf;
       Curl_bufq_skip(inraw, 1);
 
-      dec->fin_bit = (dec->head[0] & WSBIT_FIN) ? TRUE : FALSE;
-
-      if(!ws_frame_rsv_supported(dec->head[0])) {
-        failf(data, "WS: unknown reserved bit in frame header: %x",
-              dec->head[0] & WSBIT_RSV_MASK);
+      dec->frame_flags = ws_frame_firstbyte2flags(data, dec->head[0],
+                                                  dec->cont_flags);
+      if(!dec->frame_flags) {
+        failf(data, "WS: invalid first byte: %x", dec->head[0]);
         ws_dec_reset(dec);
         return CURLE_RECV_ERROR;
       }
 
-      dec->frame_flags = ws_frame_op2flags(dec->head[0]);
-      if(!dec->frame_flags) {
-        failf(data, "WS: unknown opcode: %x",
-              dec->head[0] & WSBIT_OPCODE_MASK);
-        ws_dec_reset(dec);
-        return CURLE_RECV_ERROR;
+      if(dec->frame_flags & CURLWS_CONT) {
+        dec->cont_flags = dec->frame_flags;
+      }
+      else {
+        dec->cont_flags = 0;
       }
+
       dec->head_len = 1;
       /* ws_dec_info(dec, data, "seeing opcode"); */
       continue;
@@ -324,7 +405,7 @@ static CURLcode ws_dec_pass(struct ws_decoder *dec,
 
   switch(dec->state) {
   case WS_DEC_INIT:
-    ws_dec_reset(dec);
+    ws_dec_next_frame(dec);
     dec->state = WS_DEC_HEAD;
     FALLTHROUGH();
   case WS_DEC_HEAD:
@@ -380,13 +461,6 @@ static void update_meta(struct websocket *ws,
   ws->frame.offset = payload_offset;
   ws->frame.len = cur_len;
   ws->frame.bytesleft = bytesleft;
-
-  if(!ws->dec.fin_bit || bytesleft > 0) {
-    ws->frame.flags |= CURLWS_CONT;
-  }
-  else {
-    ws->frame.flags &= ~CURLWS_CONT;
-  }
 }
 
 /* WebSockets decoding client writer */
@@ -524,10 +598,8 @@ static const struct Curl_cwtype ws_cw_decode = {
 static void ws_enc_info(struct ws_encoder *enc, struct Curl_easy *data,
                         const char *msg)
 {
-  infof(data, "WS-ENC: %s [%s%s%s payload=%" FMT_OFF_T "/%" FMT_OFF_T "]",
+  infof(data, "WS-ENC: %s [%s%s payload=%" FMT_OFF_T "/%" FMT_OFF_T "]",
         msg, ws_frame_name_of_op(enc->firstbyte),
-        (enc->firstbyte & WSBIT_OPCODE_MASK) == WSBIT_OPCODE_CONT ?
-        " CONT" : "",
         (enc->firstbyte & WSBIT_FIN) ? "" : " NON-FIN",
         enc->payload_len - enc->payload_remain, enc->payload_len);
 }
@@ -575,7 +647,6 @@ static ssize_t ws_enc_write_head(struct Curl_easy *data,
                                  CURLcode *err)
 {
   unsigned char firstbyte = 0;
-  unsigned char opcode;
   unsigned char head[14];
   size_t hlen;
   ssize_t n;
@@ -595,32 +666,16 @@ static ssize_t ws_enc_write_head(struct Curl_easy *data,
     return -1;
   }
 
-  opcode = ws_frame_flags2op((int)flags & ~CURLWS_CONT);
-  if(!opcode) {
-    failf(data, "WS: provided flags not recognized '%x'", flags);
-    *err = CURLE_SEND_ERROR;
+  firstbyte = ws_frame_flags2firstbyte(data, flags, enc->contfragment, err);
+  if(*err) {
+    failf(data, "WS: provided flags not valid: %x", flags);
     return -1;
   }
 
-  if(!(flags & CURLWS_CONT)) {
-    if(!enc->contfragment)
-      /* not marked as continuing, this is the final fragment */
-      firstbyte |= WSBIT_FIN | opcode;
-    else
-      /* marked as continuing, this is the final fragment; set CONT
-         opcode and FIN bit */
-      firstbyte |= WSBIT_FIN | WSBIT_OPCODE_CONT;
-
-    enc->contfragment = FALSE;
-  }
-  else if(enc->contfragment) {
-    /* the previous fragment was not a final one and this is not either, keep a
-       CONT opcode and no FIN bit */
-    firstbyte |= WSBIT_OPCODE_CONT;
-  }
-  else {
-    firstbyte = opcode;
-    enc->contfragment = TRUE;
+  /* fragmentation only applies to data frames (text/binary);
+   * control frames (close/ping/pong) do not affect the CONT status */
+  if(flags & (CURLWS_TEXT | CURLWS_BINARY)) {
+    enc->contfragment = (flags & CURLWS_CONT) ? (bit)TRUE : (bit)FALSE;
   }
 
   head[0] = enc->firstbyte = firstbyte;
index 47bb96bb447ab5ff03f010eaba8e1747d9cdee98..6e425e92cd7a39c9d76a36b49e3b9eaa19e730a7 100644 (file)
--- a/lib/ws.h
+++ b/lib/ws.h
@@ -43,7 +43,7 @@ struct ws_decoder {
   unsigned char head[10];
   int head_len, head_total;
   enum ws_dec_state state;
-  bool fin_bit;
+  int cont_flags;
 };
 
 /* a client-side WS frame encoder, generating frame headers and
index 8928b8a1185f300445ccce0230d76d3275b31d1a..9ca25ebc02a05599316b5fbce1c3153ee0a7d62e 100644 (file)
@@ -6,7 +6,7 @@ WebSockets
 </info>
 
 #
-# Sends three 4097 bytes TEXT frames, as one single message
+# Sends three 4097 bytes TEXT frames, as fragments of one 12291 bytes message
 <reply>
 <data nocheck="yes">
 HTTP/1.1 101 Switching to WebSockets
@@ -17,8 +17,8 @@ Something: else
 Sec-WebSocket-Accept: HkPsVga7+8LuxM4RGQ5p9tZHeYs=
 
 %hex[%01%7e%10%01]hex%%repeat[256 x helothisisdaniel]%
-%hex[%01%7e%10%01]hex%%repeat[256 x helothisisdaniel]%
-%hex[%81%7e%10%01]hex%%repeat[256 x helothisisdaniel]%
+%hex[%00%7e%10%01]hex%%repeat[256 x helothisisdaniel]%
+%hex[%80%7e%10%01]hex%%repeat[256 x helothisisdaniel]%
 </data>
 # allow upgrade
 <servercmd>
@@ -38,7 +38,7 @@ ws
 http
 </server>
 <name>
-WebSocket curl_ws_recv() loop reading three larger frames
+WebSocket curl_ws_recv() read fragmented message
 </name>
 <tool>
 lib%TESTNUMBER
index 675423d6a2ba5ae765c297f18e364d470498630c..0acc29a7ac69f5cfae122ec20a024f0c8f02885b 100644 (file)
@@ -35,20 +35,20 @@ static void websocket_close(CURL *curl)
   CURLcode result =
     curl_ws_send(curl, "", 0, &sent, 0, CURLWS_CLOSE);
   fprintf(stderr,
-          "ws: curl_ws_send returned %d, sent %d\n", result, (int)sent);
+          "ws: curl_ws_send returned %d, sent %zu\n", result, sent);
 }
 
-static void websocket(CURL *curl)
+static void websocket_frame(CURL *curl, FILE *save, int expected_flags)
 {
   char buffer[256];
   const struct curl_ws_frame *meta;
   size_t nread;
-  size_t i = 0;
-  FILE *save = fopen(libtest_arg2, FOPEN_WRITETEXT);
-  if(!save)
-    return;
+  size_t total_read = 0;
+
+  /* silence "unused parameter" warning */
+  (void)expected_flags;
 
-  /* Three 4097-bytes frames are expected, 12291 bytes */
+  /* Frames are expected to have 4097 bytes */
   while(true) {
     CURLcode result =
       curl_ws_recv(curl, buffer, sizeof(buffer), &nread, &meta);
@@ -63,20 +63,33 @@ static void websocket(CURL *curl)
     printf("%d: nread %zu Age %d Flags %x "
            "Offset %" CURL_FORMAT_CURL_OFF_T " "
            "Bytesleft %" CURL_FORMAT_CURL_OFF_T "\n",
-           (int)i,
+           (int)total_read,
            nread, meta->age, meta->flags, meta->offset, meta->bytesleft);
-    i += meta->len;
+    assert(meta->flags == expected_flags);
+    total_read += nread;
     fwrite(buffer, 1, nread, save);
     /* exit condition */
-    if(!(meta->flags & CURLWS_CONT)) {
+    if(meta->bytesleft == 0) {
       break;
     }
   }
-  fclose(save);
 
-  websocket_close(curl);
+  assert(total_read == 4097);
+}
 
-  assert(i == 12291);
+static void websocket(CURL *curl)
+{
+  FILE *save = fopen(libtest_arg2, FOPEN_WRITETEXT);
+  if(!save)
+    return;
+
+  /* Three frames are expected */
+  websocket_frame(curl, save, CURLWS_TEXT | CURLWS_CONT);
+  websocket_frame(curl, save, CURLWS_TEXT | CURLWS_CONT);
+  websocket_frame(curl, save, CURLWS_TEXT);
+
+  fclose(save);
+  websocket_close(curl);
 }
 
 CURLcode test(char *URL)
@@ -91,7 +104,7 @@ CURLcode test(char *URL)
     curl_easy_setopt(curl, CURLOPT_URL, URL);
 
     /* use the callback style */
-    curl_easy_setopt(curl, CURLOPT_USERAGENT, "websocket/2304");
+    curl_easy_setopt(curl, CURLOPT_USERAGENT, "websocket/2311");
     libtest_debug_config.nohex = 1;
     libtest_debug_config.tracetime = 1;
     curl_easy_setopt(curl, CURLOPT_DEBUGDATA, &libtest_debug_config);