From 0fb8ad002ac0345eff9c59256a4019ec113db8f2 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Wed, 6 May 2026 14:02:14 +0800 Subject: [PATCH] fix(esp_http_server): fixes websocket recv error handling Closes https://github.com/espressif/esp-idf/issues/18483 --- components/esp_http_server/src/httpd_ws.c | 17 ++++-- .../test_apps/main/CMakeLists.txt | 4 +- .../test_apps/main/test_http_server.c | 55 +++++++++++++++++++ 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/components/esp_http_server/src/httpd_ws.c b/components/esp_http_server/src/httpd_ws.c index a765c59e11c..f479a8b0a88 100644 --- a/components/esp_http_server/src/httpd_ws.c +++ b/components/esp_http_server/src/httpd_ws.c @@ -291,7 +291,8 @@ static esp_err_t httpd_ws_recv_frame_internal(httpd_req_t *req, httpd_ws_frame_t /* Grab the second byte */ uint8_t second_byte = 0; - if (httpd_recv_with_opt(req, (char *)&second_byte, sizeof(second_byte), HTTPD_RECV_OPT_BLOCKING) < sizeof(second_byte)) { + int recv_ret = httpd_recv_with_opt(req, (char *)&second_byte, sizeof(second_byte), HTTPD_RECV_OPT_BLOCKING); + if (recv_ret != (int)sizeof(second_byte)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive the second byte")); return ESP_FAIL; } @@ -308,7 +309,8 @@ static esp_err_t httpd_ws_recv_frame_internal(httpd_req_t *req, httpd_ws_frame_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), HTTPD_RECV_OPT_BLOCKING) < sizeof(length_bytes)) { + recv_ret = httpd_recv_with_opt(req, (char *)length_bytes, sizeof(length_bytes), HTTPD_RECV_OPT_BLOCKING); + if (recv_ret != (int)sizeof(length_bytes)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive 2 bytes length")); return ESP_FAIL; } @@ -317,7 +319,8 @@ static esp_err_t httpd_ws_recv_frame_internal(httpd_req_t *req, httpd_ws_frame_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), HTTPD_RECV_OPT_BLOCKING) < sizeof(length_bytes)) { + recv_ret = httpd_recv_with_opt(req, (char *)length_bytes, sizeof(length_bytes), HTTPD_RECV_OPT_BLOCKING); + if (recv_ret != (int)sizeof(length_bytes)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive 8 bytes length")); return ESP_FAIL; } @@ -335,7 +338,8 @@ static esp_err_t httpd_ws_recv_frame_internal(httpd_req_t *req, httpd_ws_frame_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), HTTPD_RECV_OPT_BLOCKING) < sizeof(aux->mask_key)) { + recv_ret = httpd_recv_with_opt(req, (char *)aux->mask_key, sizeof(aux->mask_key), HTTPD_RECV_OPT_BLOCKING); + if (recv_ret != (int)sizeof(aux->mask_key)) { ESP_LOGW(TAG, LOG_FMT("Failed to receive mask key")); return ESP_FAIL; } @@ -494,8 +498,9 @@ 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), 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) */ + int recv_ret = httpd_recv_with_opt(req, (char *)&first_byte, sizeof(first_byte), HTTPD_RECV_OPT_BLOCKING); + if (recv_ret != (int)sizeof(first_byte)) { + /* If we fail to read exactly one byte, this socket FD is invalid or the frame header is incomplete. */ /* 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")); aux->ws_final = true; diff --git a/components/esp_http_server/test_apps/main/CMakeLists.txt b/components/esp_http_server/test_apps/main/CMakeLists.txt index eb7ca6e23c3..a97994f3498 100644 --- a/components/esp_http_server/test_apps/main/CMakeLists.txt +++ b/components/esp_http_server/test_apps/main/CMakeLists.txt @@ -1,3 +1,3 @@ idf_component_register(SRC_DIRS "." - PRIV_INCLUDE_DIRS "." - PRIV_REQUIRES esp_http_server test_utils unity) + PRIV_INCLUDE_DIRS "." "../../src" "../../src/port/esp32" + PRIV_REQUIRES esp_http_server test_utils unity esp_timer) diff --git a/components/esp_http_server/test_apps/main/test_http_server.c b/components/esp_http_server/test_apps/main/test_http_server.c index 101644448ea..3b60cc771ec 100644 --- a/components/esp_http_server/test_apps/main/test_http_server.c +++ b/components/esp_http_server/test_apps/main/test_http_server.c @@ -12,6 +12,10 @@ #include #include "esp_log.h" +#ifdef CONFIG_HTTPD_WS_SUPPORT +#include "../../src/esp_httpd_priv.h" +#endif + #include "unity.h" #include "test_utils.h" @@ -49,6 +53,25 @@ static httpd_uri_t handler_limit_ws_uri(char *path, const char *subprotocol) }; return uri; } + +static int ws_recv_fail_handler_calls; + +static int ws_recv_fail_override(httpd_handle_t hd, int sockfd, char *buf, size_t buf_len, int flags) +{ + (void)hd; + (void)sockfd; + (void)buf; + (void)buf_len; + (void)flags; + return HTTPD_SOCK_ERR_FAIL; +} + +static esp_err_t ws_counting_handler(httpd_req_t *req) +{ + (void)req; + ws_recv_fail_handler_calls++; + return ESP_OK; +} #endif /* CONFIG_HTTPD_WS_SUPPORT */ static inline unsigned num_digits(unsigned x) @@ -402,6 +425,38 @@ TEST_CASE("httpd_resp_set_type rejects CRLF in content type", "[HTTP SERVER][sec httpd_resp_set_type(&fake_req, "text/html\nX-Injected: pwned")); } +#ifdef CONFIG_HTTPD_WS_SUPPORT +TEST_CASE("WS recv failure marks close without dispatching handler", "[HTTP SERVER][websocket]") +{ + httpd_config_t config = HTTPD_DEFAULT_CONFIG(); + struct httpd_data hd = {0}; + struct sock_db session = {0}; + + hd.config = config; + hd.hd_req_aux.resp_hdrs = calloc(config.max_resp_headers, sizeof(*hd.hd_req_aux.resp_hdrs)); + TEST_ASSERT_NOT_NULL(hd.hd_req_aux.resp_hdrs); + + session.fd = 123; + session.handle = (httpd_handle_t) &hd; + session.recv_fn = ws_recv_fail_override; + session.ws_handshake_done = true; + session.ws_handler = ws_counting_handler; + session.ws_control_frames = false; + session.ws_close = false; + + ws_recv_fail_handler_calls = 0; + + esp_err_t ret = httpd_req_new(&hd, &session); + + TEST_ASSERT_EQUAL(ESP_OK, ret); + TEST_ASSERT_EQUAL(0, ws_recv_fail_handler_calls); + TEST_ASSERT_TRUE(session.ws_close); + TEST_ASSERT_EQUAL(HTTPD_WS_TYPE_CLOSE, hd.hd_req_aux.ws_type); + + free(hd.hd_req_aux.resp_hdrs); +} +#endif /* CONFIG_HTTPD_WS_SUPPORT */ + void app_main(void) { unity_run_menu();