From b7ab27341ec9cc6ec31922eb9ecc780c28f84b06 Mon Sep 17 00:00:00 2001 From: Vincent Hamp Date: Sat, 12 Apr 2025 13:31:57 +0200 Subject: [PATCH] fix(esp_http_server): WebSocket frame parsing errors Fixes the Websocket frame pasring error, by making sure that two bytes are read compulsary for length bytes 126. Closes https://github.com/espressif/esp-idf/pull/15767 Closes https://github.com/espressif/esp-idf/issues/15235 --- .../esp_http_server/src/esp_httpd_priv.h | 28 +++++++++--- components/esp_http_server/src/httpd_txrx.c | 43 +++++++++++-------- components/esp_http_server/src/httpd_ws.c | 14 +++--- 3 files changed, 53 insertions(+), 32 deletions(-) diff --git a/components/esp_http_server/src/esp_httpd_priv.h b/components/esp_http_server/src/esp_httpd_priv.h index e56418236e..33af33c152 100644 --- a/components/esp_http_server/src/esp_httpd_priv.h +++ b/components/esp_http_server/src/esp_httpd_priv.h @@ -132,6 +132,15 @@ struct httpd_data { httpd_err_handler_func_t *err_handler_fns; }; +/** + * @brief Options for receiving HTTP request data + */ +typedef enum { + HTTPD_RECV_OPT_NONE = 0, + HTTPD_RECV_OPT_HALT_AFTER_PENDING = 1, /*!< Halt immediately after receiving from pending buffer */ + HTTPD_RECV_OPT_BLOCKING = 2, /*!< Receive blocking (don't return partial length) */ +} httpd_recv_opt_t; + /******************* Group : Session Management ********************/ /** @name Session Management * Functions related to HTTP session management @@ -422,22 +431,27 @@ int httpd_send(httpd_req_t *req, const char *buf, size_t buf_len); * * @note The exposed API httpd_recv() is simply this function with last parameter * set as false. This function is used internally during reception and - * processing of a new request. The option to halt after receiving pending - * data prevents the server from requesting more data than is needed for - * completing a packet in case when all the remaining part of the packet is - * in the pending buffer. + * processing of a new request. + * + * There are 2 options available that affect the behavior of the function: + * - HTTPD_RECV_OPT_HALT_AFTER_PENDING + * The option to halt after receiving pending data prevents the server from + * requesting more data than is needed for completing a packet in case when + * all the remaining part of the packet is in the pending buffer. + * + * - HTTPD_RECV_OPT_BLOCKING + * The option to not return until the `buf_len` bytes have been read. * * @param[in] req Pointer to new HTTP request which only has the socket descriptor * @param[out] buf Pointer to the buffer which will be filled with the received data * @param[in] buf_len Length of the buffer - * @param[in] halt_after_pending When set true, halts immediately after receiving from - * pending buffer + * @param[in] opt Receive option * * @return * - Length of data : if successful * - ESP_FAIL : if failed */ -int httpd_recv_with_opt(httpd_req_t *r, char *buf, size_t buf_len, bool halt_after_pending); +int httpd_recv_with_opt(httpd_req_t *r, char *buf, size_t buf_len, httpd_recv_opt_t opt); /** * @brief For un-receiving HTTP request data diff --git a/components/esp_http_server/src/httpd_txrx.c b/components/esp_http_server/src/httpd_txrx.c index fa50378f69..1ca2165d71 100644 --- a/components/esp_http_server/src/httpd_txrx.c +++ b/components/esp_http_server/src/httpd_txrx.c @@ -95,7 +95,7 @@ static size_t httpd_recv_pending(httpd_req_t *r, char *buf, size_t buf_len) return buf_len; } -int httpd_recv_with_opt(httpd_req_t *r, char *buf, size_t buf_len, bool halt_after_pending) +int httpd_recv_with_opt(httpd_req_t *r, char *buf, size_t buf_len, httpd_recv_opt_t opt) { ESP_LOGD(TAG, LOG_FMT("requested length = %"NEWLIB_NANO_COMPAT_FORMAT), NEWLIB_NANO_COMPAT_CAST(buf_len)); @@ -112,34 +112,41 @@ int httpd_recv_with_opt(httpd_req_t *r, char *buf, size_t buf_len, bool halt_aft /* If buffer filled then no need to recv. * If asked to halt after receiving pending data then * return with received length */ - if (!buf_len || halt_after_pending) { + if (!buf_len || opt == HTTPD_RECV_OPT_HALT_AFTER_PENDING) { return pending_len; } } /* Receive data of remaining length */ - int ret = ra->sd->recv_fn(ra->sd->handle, ra->sd->fd, buf, buf_len, 0); - if (ret < 0) { - ESP_LOGD(TAG, LOG_FMT("error in recv_fn")); - if ((ret == HTTPD_SOCK_ERR_TIMEOUT) && (pending_len != 0)) { - /* If recv() timeout occurred, but pending data is - * present, return length of pending data. - * This behavior is similar to that of socket recv() - * function, which, in case has only partially read the - * requested length, due to timeout, returns with read - * length, rather than error */ - return pending_len; + size_t recv_len = pending_len; + do { + int ret = ra->sd->recv_fn(ra->sd->handle, ra->sd->fd, buf, buf_len, 0); + if (ret < 0) { + ESP_LOGD(TAG, LOG_FMT("error in recv_fn")); + if ((ret == HTTPD_SOCK_ERR_TIMEOUT) && (pending_len != 0)) { + /* If recv() timeout occurred, but pending data is + * present, return length of pending data. + * This behavior is similar to that of socket recv() + * function, which, in case has only partially read the + * requested length, due to timeout, returns with read + * length, rather than error */ + return pending_len; + } + return ret; } - return ret; - } - ESP_LOGD(TAG, LOG_FMT("received length = %"NEWLIB_NANO_COMPAT_FORMAT), NEWLIB_NANO_COMPAT_CAST((ret + pending_len))); - return ret + pending_len; + recv_len += ret; + buf += ret; + buf_len -= ret; + } while (buf_len > 0 && opt == HTTPD_RECV_OPT_BLOCKING); + + ESP_LOGD(TAG, LOG_FMT("received length = %"NEWLIB_NANO_COMPAT_FORMAT), NEWLIB_NANO_COMPAT_CAST(recv_len)); + return recv_len; } int httpd_recv(httpd_req_t *r, char *buf, size_t buf_len) { - return httpd_recv_with_opt(r, buf, buf_len, false); + return httpd_recv_with_opt(r, buf, buf_len, HTTPD_RECV_OPT_NONE); } size_t httpd_unrecv(struct httpd_req *r, const char *buf, size_t buf_len) diff --git a/components/esp_http_server/src/httpd_ws.c b/components/esp_http_server/src/httpd_ws.c index bcf32cf9e6..4bffe53842 100644 --- a/components/esp_http_server/src/httpd_ws.c +++ b/components/esp_http_server/src/httpd_ws.c @@ -296,7 +296,7 @@ esp_err_t httpd_ws_recv_frame(httpd_req_t *req, httpd_ws_frame_t *frame, size_t /* Grab the second byte */ uint8_t second_byte = 0; - if (httpd_recv_with_opt(req, (char *)&second_byte, sizeof(second_byte), false) <= 0) { + if (httpd_recv_with_opt(req, (char *)&second_byte, sizeof(second_byte), HTTPD_RECV_OPT_BLOCKING) < sizeof(second_byte)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive the second byte")); return ESP_FAIL; } @@ -313,7 +313,7 @@ esp_err_t httpd_ws_recv_frame(httpd_req_t *req, httpd_ws_frame_t *frame, size_t } else if (init_len == 126) { /* Case 2: If length byte is 126, then this frame's length bit is 16 bits */ uint8_t length_bytes[2] = { 0 }; - if (httpd_recv_with_opt(req, (char *)length_bytes, sizeof(length_bytes), false) <= 0) { + if (httpd_recv_with_opt(req, (char *)length_bytes, sizeof(length_bytes), HTTPD_RECV_OPT_BLOCKING) < sizeof(length_bytes)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive 2 bytes length")); return ESP_FAIL; } @@ -322,8 +322,8 @@ esp_err_t httpd_ws_recv_frame(httpd_req_t *req, httpd_ws_frame_t *frame, size_t } else if (init_len == 127) { /* Case 3: If length is byte 127, then this frame's length bit is 64 bits */ uint8_t length_bytes[8] = { 0 }; - if (httpd_recv_with_opt(req, (char *)length_bytes, sizeof(length_bytes), false) <= 0) { - ESP_LOGW(TAG, LOG_FMT("Failed to receive 2 bytes length")); + if (httpd_recv_with_opt(req, (char *)length_bytes, sizeof(length_bytes), HTTPD_RECV_OPT_BLOCKING) < sizeof(length_bytes)) { + ESP_LOGW(TAG, LOG_FMT("Failed to receive 8 bytes length")); return ESP_FAIL; } @@ -338,7 +338,7 @@ esp_err_t httpd_ws_recv_frame(httpd_req_t *req, httpd_ws_frame_t *frame, size_t } /* If this frame is masked, dump the mask as well */ if (masked) { - if (httpd_recv_with_opt(req, (char *)aux->mask_key, sizeof(aux->mask_key), false) <= 0) { + if (httpd_recv_with_opt(req, (char *)aux->mask_key, sizeof(aux->mask_key), HTTPD_RECV_OPT_BLOCKING) < sizeof(aux->mask_key)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive mask key")); return ESP_FAIL; } @@ -375,7 +375,7 @@ esp_err_t httpd_ws_recv_frame(httpd_req_t *req, httpd_ws_frame_t *frame, size_t size_t offset = 0; while (left_len > 0) { - int read_len = httpd_recv_with_opt(req, (char *)frame->payload + offset, left_len, false); + int read_len = httpd_recv_with_opt(req, (char *)frame->payload + offset, left_len, HTTPD_RECV_OPT_NONE); if (read_len <= 0) { ESP_LOGW(TAG, LOG_FMT("Failed to receive payload")); return ESP_FAIL; @@ -483,7 +483,7 @@ esp_err_t httpd_ws_get_frame_type(httpd_req_t *req) /* Read the first byte from the frame to get the FIN flag and Opcode */ /* Please refer to RFC6455 Section 5.2 for more details */ uint8_t first_byte = 0; - if (httpd_recv_with_opt(req, (char *)&first_byte, sizeof(first_byte), false) <= 0) { + if (httpd_recv_with_opt(req, (char *)&first_byte, sizeof(first_byte), HTTPD_RECV_OPT_BLOCKING) < sizeof(first_byte)) { /* If the recv() return code is <= 0, then this socket FD is invalid (i.e. a broken connection) */ /* Here we mark it as a Close message and close it later. */ ESP_LOGW(TAG, LOG_FMT("Failed to read header byte (socket FD invalid), closing socket now"));