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

NEWS

@@ -13,6 +13,8 @@ PHP                                                                        NEWS
     deprecated. (alexandre-daubois)
   . Fixed bug GH-19681 (PHP_EXPAND_PATH broken with bash 5.3.0). (Remi)
   . Marks the stack as non-executable on Haiku. (David Carlier)
+  . Deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string is
+    now deprecated. (timwolla, nicolasgrekas)
 
 - CLI:
   . Fixed bug GH-19461 (Improve error message on listening error with IPv6

UPGRADING

@@ -387,6 +387,11 @@ PHP 8.5 UPGRADE NOTES
   . Using null as an array offset or when calling array_key_exists() is now
     deprecated. Instead an empty string should be used.
     RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_using_values_null_as_an_array_offset_and_when_calling_array_key_exists
+  . Deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string for non-CLI
+    SAPIs has been deprecated. Configure register_argc_argv=0 and switch to either
+    $_GET or $_SERVER['QUERY_STRING'] to access the information, after verifying
+    that the usage is safe.
+    RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_register_argc_argv_ini_directive
 
 - Curl:
   . The curl_close() function has been deprecated, as CurlHandle objects are

ext/standard/tests/general_functions/bug43293_1.phpt

@@ -21,4 +21,5 @@ array(3) {
   [2]=>
   int(3)
 }
-bool(false)
+array(0) {
+}

main/php_variables.c

@@ -785,10 +785,13 @@ static void php_autoglobal_merge(HashTable *dest, HashTable *src)
 PHPAPI zend_result php_hash_environment(void)
 {
 	memset(PG(http_globals), 0, sizeof(PG(http_globals)));
+	/* Register $argc and $argv for CLI SAPIs. $_SERVER['argc'] and $_SERVER['argv']
+	 * will be registered in php_auto_globals_create_server() which clears
+	 * PG(http_globals)[TRACK_VARS_SERVER] anyways, making registration at this point
+	 * useless.
+	 */
+	php_build_argv(NULL, NULL);
 	zend_activate_auto_globals();
-	if (PG(register_argc_argv)) {
-		php_build_argv(SG(request_info).query_string, &PG(http_globals)[TRACK_VARS_SERVER]);
-	}
 	return SUCCESS;
 }
 /* }}} */
@@ -875,19 +878,18 @@ static bool php_auto_globals_create_server(zend_string *name)
 	if (PG(variables_order) && (strchr(PG(variables_order),'S') || strchr(PG(variables_order),'s'))) {
 		php_register_server_variables();
 
-		if (PG(register_argc_argv)) {
-			if (SG(request_info).argc) {
-				zval *argc, *argv;
+		if (SG(request_info).argc) {
+			zval *argc, *argv;
 
-				if ((argc = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGC), 1)) != NULL &&
-					(argv = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGV), 1)) != NULL) {
-					Z_ADDREF_P(argv);
-					zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGV), argv);
-					zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGC), argc);
-				}
-			} else {
-				php_build_argv(SG(request_info).query_string, &PG(http_globals)[TRACK_VARS_SERVER]);
+			if ((argc = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGC), 1)) != NULL &&
+				(argv = zend_hash_find_ex_ind(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_ARGV), 1)) != NULL) {
+				Z_ADDREF_P(argv);
+				zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGV), argv);
+				zend_hash_update(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]), ZSTR_KNOWN(ZEND_STR_ARGC), argc);
 			}
+		} else if (PG(register_argc_argv)) {
+			zend_error(E_DEPRECATED, "Deriving $_SERVER['argv'] from the query string is deprecated. Configure register_argc_argv=0 to turn this message off");
+			php_build_argv(SG(request_info).query_string, &PG(http_globals)[TRACK_VARS_SERVER]);
 		}
 
 	} else {

php.ini-development

@@ -662,7 +662,7 @@ request_order = "GP"
 ; enabled, registering these variables consumes CPU cycles and memory each time
 ; a script is executed. For security reasons, this feature should be disabled
 ; for non-CLI SAPIs.
-; Note: This directive is hardcoded to On for the CLI SAPI
+; Note: This directive is ignored for the CLI SAPI
 ; This directive is deprecated.
 ; https://php.net/register-argc-argv
 ;register_argc_argv = Off

php.ini-production

@@ -664,7 +664,7 @@ request_order = "GP"
 ; enabled, registering these variables consumes CPU cycles and memory each time
 ; a script is executed. For security reasons, this feature should be disabled
 ; for non-CLI SAPIs.
-; Note: This directive is hardcoded to On for the CLI SAPI
+; Note: This directive is ignored for the CLI SAPI
 ; This directive is deprecated.
 ; https://php.net/register-argc-argv
 ;register_argc_argv = Off

sapi/cli/php_cli.c

@@ -115,7 +115,6 @@ PHP_CLI_API cli_shell_callbacks_t *php_cli_get_shell_callbacks(void)
 
 static const char HARDCODED_INI[] =
 	"html_errors=0\n"
-	"register_argc_argv=1\n"
 	"implicit_flush=1\n"
 	"output_buffering=0\n"
 	"max_execution_time=0\n"

sapi/embed/php_embed.c

@@ -25,7 +25,6 @@
 
 static const char HARDCODED_INI[] =
 	"html_errors=0\n"
-	"register_argc_argv=1\n"
 	"implicit_flush=1\n"
 	"output_buffering=0\n"
 	"max_execution_time=0\n"

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

@@ -0,0 +1,68 @@
+--TEST--
+FPM: bug75712 - getenv should not read from $_ENV and $_SERVER
+--SKIPIF--
+<?php include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = 1
+env[TEST] = test
+php_value[register_argc_argv] = on
+php_value[html_errors] = off
+EOT;
+
+$code = <<<EOT
+<?php
+
+var_dump(isset(getenv()['argv']));
+var_dump(isset(getenv()['SERVER_NAME']));
+var_dump(getenv()['TEST']);
+var_dump(isset(getenv()['DTEST']));
+var_dump(getenv('DTEST'));
+putenv('DTEST=dt');
+var_dump(getenv()['DTEST']);
+var_dump(getenv('DTEST'));
+
+function notcalled()
+{
+    \$_SERVER['argv'];
+}
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$response = $tester->request();
+echo "=====", PHP_EOL;
+$response->printBody();
+echo "=====", PHP_EOL;
+$tester->terminate();
+$tester->close();
+
+?>
+Done
+--EXPECTF--
+=====
+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
+bool(false)
+bool(true)
+string(4) "test"
+bool(false)
+bool(false)
+string(2) "dt"
+string(2) "dt"
+=====
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

sapi/fpm/tests/bug75712-getenv-server-vars_002.phpt

@@ -1,5 +1,5 @@
 --TEST--
-FPM: bug75712 - getenv should not read from $_ENV and $_SERVER
+FPM: bug75712 - getenv should not read from $_ENV and $_SERVER (register_argc_argv=off)
 --SKIPIF--
 <?php include "skipif.inc"; ?>
 --FILE--
@@ -15,7 +15,7 @@ listen = {{ADDR}}
 pm = static
 pm.max_children = 1
 env[TEST] = test
-php_value[register_argc_argv] = on
+php_value[register_argc_argv] = off
 EOT;
 
 $code = <<<EOT

sapi/phpdbg/phpdbg.c

@@ -982,7 +982,6 @@ static const opt_struct OPTIONS[] = { /* {{{ */
 
 const char phpdbg_ini_hardcoded[] =
 "html_errors=Off\n"
-"register_argc_argv=On\n"
 "implicit_flush=On\n"
 "display_errors=Off\n"
 "log_errors=On\n"

tests/basic/011.phpt

@@ -18,7 +18,8 @@ for ($i=0; $i<$argc; $i++) {
 }
 
 ?>
---EXPECT--
+--EXPECTF--
+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
 0: ab
 1: cd
 2: ef

tests/basic/011_empty_query.phpt

@@ -0,0 +1,20 @@
+--TEST--
+Testing $argc and $argv handling (GET empty)
+--SKIPIF--
+<?php
+    if(substr(PHP_OS, 0, 3) == 'WIN') die("skip on windows: --INI-- is ignored due to 4b9cd27ff5c0177dcb160caeae1ea79e761ada58");
+?>
+--INI--
+register_argc_argv=1
+--CGI--
+--FILE--
+<?php
+
+var_dump($_SERVER['argc'], $_SERVER['argv']);
+
+?>
+--EXPECTF--
+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
+int(0)
+array(0) {
+}

tests/basic/011_register_argc_argv_disabled.phpt

@@ -0,0 +1,22 @@
+--TEST--
+Testing $argc and $argv handling (GET, register_argc_argv=0)
+--SKIPIF--
+<?php
+    if(substr(PHP_OS, 0, 3) == 'WIN') die("skip on windows: --INI-- is ignored due to 4b9cd27ff5c0177dcb160caeae1ea79e761ada58");
+?>
+--INI--
+register_argc_argv=0
+--GET--
+ab+cd+ef+123+test
+--FILE--
+<?php
+
+var_dump($_SERVER['argc'], $_SERVER['argv']);
+
+?>
+--EXPECTF--
+Warning: Undefined array key "argc" in %s on line %d
+
+Warning: Undefined array key "argv" in %s on line %d
+NULL
+NULL

tests/basic/011_windows.phpt

@@ -14,7 +14,8 @@ for ($i=0; $i<$argc; $i++) {
 }
 
 ?>
---EXPECT--
+--EXPECTF--
+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
 0: foo=ab
 1: cd
 2: ef

tests/basic/012_register_argc_argv_disabled.phpt

@@ -0,0 +1,49 @@
+--TEST--
+Testing $argc and $argv handling (cli, register_argc_argv=0)
+--INI--
+register_argc_argv=0
+variables_order=GPS
+--ARGS--
+ab cd ef 123 test
+--FILE--
+<?php
+
+var_dump(
+    $argc,
+    $argv,
+    $_SERVER['argc'],
+    $_SERVER['argv'],
+);
+
+?>
+--EXPECTF--
+int(6)
+array(6) {
+  [0]=>
+  string(%d) "%s"
+  [1]=>
+  string(2) "ab"
+  [2]=>
+  string(2) "cd"
+  [3]=>
+  string(2) "ef"
+  [4]=>
+  string(3) "123"
+  [5]=>
+  string(4) "test"
+}
+int(6)
+array(6) {
+  [0]=>
+  string(%d) "%s"
+  [1]=>
+  string(2) "ab"
+  [2]=>
+  string(2) "cd"
+  [3]=>
+  string(2) "ef"
+  [4]=>
+  string(3) "123"
+  [5]=>
+  string(4) "test"
+}