From 3d80d98a1071e8007ae9f5b6840019c403f15437 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 1 Oct 2024 18:28:24 +0100 Subject: [PATCH] Fix GH-16137: "Deduplicate" http headers values but Set-Cookie. Those are meant to have 1 or plus values separated by a comma even if the client set them separately. close GH-16154 --- NEWS | 4 ++++ sapi/cli/php_cli_server.c | 27 +++++++++++++++++++++++---- sapi/cli/tests/gh16137.phpt | 20 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 sapi/cli/tests/gh16137.phpt diff --git a/NEWS b/NEWS index 19b6642ca1a2a..de78eabdadb08 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.2.25 +- CLI: + . Fixed bug GH-16137: duplicate http headers when set several times by + the client. (David Carlier) + - Core: . Fixed bug GH-15712: zend_strtod overflow with precision INI set on large value. (David Carlier) diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index 379ee70974d8e..732ac27c9a207 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1680,14 +1680,33 @@ static void php_cli_server_client_save_header(php_cli_server_client *client) { /* Wrap header value in a zval to add is to the HashTable which acts as an array */ zval tmp; - ZVAL_STR(&tmp, client->current_header_value); /* strip off the colon */ zend_string *lc_header_name = zend_string_tolower_ex(client->current_header_name, /* persistent */ true); GC_MAKE_PERSISTENT_LOCAL(lc_header_name); - /* Add the wrapped zend_string to the HashTable */ - zend_hash_add(&client->request.headers, lc_header_name, &tmp); - zend_hash_add(&client->request.headers_original_case, client->current_header_name, &tmp); + zval *entry = zend_hash_find(&client->request.headers, lc_header_name); + bool with_comma = !zend_string_equals_literal(lc_header_name, "set-cookie"); + + /** + * `Set-Cookie` HTTP header being the exception, they can have 1 or more values separated + * by a comma while still possibly be set separately by the client. + **/ + if (!with_comma || entry == NULL) { + ZVAL_STR(&tmp, client->current_header_value); + } 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); + + 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); + } + + /* 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_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.phpt b/sapi/cli/tests/gh16137.phpt new file mode 100644 index 0000000000000..165b8b1afd300 --- /dev/null +++ b/sapi/cli/tests/gh16137.phpt @@ -0,0 +1,20 @@ +--TEST-- +Bug GH-16137 duplicate *Forwarded* HTTP headers values. +--INI-- +allow_url_fopen=1 +--SKIPIF-- + +--FILE-- + array ( + 'method' => 'POST', + 'header' => array('x-forwarded-for: 127.0.0.1', 'x-forwarded-for: 192.168.1.254') +))); +var_dump(file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS, true, $ctx)); +?> +--EXPECT-- +string(24) "127.0.0.1, 192.168.1.254"