From 3588df9478d7c27046b34cdb510728a26bedabc7 Mon Sep 17 00:00:00 2001 From: Calvin Ruocco Date: Mon, 10 Mar 2025 17:19:58 +0100 Subject: [PATCH] ws: fix and extend CURLWS_CONT handling Follow-up to fa3d1e7d43bf0e4f589aeae737 Add test 2311 to verify Closes #16687 --- docs/libcurl/curl_ws_recv.md | 10 +- docs/libcurl/curl_ws_send.md | 5 +- lib/ws.c | 251 +++++++++++++++++++++-------------- lib/ws.h | 2 +- tests/data/test2311 | 8 +- tests/libtest/lib2311.c | 41 ++++-- 6 files changed, 196 insertions(+), 121 deletions(-) diff --git a/docs/libcurl/curl_ws_recv.md b/docs/libcurl/curl_ws_recv.md index b126305833..2409335f0d 100644 --- a/docs/libcurl/curl_ws_recv.md +++ b/docs/libcurl/curl_ws_recv.md @@ -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% diff --git a/docs/libcurl/curl_ws_send.md b/docs/libcurl/curl_ws_send.md index 852cbf3d2a..757059fbd4 100644 --- a/docs/libcurl/curl_ws_send.md +++ b/docs/libcurl/curl_ws_send.md @@ -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). diff --git a/lib/ws.c b/lib/ws.c index 372338fcbd..d6ba74a440 100644 --- a/lib/ws.c +++ b/lib/ws.c @@ -47,16 +47,26 @@ #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) @@ -66,58 +76,120 @@ #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; diff --git a/lib/ws.h b/lib/ws.h index 47bb96bb44..6e425e92cd 100644 --- 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 diff --git a/tests/data/test2311 b/tests/data/test2311 index 8928b8a118..9ca25ebc02 100644 --- a/tests/data/test2311 +++ b/tests/data/test2311 @@ -6,7 +6,7 @@ WebSockets # -# Sends three 4097 bytes TEXT frames, as one single message +# Sends three 4097 bytes TEXT frames, as fragments of one 12291 bytes message 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]% # allow upgrade @@ -38,7 +38,7 @@ ws http -WebSocket curl_ws_recv() loop reading three larger frames +WebSocket curl_ws_recv() read fragmented message lib%TESTNUMBER diff --git a/tests/libtest/lib2311.c b/tests/libtest/lib2311.c index 675423d6a2..0acc29a7ac 100644 --- a/tests/libtest/lib2311.c +++ b/tests/libtest/lib2311.c @@ -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); -- 2.47.2