Merge branch 'PHP-8.3' into PHP-8.4

php · Oct 3, 2024 · 139acce · 139acce
2 parents 6f7f32c + 4b7a906
commit 139acce
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 4 deletions.
diff --git a/NEWS b/NEWS
@@ -9,6 +9,10 @@ PHP                                                                        NEWS
     bypassable due to the environment variable collision). (CVE-2024-8927)
     (nielsdos)
 
+- CLI:
+  . Fixed bug GH-16137: duplicate http headers when set several times by
+    the client. (David Carlier)
+
 - Core:
   . Fixed bug GH-16040 (Use-after-free of object released in hook). (ilutov)
   . Fixed bug GH-16054 (Segmentation fault when resizing hash table iterator

diff --git 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
@@ -0,0 +1,20 @@
+--TEST--
+Bug GH-16137 duplicate *Forwarded* HTTP headers values.
+--INI--
+allow_url_fopen=1
+--SKIPIF--
+<?php
+include "skipif.inc";
+?>
+--FILE--
+<?php
+include "php_cli_server.inc";
+php_cli_server_start("echo \$_SERVER['HTTP_X_FORWARDED_FOR'];");
+$ctx = stream_context_create(array('http' => 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"