From b64c70acda43f0e06d4e74e94963e2471bf68586 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Thu, 19 Mar 2026 15:58:04 +0800 Subject: [PATCH] fix: fixes memory leak with subprotocols --- .../esp_http_server/include/esp_http_server.h | 3 +- components/esp_http_server/src/httpd_uri.c | 9 ++++ .../test_apps/main/test_http_server.c | 50 ++++++++++++++++++- .../test_apps/sdkconfig.defaults | 1 + 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/components/esp_http_server/include/esp_http_server.h b/components/esp_http_server/include/esp_http_server.h index 61649f855a..b1a2197c9c 100644 --- a/components/esp_http_server/include/esp_http_server.h +++ b/components/esp_http_server/include/esp_http_server.h @@ -872,7 +872,8 @@ esp_err_t httpd_sess_set_pending_override(httpd_handle_t hd, int sockfd, httpd_p * @note * - This function is necessary in order to handle multiple requests simultaneously. * See examples/async_requests for example usage. - * - You must call httpd_req_async_handler_complete() when you are done with the request. + * - You must call httpd_req_async_handler_complete() when you are done with the request + * and also on any error conditions. * * @param[in] r The request to create an async copy of * @param[out] out A newly allocated request which can be used on an async thread diff --git a/components/esp_http_server/src/httpd_uri.c b/components/esp_http_server/src/httpd_uri.c index 1710253cee..006538e82c 100644 --- a/components/esp_http_server/src/httpd_uri.c +++ b/components/esp_http_server/src/httpd_uri.c @@ -213,6 +213,9 @@ esp_err_t httpd_unregister_uri_handler(httpd_handle_t handle, ESP_LOGD(TAG, LOG_FMT("[%d] removing %s"), i, hd->hd_calls[i]->uri); free((char*)hd->hd_calls[i]->uri); +#ifdef CONFIG_HTTPD_WS_SUPPORT + free((char*)hd->hd_calls[i]->supported_subprotocol); +#endif // CONFIG_HTTPD_WS_SUPPORT free(hd->hd_calls[i]); hd->hd_calls[i] = NULL; @@ -251,6 +254,9 @@ esp_err_t httpd_unregister_uri(httpd_handle_t handle, const char *uri) ESP_LOGD(TAG, LOG_FMT("[%d] removing %s"), i, uri); free((char*)hd->hd_calls[i]->uri); +#ifdef CONFIG_HTTPD_WS_SUPPORT + free((char*)hd->hd_calls[i]->supported_subprotocol); +#endif // CONFIG_HTTPD_WS_SUPPORT free(hd->hd_calls[i]); hd->hd_calls[i] = NULL; found = true; @@ -282,6 +288,9 @@ void httpd_unregister_all_uri_handlers(struct httpd_data *hd) ESP_LOGD(TAG, LOG_FMT("[%d] removing %s"), i, hd->hd_calls[i]->uri); free((char*)hd->hd_calls[i]->uri); +#ifdef CONFIG_HTTPD_WS_SUPPORT + free((char*)hd->hd_calls[i]->supported_subprotocol); +#endif // CONFIG_HTTPD_WS_SUPPORT free(hd->hd_calls[i]); hd->hd_calls[i] = NULL; } 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 586539d1e0..6b91505524 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 @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -8,6 +8,7 @@ #include #include #include +#include #include "unity.h" #include "test_utils.h" @@ -31,6 +32,21 @@ httpd_uri_t handler_limit_uri (char* path) return uri; }; +#ifdef CONFIG_HTTPD_WS_SUPPORT +static httpd_uri_t handler_limit_ws_uri(char *path, const char *subprotocol) +{ + httpd_uri_t uri = { + .uri = path, + .method = HTTP_GET, + .handler = null_func, + .user_ctx = NULL, + .is_websocket = true, + .supported_subprotocol = subprotocol, + }; + return uri; +} +#endif /* CONFIG_HTTPD_WS_SUPPORT */ + static inline unsigned num_digits(unsigned x) { unsigned digits = 1; @@ -81,6 +97,36 @@ void test_handler_limit(httpd_handle_t hd) TEST_ASSERT(httpd_unregister_uri_handler(hd, uris[i].uri, uris[i].method) == ESP_OK); } basic_sanity = false; + +#ifdef CONFIG_HTTPD_WS_SUPPORT + /* --- WS subprotocol memory leak check --- + * Each registration strdup's the supported_subprotocol string. + * Verify that unregistration frees it (expected leak per handler: + * strlen(subprotocol) + 1 bytes if the bug is present). + */ + char ws_paths[HTTPD_TEST_MAX_URI_HANDLERS][8]; + httpd_uri_t ws_uris[HTTPD_TEST_MAX_URI_HANDLERS]; + const char *subprotocol = "chat"; + + for (i = 0; i < HTTPD_TEST_MAX_URI_HANDLERS; i++) { + sprintf(ws_paths[i], "/ws%d", i); + ws_uris[i] = handler_limit_ws_uri(ws_paths[i], subprotocol); + } + + size_t heap_before = heap_caps_get_free_size(MALLOC_CAP_8BIT); + + for (i = 0; i < HTTPD_TEST_MAX_URI_HANDLERS; i++) { + TEST_ASSERT(httpd_register_uri_handler(hd, &ws_uris[i]) == ESP_OK); + } + for (i = 0; i < HTTPD_TEST_MAX_URI_HANDLERS; i++) { + TEST_ASSERT(httpd_unregister_uri_handler(hd, ws_uris[i].uri, ws_uris[i].method) == ESP_OK); + } + + size_t heap_after = heap_caps_get_free_size(MALLOC_CAP_8BIT); + int leaked = (int)heap_before - (int)heap_after; + + TEST_ASSERT_MESSAGE(leaked <= 0, "Heap leaked after WS handler unregister"); +#endif /* CONFIG_HTTPD_WS_SUPPORT */ } /********************* Test Handler Limit End *******************/ @@ -100,7 +146,7 @@ httpd_handle_t test_httpd_start(uint16_t id) /* Currently this only tests for the number of tasks. * Heap leakage is not tested as LWIP allocates memory - * which may not be freed immedietly causing erroneous + * which may not be freed immediately causing erroneous * evaluation. Another test to implement would be the * monitoring of open sockets, but LWIP presently provides * no such API for getting the number of open sockets. diff --git a/components/esp_http_server/test_apps/sdkconfig.defaults b/components/esp_http_server/test_apps/sdkconfig.defaults index e7d6da5519..e215671eec 100644 --- a/components/esp_http_server/test_apps/sdkconfig.defaults +++ b/components/esp_http_server/test_apps/sdkconfig.defaults @@ -7,3 +7,4 @@ CONFIG_COMPILER_STACK_CHECK_MODE_STRONG=y CONFIG_COMPILER_STACK_CHECK=y CONFIG_ESP_TASK_WDT_EN=n +CONFIG_HTTPD_WS_SUPPORT=y