From 099f6886d8d4d65fb63b650899737f838496c7c0 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 2 Nov 2026 03:43:16 +0000 Subject: [PATCH 1/3] sapi/cli: php_cli_server_client_save_header() crash on repeated headers. The merged value was not marked persistent local, which tripped the ZEND_RC_DEBUG assertions, and the second value leaked. Spellings differing only by case also left headers_original_case pointing at the value freed by the headers update. Repeats now update the entry of the first spelling. --- sapi/cli/php_cli_server.c | 15 ++++++++- sapi/cli/tests/gh16137_duplicate_headers.phpt | 32 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 sapi/cli/tests/gh16137_duplicate_headers.phpt diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index 6b17f083c125..7c6c44cf06e6 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1701,17 +1701,30 @@ static void php_cli_server_client_save_header(php_cli_server_client *client) } else { zend_string *curval = Z_STR_P(entry); zend_string *newval = zend_string_safe_alloc(1, ZSTR_LEN(curval), ZSTR_LEN(client->current_header_value) + 2, /* persistent */true); + GC_MAKE_PERSISTENT_LOCAL(newval); memcpy(ZSTR_VAL(newval), ZSTR_VAL(curval), ZSTR_LEN(curval)); memcpy(ZSTR_VAL(newval) + ZSTR_LEN(curval), ", ", 2); memcpy(ZSTR_VAL(newval) + ZSTR_LEN(curval) + 2, ZSTR_VAL(client->current_header_value), ZSTR_LEN(client->current_header_value) + 1); ZVAL_STR(&tmp, newval); + zend_string_release_ex(client->current_header_value, /* persistent */ true); + } + + zend_string *orig = client->current_header_name; + if (entry != NULL) { + zend_string *key; + ZEND_HASH_MAP_FOREACH_STR_KEY(&client->request.headers_original_case, key) { + if (zend_string_equals_ci(key, client->current_header_name)) { + orig = key; + break; + } + } ZEND_HASH_FOREACH_END(); } /* Add/Update the wrapped zend_string to the HashTable */ zend_hash_update(&client->request.headers, lc_header_name, &tmp); - zend_hash_update(&client->request.headers_original_case, client->current_header_name, &tmp); + zend_hash_update(&client->request.headers_original_case, orig, &tmp); zend_string_release_ex(lc_header_name, /* persistent */ true); zend_string_release_ex(client->current_header_name, /* persistent */ true); diff --git a/sapi/cli/tests/gh16137_duplicate_headers.phpt b/sapi/cli/tests/gh16137_duplicate_headers.phpt new file mode 100644 index 000000000000..0c7c2f81a82d --- /dev/null +++ b/sapi/cli/tests/gh16137_duplicate_headers.phpt @@ -0,0 +1,32 @@ +--TEST-- +GH-16137: CLI server handling of repeated request headers +--SKIPIF-- + +--FILE-- + ['header' => [ + 'X-Foo: a', + 'X-Foo: b', + 'Cookie: Foo', + 'cOOKIE: Bar', + 'Set-Cookie: first', + 'set-cookie: second', +]]]); +echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS, false, $ctx); +echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS); +?> +--EXPECT-- +array(3) { + ["X-Foo"]=> + string(4) "a, b" + ["Cookie"]=> + string(8) "Foo, Bar" + ["Set-Cookie"]=> + string(6) "second" +} +array(0) { +} From 226a7399333e3fc6906d22223856dce57e5b6b7f Mon Sep 17 00:00:00 2001 From: David Carlier Date: Mon, 2 Nov 2026 10:31:40 +0000 Subject: [PATCH 2/3] ditch the hash loop, while preserving stable branch behavior. --- sapi/cli/php_cli_server.c | 19 +++++-------------- sapi/cli/tests/gh16137_duplicate_headers.phpt | 6 +++++- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index 7c6c44cf06e6..a4571305d819 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1408,9 +1408,8 @@ static void php_cli_server_request_ctor(php_cli_server_request *req) /* {{{ */ req->query_string = NULL; req->query_string_len = 0; zend_hash_init(&req->headers, 0, NULL, cli_header_value_dtor, 1); - /* No destructor is registered as the value pointed by is the same as for &req->headers */ GC_MAKE_PERSISTENT_LOCAL(&req->headers); - zend_hash_init(&req->headers_original_case, 0, NULL, NULL, 1); + zend_hash_init(&req->headers_original_case, 0, NULL, cli_header_value_dtor, 1); GC_MAKE_PERSISTENT_LOCAL(&req->headers_original_case); req->content = NULL; req->content_len = 0; @@ -1711,20 +1710,12 @@ static void php_cli_server_client_save_header(php_cli_server_client *client) zend_string_release_ex(client->current_header_value, /* persistent */ true); } - zend_string *orig = client->current_header_name; - if (entry != NULL) { - zend_string *key; - ZEND_HASH_MAP_FOREACH_STR_KEY(&client->request.headers_original_case, key) { - if (zend_string_equals_ci(key, client->current_header_name)) { - orig = key; - break; - } - } ZEND_HASH_FOREACH_END(); - } - /* Add/Update the wrapped zend_string to the HashTable */ zend_hash_update(&client->request.headers, lc_header_name, &tmp); - zend_hash_update(&client->request.headers_original_case, orig, &tmp); + /* A repeat spelled differently leaves the first spelling's entry in place while the + * update above releases its value, so this table must own a reference too. */ + zend_string_addref(Z_STR(tmp)); + zend_hash_update(&client->request.headers_original_case, client->current_header_name, &tmp); zend_string_release_ex(lc_header_name, /* persistent */ true); zend_string_release_ex(client->current_header_name, /* persistent */ true); diff --git a/sapi/cli/tests/gh16137_duplicate_headers.phpt b/sapi/cli/tests/gh16137_duplicate_headers.phpt index 0c7c2f81a82d..5fdc149de37b 100644 --- a/sapi/cli/tests/gh16137_duplicate_headers.phpt +++ b/sapi/cli/tests/gh16137_duplicate_headers.phpt @@ -20,12 +20,16 @@ echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS, false, $ctx); echo file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS); ?> --EXPECT-- -array(3) { +array(5) { ["X-Foo"]=> string(4) "a, b" ["Cookie"]=> + string(3) "Foo" + ["cOOKIE"]=> string(8) "Foo, Bar" ["Set-Cookie"]=> + string(5) "first" + ["set-cookie"]=> string(6) "second" } array(0) { From c85a918ed772e3ec7c5301d647fb1167e4c0dbe1 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 3 Nov 2026 18:09:56 +0000 Subject: [PATCH 3/3] clarifying comment --- sapi/cli/php_cli_server.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index a4571305d819..671d5d5cd15f 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1712,8 +1712,9 @@ static void php_cli_server_client_save_header(php_cli_server_client *client) /* Add/Update the wrapped zend_string to the HashTable */ zend_hash_update(&client->request.headers, lc_header_name, &tmp); - /* A repeat spelled differently leaves the first spelling's entry in place while the - * update above releases its value, so this table must own a reference too. */ + /* Each table owns a reference to the new value. The old value can outlive the update + * above, as a repeat spelled differently keeps the first spelling's entry in + * headers_original_case. */ zend_string_addref(Z_STR(tmp)); zend_hash_update(&client->request.headers_original_case, client->current_header_name, &tmp);