From 1041b691319bfbc6ce17148799e6e233003c7b59 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Fri, 29 May 2026 15:22:47 +0800 Subject: [PATCH] feat(esp_http_client): detect oversized headers in tx buffer while sending request --- components/esp_common/src/esp_err_to_name.c | 6 + components/esp_http_client/Kconfig | 12 ++ components/esp_http_client/esp_http_client.c | 33 ++++++ .../esp_http_client/include/esp_http_client.h | 3 + components/esp_http_client/lib/http_header.c | 2 +- .../test_apps/main/CMakeLists.txt | 4 +- .../test_apps/main/test_http_client.c | 106 +++++++++++++++++- .../test_apps/main/test_http_header.c | 77 +++++++++++++ .../test_apps/pytest_esp_http_client_ut.py | 2 +- .../test_apps/sdkconfig.ci.strict_header | 2 + 10 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 components/esp_http_client/test_apps/main/test_http_header.c create mode 100644 components/esp_http_client/test_apps/sdkconfig.ci.strict_header diff --git a/components/esp_common/src/esp_err_to_name.c b/components/esp_common/src/esp_err_to_name.c index 9e613764b5b..d10c861bcd9 100644 --- a/components/esp_common/src/esp_err_to_name.c +++ b/components/esp_common/src/esp_err_to_name.c @@ -686,6 +686,12 @@ static const esp_err_msg_t esp_err_msg_table[] = { # ifdef ESP_ERR_HTTP_REDIRECT_DOWNGRADE ERR_TBL_IT(ESP_ERR_HTTP_REDIRECT_DOWNGRADE), /* 28685 0x700d HTTPS origin redirected to a non-HTTPS scheme (downgrade blocked) */ +# endif +# ifdef ESP_ERR_HTTP_HEADER_TOO_LONG + ERR_TBL_IT(ESP_ERR_HTTP_HEADER_TOO_LONG), /* 28686 0x700e A single request header is larger than + buffer_size_tx and cannot be sent (only + when CONFIG_ESP_HTTP_CLIENT_STRICT_HEADE + R_BUFFER is enabled) */ # endif // components/esp-tls/esp_tls_errors.h # ifdef ESP_ERR_ESP_TLS_BASE diff --git a/components/esp_http_client/Kconfig b/components/esp_http_client/Kconfig index 3eaaf171c4f..a4c980b1333 100644 --- a/components/esp_http_client/Kconfig +++ b/components/esp_http_client/Kconfig @@ -66,4 +66,16 @@ menu "ESP HTTP client" This config option helps in setting the maximum size of response header that can be saved in esp_http_client component. Note that the same size is used for key and value in the response headers. + + config ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER + bool "Fail the request when a header does not fit in the tx buffer" + default y + help + When enabled, esp_http_client_request_send() returns + ESP_ERR_HTTP_HEADER_TOO_LONG if any individual header is larger than + buffer_size_tx. With this option disabled, the legacy behavior is + preserved: oversized headers are silently dropped and the request + is emitted without its '\r\n\r\n' terminator, which typically + results in the server timing out the connection. The strict check + adds two small comparisons in the header send loop. endmenu diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index 1567cd21cdb..2b728ec53b3 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -1817,7 +1817,26 @@ esp_err_t esp_http_client_request_send(esp_http_client_handle_t client, int writ } int wlen = client->buffer_size_tx - first_line_len; +#ifdef CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER + int prev_header_index = client->header_index; +#endif while ((client->header_index = http_header_generate_string(client->request->headers, client->header_index, client->request->buffer->data + first_line_len, &wlen))) { +#ifdef CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER + /* No-progress detection: a positive return equal to (or below) + * the input index means the offending header at prev_header_index + * is larger than the buffer and pagination cannot advance. */ + if (client->header_index <= prev_header_index) { + ESP_LOGD(TAG, "Header at index %d does not fit in tx buffer (size: %d)", + prev_header_index, client->buffer_size_tx); + /* This is a permanent failure, not a transient one. Clear errno so + * the async caller (which keys "retry later" off errno == EAGAIN) + * cannot misread a stale EAGAIN and spin retrying a request that + * can never succeed. */ + errno = 0; + return ESP_ERR_HTTP_HEADER_TOO_LONG; + } + prev_header_index = client->header_index; +#endif if (wlen <= 0) { break; } @@ -1843,6 +1862,20 @@ esp_err_t esp_http_client_request_send(esp_http_client_handle_t client, int writ wlen = client->buffer_size_tx; } +#ifdef CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER + /* Case where the very first header (at index 0) is larger than the + * buffer: the helper returns 0 with *buffer_len zeroed, so the loop + * exits without entering the body. Distinguish from a legitimate + * empty/already-done state by the zeroed wlen. */ + if (client->header_index == 0 && wlen == 0) { + ESP_LOGD(TAG, "Header at index 0 does not fit in tx buffer (size: %d)", client->buffer_size_tx); + /* Permanent failure: clear errno so the async caller does not treat a + * stale EAGAIN as "retry later" and spin on an unsendable request. */ + errno = 0; + return ESP_ERR_HTTP_HEADER_TOO_LONG; + } +#endif + client->data_written_index = 0; client->data_write_left = client->post_len; client->state = HTTP_STATE_REQ_COMPLETE_HEADER; diff --git a/components/esp_http_client/include/esp_http_client.h b/components/esp_http_client/include/esp_http_client.h index 4a257808778..925336bf267 100644 --- a/components/esp_http_client/include/esp_http_client.h +++ b/components/esp_http_client/include/esp_http_client.h @@ -298,6 +298,7 @@ typedef enum { #define ESP_ERR_HTTP_READ_TIMEOUT (ESP_ERR_HTTP_BASE + 11) /*!< HTTP data read timeout */ #define ESP_ERR_HTTP_INCOMPLETE_DATA (ESP_ERR_HTTP_BASE + 12) /*!< Incomplete data received, less than Content-Length or last chunk */ #define ESP_ERR_HTTP_REDIRECT_DOWNGRADE (ESP_ERR_HTTP_BASE + 13) /*!< HTTPS origin redirected to a non-HTTPS scheme (downgrade blocked) */ +#define ESP_ERR_HTTP_HEADER_TOO_LONG (ESP_ERR_HTTP_BASE + 14) /*!< A single request header is larger than buffer_size_tx and cannot be sent (only when CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER is enabled) */ /** * @brief Start a HTTP session @@ -368,6 +369,8 @@ esp_err_t esp_http_client_prepare(esp_http_client_handle_t client); * - ESP_OK on successful * - ESP_FAIL on error * - ESP_ERR_HTTP_WRITE_DATA if write operation fails + * - ESP_ERR_HTTP_HEADER_TOO_LONG if a single request header is larger than buffer_size_tx + * (only when CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER is enabled) */ esp_err_t esp_http_client_request_send(esp_http_client_handle_t client, int write_len); diff --git a/components/esp_http_client/lib/http_header.c b/components/esp_http_client/lib/http_header.c index 8924d1824c6..0bbc93fdbc5 100644 --- a/components/esp_http_client/lib/http_header.c +++ b/components/esp_http_client/lib/http_header.c @@ -187,7 +187,7 @@ int http_header_generate_string(http_header_handle_t header, int index, char *bu if (size + 1 > *buffer_len - 2) { // if this item would not fit to the buffer, return the index of the last fitting one ret_idx = idx - 1; - ESP_LOGE(TAG, "Buffer length is small to fit all the headers"); + ESP_LOGE(TAG, "Buffer length is small to fit all the headers, required %d bytes, buffer size %d", size + 1, *buffer_len - 2); break; } } diff --git a/components/esp_http_client/test_apps/main/CMakeLists.txt b/components/esp_http_client/test_apps/main/CMakeLists.txt index 0c7a34fc190..e063a6242eb 100644 --- a/components/esp_http_client/test_apps/main/CMakeLists.txt +++ b/components/esp_http_client/test_apps/main/CMakeLists.txt @@ -1,3 +1,5 @@ idf_component_register(SRC_DIRS "." PRIV_INCLUDE_DIRS "." - PRIV_REQUIRES esp_http_client test_utils unity) + "../../lib/include" + PRIV_REQUIRES esp_http_client tcp_transport test_utils unity + WHOLE_ARCHIVE) diff --git a/components/esp_http_client/test_apps/main/test_http_client.c b/components/esp_http_client/test_apps/main/test_http_client.c index 2c9d2128b0f..29214c4059a 100644 --- a/components/esp_http_client/test_apps/main/test_http_client.c +++ b/components/esp_http_client/test_apps/main/test_http_client.c @@ -1,16 +1,18 @@ /* - * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #include #include +#include #include #include #include "unity.h" #include "test_utils.h" +#include "sdkconfig.h" #define HOST "httpbin.org" #define USERNAME "user" @@ -221,6 +223,108 @@ TEST_CASE("esp_http_client_close() and cleanup() should not dispatch duplicate d TEST_ASSERT_LESS_OR_EQUAL(1, disconnect_event_count); } +#ifdef CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER + +#if CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT +#include "esp_transport.h" + +/* + * Minimal stub transport for the Case B test below. write() always reports + * success so the test can isolate the strict-header check from the real + * transport's failure path. + */ +static int stub_transport_connect(esp_transport_handle_t t, const char *host, int port, int timeout_ms) { return 0; } +static int stub_transport_write(esp_transport_handle_t t, const char *buffer, int len, int timeout_ms) { return len; } +static int stub_transport_read(esp_transport_handle_t t, char *buffer, int len, int timeout_ms) { return 0; } +static int stub_transport_close(esp_transport_handle_t t) { return 0; } +static int stub_transport_destroy(esp_transport_handle_t t) { return 0; } +static int stub_transport_poll(esp_transport_handle_t t, int timeout_ms) { return 1; } +#endif // CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT + +TEST_CASE("esp_http_client_request_send fails when a header exceeds tx buffer", "[ESP HTTP CLIENT]") +{ + /* + * The "first header too big" path: with a small buffer_size_tx and an + * oversized header, http_header_generate_string() returns 0 and the + * helper exits the write loop without ever touching the transport. + * The after-loop strict check must surface ESP_ERR_HTTP_HEADER_TOO_LONG + * instead of silently completing as ESP_OK. + */ + esp_http_client_config_t config = { + .url = "http://example.com/", + .buffer_size_tx = 128, + }; + esp_http_client_handle_t client = esp_http_client_init(&config); + TEST_ASSERT_NOT_NULL(client); + + /* Drop the default User-Agent and Host headers so the oversized header + * is the very first entry. That keeps us on the Case A path (helper + * returns 0 with wlen=0) so the after-loop strict check fires before + * esp_transport_write is ever attempted — no network needed. */ + esp_http_client_set_header(client, "User-Agent", NULL); + esp_http_client_set_header(client, "Host", NULL); + + char huge_value[200]; + memset(huge_value, 'A', sizeof(huge_value) - 1); + huge_value[sizeof(huge_value) - 1] = '\0'; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_set_header(client, "X-Huge", huge_value)); + + esp_err_t err = esp_http_client_request_send(client, 0); + TEST_ASSERT_EQUAL(ESP_ERR_HTTP_HEADER_TOO_LONG, err); + + esp_http_client_cleanup(client); +} + +#if CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT +TEST_CASE("esp_http_client_request_send fails when an oversized header is mid-list", "[ESP HTTP CLIENT]") +{ + /* + * Case B: the small header at index 0 paginates, the oversized header + * at index 1 cannot fit on its own. A stub transport accepts the first + * chunk so the loop runs a second iteration, where the strict check + * fires. The stub can only succeed, so an ESP_ERR_HTTP_HEADER_TOO_LONG + * return here is unambiguously the strict check (not transport failure). + */ + esp_transport_handle_t stub = esp_transport_init(); + TEST_ASSERT_NOT_NULL(stub); + TEST_ASSERT_EQUAL(ESP_OK, esp_transport_set_func(stub, + stub_transport_connect, + stub_transport_read, + stub_transport_write, + stub_transport_close, + stub_transport_poll, + stub_transport_poll, + stub_transport_destroy)); + + esp_http_client_config_t config = { + .url = "http://example.com/", + .buffer_size_tx = 128, + .transport = stub, + }; + esp_http_client_handle_t client = esp_http_client_init(&config); + TEST_ASSERT_NOT_NULL(client); + + /* Clear defaults and set: one small header (fits), one huge header + * (alone too big for the buffer). */ + esp_http_client_set_header(client, "User-Agent", NULL); + esp_http_client_set_header(client, "Host", NULL); + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_set_header(client, "K1", "V1")); + + char huge_value[200]; + memset(huge_value, 'A', sizeof(huge_value) - 1); + huge_value[sizeof(huge_value) - 1] = '\0'; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_set_header(client, "X-Huge", huge_value)); + + esp_err_t err = esp_http_client_request_send(client, 0); + TEST_ASSERT_EQUAL(ESP_ERR_HTTP_HEADER_TOO_LONG, err); + + esp_http_client_cleanup(client); + esp_transport_destroy(stub); +} +#endif // CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT + +#endif // CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER + void app_main(void) { unity_run_menu(); diff --git a/components/esp_http_client/test_apps/main/test_http_header.c b/components/esp_http_client/test_apps/main/test_http_header.c new file mode 100644 index 00000000000..b6a342fcd7d --- /dev/null +++ b/components/esp_http_client/test_apps/main/test_http_header.c @@ -0,0 +1,77 @@ +/* + * SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/* + * Unit tests for http_header_generate_string() — cover the empty list, + * all-headers-fit and pagination paths. + */ + +#include +#include +#include "unity.h" +#include "http_header.h" + +TEST_CASE("http_header_generate_string: empty list returns 0 without touching buffer", "[http_header]") +{ + http_header_handle_t hdr = http_header_init(); + TEST_ASSERT_NOT_NULL(hdr); + + char buf[64] = "untouched"; + int buf_len = sizeof(buf); + int ret = http_header_generate_string(hdr, 0, buf, &buf_len); + + TEST_ASSERT_EQUAL_INT(0, ret); + TEST_ASSERT_EQUAL_INT((int)sizeof(buf), buf_len); + TEST_ASSERT_EQUAL_STRING("untouched", buf); + + http_header_destroy(hdr); +} + +TEST_CASE("http_header_generate_string: all headers fit in one call", "[http_header]") +{ + http_header_handle_t hdr = http_header_init(); + TEST_ASSERT_NOT_NULL(hdr); + TEST_ASSERT_EQUAL(ESP_OK, http_header_set(hdr, "K1", "V1")); + TEST_ASSERT_EQUAL(ESP_OK, http_header_set(hdr, "K2", "V2")); + + char buf[64] = {0}; + int buf_len = sizeof(buf); + int ret = http_header_generate_string(hdr, 0, buf, &buf_len); + + const char *expected = "K1: V1\r\nK2: V2\r\n\r\n"; + TEST_ASSERT_EQUAL_INT(2, ret); + TEST_ASSERT_EQUAL_STRING(expected, buf); + TEST_ASSERT_EQUAL_INT((int)strlen(expected), buf_len); + + http_header_destroy(hdr); +} + +TEST_CASE("http_header_generate_string: pagination across two calls", "[http_header]") +{ + http_header_handle_t hdr = http_header_init(); + TEST_ASSERT_NOT_NULL(hdr); + TEST_ASSERT_EQUAL(ESP_OK, http_header_set(hdr, "K1", "V1")); + TEST_ASSERT_EQUAL(ESP_OK, http_header_set(hdr, "K2", "V2")); + + /* 12-byte buffer fits one 8-byte header, but not both headers + terminator. */ + char buf[12] = {0}; + int buf_len = sizeof(buf); + int ret = http_header_generate_string(hdr, 0, buf, &buf_len); + + TEST_ASSERT_EQUAL_INT(1, ret); + TEST_ASSERT_EQUAL_STRING("K1: V1\r\n", buf); + TEST_ASSERT_EQUAL_INT((int)strlen("K1: V1\r\n"), buf_len); + + memset(buf, 0, sizeof(buf)); + buf_len = sizeof(buf); + ret = http_header_generate_string(hdr, 1, buf, &buf_len); + + TEST_ASSERT_EQUAL_INT(2, ret); + TEST_ASSERT_EQUAL_STRING("K2: V2\r\n\r\n", buf); + TEST_ASSERT_EQUAL_INT((int)strlen("K2: V2\r\n\r\n"), buf_len); + + http_header_destroy(hdr); +} diff --git a/components/esp_http_client/test_apps/pytest_esp_http_client_ut.py b/components/esp_http_client/test_apps/pytest_esp_http_client_ut.py index 84319c91157..d8f8aa32b3c 100644 --- a/components/esp_http_client/test_apps/pytest_esp_http_client_ut.py +++ b/components/esp_http_client/test_apps/pytest_esp_http_client_ut.py @@ -6,7 +6,7 @@ from pytest_embedded_idf.utils import idf_parametrize @pytest.mark.generic -@idf_parametrize('config', ['default'], indirect=['config']) +@idf_parametrize('config', ['default', 'strict_header'], indirect=['config']) @idf_parametrize('target', ['supported_targets'], indirect=['target']) def test_esp_http_client(dut: Dut) -> None: dut.run_all_single_board_cases() diff --git a/components/esp_http_client/test_apps/sdkconfig.ci.strict_header b/components/esp_http_client/test_apps/sdkconfig.ci.strict_header new file mode 100644 index 00000000000..fc86f5414d9 --- /dev/null +++ b/components/esp_http_client/test_apps/sdkconfig.ci.strict_header @@ -0,0 +1,2 @@ +CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER=y +CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT=y