Merge branch 'contrib/github_pr_18684' into 'master'

feat(httpd): avoid useless string copy by introducing httpd_req_get_url_query_str_ptr() (GitHub PR)

Closes IDFGH-17758

See merge request espressif/esp-idf!49682
This commit is contained in:
Mahavir Jain
2026-06-30 18:15:48 +05:30
3 changed files with 321 additions and 95 deletions
@@ -15,9 +15,10 @@
#include "freertos/FreeRTOS.h"
#include "freertos/semphr.h"
#ifdef CONFIG_HTTPD_WS_SUPPORT
/* http_parser.h must precede esp_httpd_priv.h, which embeds a
* struct http_parser_url by value */
#include <http_parser.h>
#include "../../src/esp_httpd_priv.h"
#endif
#include "unity.h"
#include "test_utils.h"
@@ -670,6 +671,141 @@ TEST_CASE("WS handshake invalid Sec-WebSocket-Key returns 400", "[HTTP SERVER][w
#endif /* CONFIG_HTTPD_WS_SUPPORT */
/********* URL query / header pointer-accessor tests *********
* These exercise httpd_req_get_url_query_str_ptr() and
* httpd_req_get_hdr_value_str_ptr() against hand-built requests that mirror
* the parser's output: the query is parsed with http_parser_parse_url() (the
* exact mechanism the server uses), and the header scratch buffer reproduces
* the parser layout ("Field: value" with the CRLF terminators replaced by
* null bytes). The new pointer APIs are cross-checked against the existing
* copy/length variants so any divergence is caught.
*
* Note: these assume CONFIG_HTTPD_VALIDATE_REQ is disabled (the default), so
* httpd_valid_req() accepts the stack request used here. */
/* Parse a query-carrying URI into a stack request, like verify_url() does */
static void build_query_req(httpd_req_t *req, struct httpd_req_aux *aux, const char *uri)
{
memset(req, 0, sizeof(*req));
memset(aux, 0, sizeof(*aux));
req->aux = aux;
strlcpy((char *)req->uri, uri, sizeof(req->uri));
http_parser_url_init(&aux->url_parse_res);
TEST_ASSERT_EQUAL(0, http_parser_parse_url(req->uri, strlen(req->uri), 0,
&aux->url_parse_res));
}
TEST_CASE("httpd_req_get_url_query_str_ptr returns query without copy", "[HTTP SERVER]")
{
httpd_req_t req;
struct httpd_req_aux aux;
const char *expected = "foo=bar&baz=qux";
build_query_req(&req, &aux, "/path?foo=bar&baz=qux");
const char *q = NULL;
size_t qlen = 0;
TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_url_query_str_ptr(&req, &q, &qlen));
TEST_ASSERT_NOT_NULL(q);
TEST_ASSERT_EQUAL(strlen(expected), qlen);
TEST_ASSERT_EQUAL(0, memcmp(q, expected, qlen));
/* Must point into the request URI buffer, i.e. no copy was made */
TEST_ASSERT_TRUE(q >= req.uri && q < req.uri + sizeof(req.uri));
/* Cross-check against the length and copy variants */
TEST_ASSERT_EQUAL(qlen, httpd_req_get_url_query_len(&req));
char buf[64];
TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_url_query_str(&req, buf, sizeof(buf)));
TEST_ASSERT_EQUAL(0, strncmp(buf, q, qlen));
}
TEST_CASE("httpd_req_get_url_query_str_ptr handles empty and missing query", "[HTTP SERVER]")
{
httpd_req_t req = {0};
struct httpd_req_aux aux = {0};
req.aux = &aux;
const char *q = NULL;
size_t qlen = 0;
/* Empty URI -> ESP_FAIL */
TEST_ASSERT_EQUAL(ESP_FAIL, httpd_req_get_url_query_str_ptr(&req, &q, &qlen));
/* URI without a query -> ESP_ERR_NOT_FOUND */
build_query_req(&req, &aux, "/path/only");
TEST_ASSERT_EQUAL(ESP_ERR_NOT_FOUND, httpd_req_get_url_query_str_ptr(&req, &q, &qlen));
}
TEST_CASE("httpd_req_get_url_query_str_ptr validates NULL args", "[HTTP SERVER][security]")
{
httpd_req_t req;
struct httpd_req_aux aux;
build_query_req(&req, &aux, "/p?x=1");
const char *q = NULL;
size_t qlen = 0;
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, httpd_req_get_url_query_str_ptr(NULL, &q, &qlen));
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, httpd_req_get_url_query_str_ptr(&req, NULL, &qlen));
/* Regression: a query is present, so a NULL buf_len used to be written to */
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, httpd_req_get_url_query_str_ptr(&req, &q, NULL));
}
TEST_CASE("httpd_req_get_hdr_value_str_ptr returns value without copy", "[HTTP SERVER]")
{
httpd_req_t req = {0};
struct httpd_req_aux aux = {0};
/* Parser layout: "Field: value" entries, CRLF terminators replaced by nulls */
static char scratch[] = "Host: example.com\0\0X-Custom: hello world";
aux.scratch = scratch;
aux.req_hdrs_count = 2;
req.aux = &aux;
const char *val = NULL;
size_t vlen = 0;
TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_hdr_value_str_ptr(&req, "X-Custom", &val, &vlen));
TEST_ASSERT_EQUAL(strlen("hello world"), vlen);
TEST_ASSERT_EQUAL(0, strcmp(val, "hello world"));
/* Must point into the scratch buffer, i.e. no copy was made */
TEST_ASSERT_TRUE(val >= scratch && val < scratch + sizeof(scratch));
/* Field match is case-insensitive, like the copy variant */
TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_hdr_value_str_ptr(&req, "host", &val, &vlen));
TEST_ASSERT_EQUAL(0, strcmp(val, "example.com"));
/* Cross-check the new pointer API against the length and copy variants */
TEST_ASSERT_EQUAL(strlen("hello world"), httpd_req_get_hdr_value_len(&req, "X-Custom"));
char buf[32];
TEST_ASSERT_EQUAL(ESP_OK, httpd_req_get_hdr_value_str(&req, "X-Custom", buf, sizeof(buf)));
TEST_ASSERT_EQUAL(0, strcmp(buf, "hello world"));
/* Missing field -> ESP_ERR_NOT_FOUND */
TEST_ASSERT_EQUAL(ESP_ERR_NOT_FOUND,
httpd_req_get_hdr_value_str_ptr(&req, "Nonexistent", &val, &vlen));
}
TEST_CASE("httpd_req_get_hdr_value_str_ptr validates NULL args", "[HTTP SERVER][security]")
{
httpd_req_t req = {0};
struct httpd_req_aux aux = {0};
static char scratch[] = "X-Custom: hello world";
aux.scratch = scratch;
aux.req_hdrs_count = 1;
req.aux = &aux;
const char *val = NULL;
size_t vlen = 0;
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
httpd_req_get_hdr_value_str_ptr(NULL, "X-Custom", &val, &vlen));
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
httpd_req_get_hdr_value_str_ptr(&req, NULL, &val, &vlen));
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
httpd_req_get_hdr_value_str_ptr(&req, "X-Custom", NULL, &vlen));
/* Regression-style guard: a value is present, so a NULL val_len must be
* rejected rather than written to */
TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG,
httpd_req_get_hdr_value_str_ptr(&req, "X-Custom", &val, NULL));
}
void app_main(void)
{
unity_run_menu();