From d77f084065bf57e4d846dcd38e8b3dc97ad6ada0 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Tue, 25 Aug 2026 12:19:22 +0800 Subject: [PATCH 1/4] fix(esp_http_server): reject Transfer-Encoding requests with 501 --- .../esp_http_server/include/esp_http_server.h | 6 ++++-- components/esp_http_server/src/httpd_parse.c | 17 +++++++++++++++++ components/esp_http_server/src/httpd_txrx.c | 13 ++++++++++--- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/components/esp_http_server/include/esp_http_server.h b/components/esp_http_server/include/esp_http_server.h index bcebe2dd1aa..1ca768ae257 100644 --- a/components/esp_http_server/include/esp_http_server.h +++ b/components/esp_http_server/include/esp_http_server.h @@ -689,8 +689,10 @@ typedef enum { * guaranteed as the HTTP request may be partially received/parsed. * - The function must return ESP_OK if underlying socket needs to * be kept open. Any other value will ensure that the socket is - * closed. The return value is ignored when error is of type - * `HTTPD_500_INTERNAL_SERVER_ERROR` and the socket closed anyway. + * closed. The return value is ignored, and the socket is closed + * anyway, when the error is `HTTPD_500_INTERNAL_SERVER_ERROR` or + * `HTTPD_501_METHOD_NOT_IMPLEMENTED` (for 501 the request is only + * partially parsed, so the session cannot continue safely). * * @param[in] req HTTP request for which the error needs to be handled * @param[in] error Error type diff --git a/components/esp_http_server/src/httpd_parse.c b/components/esp_http_server/src/httpd_parse.c index a5be0cced71..8245ae3291a 100644 --- a/components/esp_http_server/src/httpd_parse.c +++ b/components/esp_http_server/src/httpd_parse.c @@ -55,6 +55,8 @@ typedef struct { size_t raw_datalen; /*!< Full length of the raw data in scratch buffer */ } parser_data_t; +static const char *httpd_find_hdr_value(struct httpd_req_aux *ra, const char *field); + static esp_err_t verify_url (http_parser *parser) { parser_data_t *parser_data = (parser_data_t *) parser->data; @@ -372,6 +374,21 @@ static esp_err_t cb_headers_complete(http_parser *parser) return ESP_FAIL; } + /* This server implements no transfer codings, so reject any request that + * carries a Transfer-Encoding header with 501 (RFC 9112 section 6.1). + * Without this check a chunked body is treated as an empty body and its + * chunk framing is parsed as a separate pipelined request, which is a + * request-smuggling primitive when the server sits behind a proxy. The + * header-presence check also covers codings that http_parser does not + * flag as chunked (e.g. "gzip, chunked"). */ + if ((parser->flags & F_CHUNKED) || + httpd_find_hdr_value(ra, "Transfer-Encoding") != NULL) { + ESP_LOGW(TAG, LOG_FMT("Transfer-Encoding is not supported")); + parser_data->error = HTTPD_501_METHOD_NOT_IMPLEMENTED; + parser_data->status = PARSING_FAILED; + return ESP_FAIL; + } + /* In absence of body/chunked encoding, http_parser sets content_len to ULLONG_MAX */ if (parser->content_length != ULLONG_MAX) { /* Content-Length was specified. Reject any value above UINT32_MAX: it is diff --git a/components/esp_http_server/src/httpd_txrx.c b/components/esp_http_server/src/httpd_txrx.c index ae2b42e8be4..cb630696265 100644 --- a/components/esp_http_server/src/httpd_txrx.c +++ b/components/esp_http_server/src/httpd_txrx.c @@ -603,9 +603,16 @@ esp_err_t httpd_req_handle_err(httpd_req_t *req, httpd_err_code_t error) if (hd->err_handler_fns[error]) { ret = hd->err_handler_fns[error](req, error); - /* If error code is 500, force return failure - * irrespective of the handler's return value */ - ret = (error == HTTPD_500_INTERNAL_SERVER_ERROR ? ESP_FAIL : ret); + /* Force failure irrespective of the handler's return value for + * errors after which the session cannot continue safely: 500 + * (internal state is broken) and 501 (parsing aborted mid-request, + * so the position of the next request in the byte stream is + * unknown and keep-alive would let leftover bytes be parsed and + * answered as a further request). */ + if (error == HTTPD_500_INTERNAL_SERVER_ERROR || + error == HTTPD_501_METHOD_NOT_IMPLEMENTED) { + ret = ESP_FAIL; + } } else { /* If no handler is registered for this error default * behavior is to send the HTTP error response and From 67ae553eabea317630bd709debb27d05b67af7d0 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Tue, 25 Aug 2026 12:20:02 +0800 Subject: [PATCH 2/4] test(esp_http_server): cover 501 reply for Transfer-Encoding requests --- .../test_apps/main/test_http_server.c | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) 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 778ca940b33..f2597336f3b 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 @@ -560,6 +560,126 @@ TEST_CASE("httpd_queue_work fast-fails on ctrl mbox saturation", "[HTTP SERVER]" TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); } +/* ---- Transfer-Encoding rejection (RFC 9112 section 6.1) ---- */ + +/* The server implements no transfer codings, so any request carrying a + * Transfer-Encoding header must be answered with 501. Without the parser + * check, a chunked body is misread as an empty body and its chunk framing + * is then parsed as a separate pipelined request — a request-smuggling + * primitive when the server sits behind a proxy. */ +static httpd_handle_t start_plain_test_server(uint16_t server_port, uint16_t ctrl_port) +{ + httpd_handle_t hd = NULL; + httpd_config_t config = HTTPD_DEFAULT_CONFIG(); + config.server_port = server_port; + config.ctrl_port = ctrl_port; + TEST_ASSERT_EQUAL(ESP_OK, httpd_start(&hd, &config)); + return hd; +} + +static esp_err_t te_ok_handler(httpd_req_t *req) +{ + return httpd_resp_sendstr(req, "OK"); +} + +/* Assert the captured wire data holds exactly one HTTP response and that the + * server closed the session afterwards. This is what proves the bytes after + * the rejected framing were NOT parsed and answered as a further request. */ +static void assert_single_response_then_close(const mock_server_response_t *resp) +{ + TEST_ASSERT_TRUE_MESSAGE(resp->len > 0, + "no response received before session close"); + TEST_ASSERT_TRUE_MESSAGE(resp->server_closed, + "server must close the session after 501"); + TEST_ASSERT_NULL_MESSAGE(strstr(resp->data + 1, "HTTP/1."), + "trailing bytes were answered as a second request"); +} + +TEST_CASE("Chunked request is rejected with 501", "[HTTP SERVER][security]") +{ + test_case_uses_tcpip(); + httpd_handle_t hd = start_plain_test_server(8098, ESP_HTTPD_DEF_CTRL_PORT + 17); + /* Register real handlers: without the parser-level reject, "POST /any" + * would succeed with an empty body (keep-alive stays up) and the chunk + * framing plus the pipelined GET would be parsed as further requests, + * producing a second response on the same connection. */ + httpd_uri_t post_any = { + .uri = "/any", .method = HTTP_POST, .handler = te_ok_handler, + }; + httpd_uri_t get_smuggled = { + .uri = "/smuggled", .method = HTTP_GET, .handler = te_ok_handler, + }; + TEST_ASSERT_EQUAL(ESP_OK, httpd_register_uri_handler(hd, &post_any)); + TEST_ASSERT_EQUAL(ESP_OK, httpd_register_uri_handler(hd, &get_smuggled)); + mock_server_request_t req = { + .data = "POST /any HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "5\r\nhello\r\n0\r\n\r\n" + "GET /smuggled HTTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n", + }; + mock_server_response_t *resp = mock_server_send_request(8098, &req); + TEST_ASSERT_NOT_NULL(resp); + assert_single_response_then_close(resp); + mock_server_assert_status(resp, 501); + mock_server_response_free(resp); + TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); +} + +static esp_err_t err_501_keepalive_handler(httpd_req_t *req, httpd_err_code_t error) +{ + httpd_resp_send_err(req, error, NULL); + /* Try to keep the session open. The server must ignore this for 501: + * parsing aborted mid-request, so the stream has no safe continuation. */ + return ESP_OK; +} + +TEST_CASE("Custom 501 handler cannot keep the session open", "[HTTP SERVER][security]") +{ + test_case_uses_tcpip(); + httpd_handle_t hd = start_plain_test_server(8100, ESP_HTTPD_DEF_CTRL_PORT + 19); + TEST_ASSERT_EQUAL(ESP_OK, httpd_register_err_handler(hd, HTTPD_501_METHOD_NOT_IMPLEMENTED, + err_501_keepalive_handler)); + mock_server_request_t req = { + .data = "POST /any HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "5\r\nhello\r\n0\r\n\r\n", + }; + mock_server_response_t *resp = mock_server_send_request(8100, &req); + TEST_ASSERT_NOT_NULL(resp); + assert_single_response_then_close(resp); + mock_server_assert_status(resp, 501); + mock_server_response_free(resp); + TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); +} + +TEST_CASE("Unknown Transfer-Encoding is rejected with 501", "[HTTP SERVER][security]") +{ + test_case_uses_tcpip(); + httpd_handle_t hd = start_plain_test_server(8099, ESP_HTTPD_DEF_CTRL_PORT + 18); + /* "gzip" does not set the parser's chunked flag; presence of the header + * alone must trigger the 501 so the body cannot be silently skipped. */ + mock_server_request_t req = { + .data = "POST /any HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: gzip\r\n" + "Content-Length: 5\r\n" + "\r\n" + "hello", + }; + mock_server_response_t *resp = mock_server_send_request(8099, &req); + TEST_ASSERT_NOT_NULL(resp); + assert_single_response_then_close(resp); + mock_server_assert_status(resp, 501); + mock_server_response_free(resp); + TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); +} + #ifdef CONFIG_HTTPD_WS_SUPPORT /* ------------------------------------------------------------------------- * * White-box fixtures for the dedicated control-frame handler. From 6dd4cfe95d77cc6898e963ccecb8b0b06bc3c7e7 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Fri, 4 Sep 2026 17:23:46 +0800 Subject: [PATCH 3/4] fix(esp_http_server): reply 501 to requests with an unrecognized method --- .../esp_http_server/include/esp_http_server.h | 15 ++++++------ components/esp_http_server/src/httpd_parse.c | 23 +++++++++++++------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/components/esp_http_server/include/esp_http_server.h b/components/esp_http_server/include/esp_http_server.h index 1ca768ae257..cd3299cdb37 100644 --- a/components/esp_http_server/include/esp_http_server.h +++ b/components/esp_http_server/include/esp_http_server.h @@ -380,7 +380,7 @@ esp_err_t httpd_stop(httpd_handle_t handle); */ typedef struct httpd_req { httpd_handle_t handle; /*!< Handle to server instance */ - int method; /*!< The type of HTTP request, -1 if unsupported method, HTTP_ANY for wildcard method to support every method */ + int method; /*!< The type of HTTP request (enum http_method); a request with an unrecognized method is rejected with 501 before any URI handler runs */ const char uri[CONFIG_HTTPD_MAX_URI_LEN + 1]; /*!< The URI of this request (1 byte extra for null termination) */ size_t content_len; /*!< Length of the request body */ void *aux; /*!< Internally used members */ @@ -612,10 +612,11 @@ typedef enum { */ HTTPD_500_INTERNAL_SERVER_ERROR = 0, - /* For methods not supported by http_parser. Presently - * http_parser halts parsing when such methods are - * encountered and so the server responds with 400 Bad - * Request error instead. + /* For request methods that http_parser does not recognize, + * and for requests carrying a Transfer-Encoding header (no + * transfer coding is implemented). Parsing aborted mid-request + * in both cases, so the server closes the session after the + * response regardless of what a custom error handler returns. */ HTTPD_501_METHOD_NOT_IMPLEMENTED, @@ -623,8 +624,8 @@ typedef enum { HTTPD_505_VERSION_NOT_SUPPORTED, /* Returned when http_parser halts parsing due to incorrect - * syntax of request, unsupported method in request URI or - * due to chunked encoding / upgrade field present in headers + * syntax of request or due to an upgrade field present in + * headers that the server does not handle */ HTTPD_400_BAD_REQUEST, diff --git a/components/esp_http_server/src/httpd_parse.c b/components/esp_http_server/src/httpd_parse.c index 8245ae3291a..2f8b2901f0b 100644 --- a/components/esp_http_server/src/httpd_parse.c +++ b/components/esp_http_server/src/httpd_parse.c @@ -68,12 +68,10 @@ static esp_err_t verify_url (http_parser *parser) const char *at = parser_data->last.at; size_t length = parser_data->last.length; + /* http_parser stops with HPE_INVALID_METHOD before the URL callback for + * any method token it does not know, so the method is always valid here. + * That parser error is mapped to 501 in parse_block(). */ r->method = parser->method; - if (r->method < 0) { - ESP_LOGW(TAG, LOG_FMT("HTTP method not supported (%d)"), r->method); - parser_data->error = HTTPD_501_METHOD_NOT_IMPLEMENTED; - return ESP_FAIL; - } if (sizeof(r->uri) < (length + 1)) { ESP_LOGW(TAG, LOG_FMT("URI length (%"NEWLIB_NANO_COMPAT_FORMAT") greater than supported (%"NEWLIB_NANO_COMPAT_FORMAT")"), @@ -600,6 +598,17 @@ static int read_block(httpd_req_t *req, http_parser *parser, size_t offset, size return nbytes; } +/* Map an http_parser failure to the HTTP status sent back. An unrecognized + * request method is a 501 (RFC 9110 section 15.6.2), every other parser + * error means the request syntax is malformed. */ +static httpd_err_code_t parser_errno_to_err_code(const http_parser *parser) +{ + if (HTTP_PARSER_ERRNO(parser) == HPE_INVALID_METHOD) { + return HTTPD_501_METHOD_NOT_IMPLEMENTED; + } + return HTTPD_400_BAD_REQUEST; +} + static int parse_block(http_parser *parser, size_t offset, size_t length) { parser_data_t *data = (parser_data_t *)(parser->data); @@ -661,14 +670,14 @@ static int parse_block(http_parser *parser, size_t offset, size_t length) return 0; } else if (nparsed != length) { /* http_parser error */ - data->error = HTTPD_400_BAD_REQUEST; + data->error = parser_errno_to_err_code(parser); data->status = PARSING_FAILED; ESP_LOGW(TAG, LOG_FMT("incomplete (%"NEWLIB_NANO_COMPAT_FORMAT"/%"NEWLIB_NANO_COMPAT_FORMAT") with parser error = %d"), NEWLIB_NANO_COMPAT_CAST(nparsed), NEWLIB_NANO_COMPAT_CAST(length), parser->http_errno); return -1; } else if (HTTP_PARSER_ERRNO(parser) != HPE_OK) { /* http_parser error */ - data->error = HTTPD_400_BAD_REQUEST; + data->error = parser_errno_to_err_code(parser); data->status = PARSING_FAILED; ESP_LOGE(TAG, LOG_FMT("parser error: %s"), http_errno_description(HTTP_PARSER_ERRNO(parser))); return -1; From 036cd477a3002e19e9267138959ce1ee917ecdc2 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Fri, 4 Sep 2026 17:24:16 +0800 Subject: [PATCH 4/4] test(esp_http_server): cover 501 reply for unrecognized request methods --- .../test_apps/main/test_http_server.c | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) 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 f2597336f3b..79fbcbed44b 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 @@ -680,6 +680,56 @@ TEST_CASE("Unknown Transfer-Encoding is rejected with 501", "[HTTP SERVER][secur TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); } +/* ---- Unrecognized request method (RFC 9110 section 15.6.2) ---- */ + +/* http_parser stops on a method token it does not know, before the URL + * callback ever runs. The server must map that parser error to 501, not to + * the generic 400 used for malformed syntax, and must close the session: + * parsing aborted mid-request, so the stream has no safe continuation. */ +TEST_CASE("Unknown HTTP method is rejected with 501", "[HTTP SERVER]") +{ + test_case_uses_tcpip(); + httpd_handle_t hd = start_plain_test_server(8101, ESP_HTTPD_DEF_CTRL_PORT + 20); + httpd_uri_t get_any = { + .uri = "/any", .method = HTTP_GET, .handler = te_ok_handler, + }; + TEST_ASSERT_EQUAL(ESP_OK, httpd_register_uri_handler(hd, &get_any)); + /* "FOO" is a syntactically valid token that no server implements. The + * pipelined GET behind it must never be answered. */ + mock_server_request_t req = { + .data = "FOO /any HTTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n" + "GET /any HTTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n", + }; + mock_server_response_t *resp = mock_server_send_request(8101, &req); + TEST_ASSERT_NOT_NULL(resp); + assert_single_response_then_close(resp); + mock_server_assert_status(resp, 501); + mock_server_response_free(resp); + TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); +} + +/* Guard: only the unknown-method parser error maps to 501. Any other + * request-line syntax error must still be answered with 400. */ +TEST_CASE("Malformed request line is rejected with 400", "[HTTP SERVER]") +{ + test_case_uses_tcpip(); + httpd_handle_t hd = start_plain_test_server(8102, ESP_HTTPD_DEF_CTRL_PORT + 21); + mock_server_request_t req = { + .data = "GET /any HTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n", + }; + mock_server_response_t *resp = mock_server_send_request(8102, &req); + TEST_ASSERT_NOT_NULL(resp); + mock_server_assert_status(resp, 400); + mock_server_response_free(resp); + TEST_ASSERT_EQUAL(ESP_OK, httpd_stop(hd)); +} + #ifdef CONFIG_HTTPD_WS_SUPPORT /* ------------------------------------------------------------------------- * * White-box fixtures for the dedicated control-frame handler.