Skip to content

Commit 37bf0ec

Browse files
TimWollaTimWolla
authored
main: Deprecate deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string (#19606)
* main: Ignore `register_argc_argv` when `SG(request_info).argc` is available * sapi: Remove hardcoded `register_argc_argv` for CLI SAPIs This INI is ignored since the previous commit, which makes the hardcoded setting obsolete. * main: Deprecate deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_register_argc_argv_ini_directive * main: Adjust deprecation message for `register_argc_argv` * NEWS/UPGRADING
1 parent 3f66cbe commit 37bf0ec

16 files changed

+192
-24
lines changed

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ PHP NEWS
1313
deprecated. (alexandre-daubois)
1414
. Fixed bug GH-19681 (PHP_EXPAND_PATH broken with bash 5.3.0). (Remi)
1515
. Marks the stack as non-executable on Haiku. (David Carlier)
16+
. Deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string is
17+
now deprecated. (timwolla, nicolasgrekas)
1618

1719
- CLI:
1820
. Fixed bug GH-19461 (Improve error message on listening error with IPv6

‎UPGRADING‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,11 @@ PHP 8.5 UPGRADE NOTES
387387
. Using null as an array offset or when calling array_key_exists() is now
388388
deprecated. Instead an empty string should be used.
389389
RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_using_values_null_as_an_array_offset_and_when_calling_array_key_exists
390+
. Deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string for non-CLI
391+
SAPIs has been deprecated. Configure register_argc_argv=0 and switch to either
392+
$_GET or $_SERVER['QUERY_STRING'] to access the information, after verifying
393+
that the usage is safe.
394+
RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_register_argc_argv_ini_directive
390395

391396
- Curl:
392397
. The curl_close() function has been deprecated, as CurlHandle objects are

‎ext/standard/tests/general_functions/bug43293_1.phpt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,5 @@ array(3) {
2121
[2]=>
2222
int(3)
2323
}
24-
bool(false)
24+
array(0) {
25+
}

‎main/php_variables.c‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -785,10 +785,13 @@ static void php_autoglobal_merge(HashTable *dest, HashTable *src)
785785
PHPAPI zend_result php_hash_environment(void)
786786
{
787787
memset(PG(http_globals), 0, sizeof(PG(http_globals)));
788+
/* Register $argc and $argv for CLI SAPIs. $_SERVER['argc'] and $_SERVER['argv']
789+
* will be registered in php_auto_globals_create_server() which clears
790+
* PG(http_globals)[TRACK_VARS_SERVER] anyways, making registration at this point
791+
* useless.
792+
*/
793+
php_build_argv(NULL, NULL);
788794
zend_activate_auto_globals();
789-
if (PG(register_argc_argv)) {
790-
php_build_argv(SG(request_info).query_string, &PG(http_globals)[TRACK_VARS_SERVER]);
791-
}
792795
return SUCCESS;
793796
}
794797
/* }}} */
@@ -875,19 +878,18 @@ static bool php_auto_globals_create_server(zend_string *name)
875878
if (PG(variables_order) && (strchr(PG(variables_order),'S') || strchr(PG(variables_order),'s'))) {
876879
php_register_server_variables();
877880

878-
if (PG(register_argc_argv)) {
879-
if (SG(request_info).argc) {
880-
zval *argc, *argv;
881+
if (SG(request_info).argc) {
882+
zval *argc, *argv;
881883

882-
if ((argc = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGC), 1)) != NULL &&
883-
(argv = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGV), 1)) != NULL) {
884-
Z_ADDREF_P(argv);
885-
zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGV), argv);
886-
zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGC), argc);
887-
}
888-
} else {
889-
php_build_argv(SG(request_info).query_string, &PG(http_globals)[TRACK_VARS_SERVER]);
884+
if ((argc = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGC), 1)) != NULL &&
885+
(argv = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGV), 1)) != NULL) {
886+
Z_ADDREF_P(argv);
887+
zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGV), argv);
888+
zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGC), argc);
890889
}
890+
} else if (PG(register_argc_argv)) {
891+
zend_error(E_DEPRECATED, "Deriving $_SERVER['argv'] from the query string is deprecated. Configure register_argc_argv=0 to turn this message off");
892+
php_build_argv(SG(request_info).query_string, &PG(http_globals)[TRACK_VARS_SERVER]);
891893
}
892894

893895
} else {

‎php.ini-development‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,7 @@ request_order = "GP"
662662
; enabled, registering these variables consumes CPU cycles and memory each time
663663
; a script is executed. For security reasons, this feature should be disabled
664664
; for non-CLI SAPIs.
665-
; Note: This directive is hardcoded to On for the CLI SAPI
665+
; Note: This directive is ignored for the CLI SAPI
666666
; This directive is deprecated.
667667
; https://php.net/register-argc-argv
668668
;register_argc_argv = Off

‎php.ini-production‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ request_order = "GP"
664664
; enabled, registering these variables consumes CPU cycles and memory each time
665665
; a script is executed. For security reasons, this feature should be disabled
666666
; for non-CLI SAPIs.
667-
; Note: This directive is hardcoded to On for the CLI SAPI
667+
; Note: This directive is ignored for the CLI SAPI
668668
; This directive is deprecated.
669669
; https://php.net/register-argc-argv
670670
;register_argc_argv = Off

‎sapi/cli/php_cli.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ PHP_CLI_API cli_shell_callbacks_t *php_cli_get_shell_callbacks(void)
115115

116116
static const char HARDCODED_INI[] =
117117
"html_errors=0\n"
118-
"register_argc_argv=1\n"
119118
"implicit_flush=1\n"
120119
"output_buffering=0\n"
121120
"max_execution_time=0\n"

‎sapi/embed/php_embed.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
static const char HARDCODED_INI[] =
2727
"html_errors=0\n"
28-
"register_argc_argv=1\n"
2928
"implicit_flush=1\n"
3029
"output_buffering=0\n"
3130
"max_execution_time=0\n"

‎sapi/fpm/tests/bug75712-getenv-server-vars_001.phpt‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
--TEST--
2+
FPM: bug75712 - getenv should not read from $_ENV and $_SERVER
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = static
16+
pm.max_children = 1
17+
env[TEST] = test
18+
php_value[register_argc_argv] = on
19+
php_value[html_errors] = off
20+
EOT;
21+
22+
$code = <<<EOT
23+
<?php
24+
25+
var_dump(isset(getenv()['argv']));
26+
var_dump(isset(getenv()['SERVER_NAME']));
27+
var_dump(getenv()['TEST']);
28+
var_dump(isset(getenv()['DTEST']));
29+
var_dump(getenv('DTEST'));
30+
putenv('DTEST=dt');
31+
var_dump(getenv()['DTEST']);
32+
var_dump(getenv('DTEST'));
33+
34+
function notcalled()
35+
{
36+
\$_SERVER['argv'];
37+
}
38+
EOT;
39+
40+
$tester = new FPM\Tester($cfg, $code);
41+
$tester->start();
42+
$tester->expectLogStartNotices();
43+
$response = $tester->request();
44+
echo "=====", PHP_EOL;
45+
$response->printBody();
46+
echo "=====", PHP_EOL;
47+
$tester->terminate();
48+
$tester->close();
49+
50+
?>
51+
Done
52+
--EXPECTF--
53+
=====
54+
Deprecated: Deriving $_SERVER['argv'] from the query string is deprecated. Configure register_argc_argv=0 to turn this message off in %s on line %d
55+
bool(false)
56+
bool(true)
57+
string(4) "test"
58+
bool(false)
59+
bool(false)
60+
string(2) "dt"
61+
string(2) "dt"
62+
=====
63+
Done
64+
--CLEAN--
65+
<?php
66+
require_once "tester.inc";
67+
FPM\Tester::clean();
68+
?>

sapi/fpm/tests/bug75712-getenv-server-vars.phpt renamed to sapi/fpm/tests/bug75712-getenv-server-vars_002.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
FPM: bug75712 - getenv should not read from $_ENV and $_SERVER
2+
FPM: bug75712 - getenv should not read from $_ENV and $_SERVER (register_argc_argv=off)
33
--SKIPIF--
44
<?php include "skipif.inc"; ?>
55
--FILE--
@@ -15,7 +15,7 @@ listen = {{ADDR}}
1515
pm = static
1616
pm.max_children = 1
1717
env[TEST] = test
18-
php_value[register_argc_argv] = on
18+
php_value[register_argc_argv] = off
1919
EOT;
2020

2121
$code = <<<EOT

0 commit comments

Comments
 (0)