Skip to content

Commit 3d80d98

Browse files
committed
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
1 parent c4bb075 commit 3d80d98

File tree

3 files changed

+47
-4
lines changed

3 files changed

+47
-4
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.2.25
44

5+
- CLI:
6+
. Fixed bug GH-16137: duplicate http headers when set several times by
7+
the client. (David Carlier)
8+
59
- Core:
610
. Fixed bug GH-15712: zend_strtod overflow with precision INI set on
711
large value. (David Carlier)

sapi/cli/php_cli_server.c

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,14 +1680,33 @@ static void php_cli_server_client_save_header(php_cli_server_client *client)
16801680
{
16811681
/* Wrap header value in a zval to add is to the HashTable which acts as an array */
16821682
zval tmp;
1683-
ZVAL_STR(&tmp, client->current_header_value);
16841683
/* strip off the colon */
16851684
zend_string *lc_header_name = zend_string_tolower_ex(client->current_header_name, /* persistent */ true);
16861685
GC_MAKE_PERSISTENT_LOCAL(lc_header_name);
16871686

1688-
/* Add the wrapped zend_string to the HashTable */
1689-
zend_hash_add(&client->request.headers, lc_header_name, &tmp);
1690-
zend_hash_add(&client->request.headers_original_case, client->current_header_name, &tmp);
1687+
zval *entry = zend_hash_find(&client->request.headers, lc_header_name);
1688+
bool with_comma = !zend_string_equals_literal(lc_header_name, "set-cookie");
1689+
1690+
/**
1691+
* `Set-Cookie` HTTP header being the exception, they can have 1 or more values separated
1692+
* by a comma while still possibly be set separately by the client.
1693+
**/
1694+
if (!with_comma || entry == NULL) {
1695+
ZVAL_STR(&tmp, client->current_header_value);
1696+
} else {
1697+
zend_string *curval = Z_STR_P(entry);
1698+
zend_string *newval = zend_string_safe_alloc(1, ZSTR_LEN(curval), ZSTR_LEN(client->current_header_value) + 2, /* persistent */true);
1699+
1700+
memcpy(ZSTR_VAL(newval), ZSTR_VAL(curval), ZSTR_LEN(curval));
1701+
memcpy(ZSTR_VAL(newval) + ZSTR_LEN(curval), ", ", 2);
1702+
memcpy(ZSTR_VAL(newval) + ZSTR_LEN(curval) + 2, ZSTR_VAL(client->current_header_value), ZSTR_LEN(client->current_header_value) + 1);
1703+
1704+
ZVAL_STR(&tmp, newval);
1705+
}
1706+
1707+
/* Add/Update the wrapped zend_string to the HashTable */
1708+
zend_hash_update(&client->request.headers, lc_header_name, &tmp);
1709+
zend_hash_update(&client->request.headers_original_case, client->current_header_name, &tmp);
16911710

16921711
zend_string_release_ex(lc_header_name, /* persistent */ true);
16931712
zend_string_release_ex(client->current_header_name, /* persistent */ true);

sapi/cli/tests/gh16137.phpt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Bug GH-16137 duplicate *Forwarded* HTTP headers values.
3+
--INI--
4+
allow_url_fopen=1
5+
--SKIPIF--
6+
<?php
7+
include "skipif.inc";
8+
?>
9+
--FILE--
10+
<?php
11+
include "php_cli_server.inc";
12+
php_cli_server_start("echo \$_SERVER['HTTP_X_FORWARDED_FOR'];");
13+
$ctx = stream_context_create(array('http' => array (
14+
'method' => 'POST',
15+
'header' => array('x-forwarded-for: 127.0.0.1', 'x-forwarded-for: 192.168.1.254')
16+
)));
17+
var_dump(file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS, true, $ctx));
18+
?>
19+
--EXPECT--
20+
string(24) "127.0.0.1, 192.168.1.254"

0 commit comments

Comments
 (0)