Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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
Expand Down
3 changes: 2 additions & 1 deletion ext/standard/tests/general_functions/bug43293_1.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ array(3) {
[2]=>
int(3)
}
bool(false)
array(0) {
}
30 changes: 16 additions & 14 deletions main/php_variables.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
/* }}} */
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion php.ini-development
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion php.ini-production
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion sapi/cli/php_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion sapi/embed/php_embed.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
68 changes: 68 additions & 0 deletions sapi/fpm/tests/bug75712-getenv-server-vars_001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
--TEST--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: I usually prefer not to use 001 names in FPM to see immediately from the test name what it is for which was quite useful for me. But it's not a huge issue if there are few such tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are already some tests that are even less explaining...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to avoid those either, but in this case the two tests are very closely related and only differ in the INI value, so numbering them made sense to me.

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of this echo..?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to do formatting in tests but if you feel it's important, it should be added to printBody (adding new parameter). It's a NIT though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the echo is to clearly delimit the body output to distinguish what is being printed by the subprocess vs what is being printed by the test script to make sure that the Deprecation is coming from the subprocess.

$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();
?>
Original file line number Diff line number Diff line change
@@ -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--
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion sapi/phpdbg/phpdbg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion tests/basic/011.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions tests/basic/011_empty_query.phpt
Original file line number Diff line number Diff line change
@@ -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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(substr(PHP_OS, 0, 3) == 'WIN') die("skip on windows: --INI-- is ignored due to 4b9cd27ff5c0177dcb160caeae1ea79e761ada58");
if(substr(PHP_OS, 0, 3) == 'WIN') die("skip on windows: --INI-- is ignored due to 4b9cd27ff5c0177dcb160caeae1ea79e761ada58");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with tests/basic/011.phpt (which itself is consistent with ext/date/tests/date_default_timezone_get-1.phpt).

?>
--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) {
}
22 changes: 22 additions & 0 deletions tests/basic/011_register_argc_argv_disabled.phpt
Original file line number Diff line number Diff line change
@@ -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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(substr(PHP_OS, 0, 3) == 'WIN') die("skip on windows: --INI-- is ignored due to 4b9cd27ff5c0177dcb160caeae1ea79e761ada58");
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
3 changes: 2 additions & 1 deletion tests/basic/011_windows.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions tests/basic/012_register_argc_argv_disabled.phpt
Original file line number Diff line number Diff line change
@@ -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"
}