main: Deprecate deriving $_SERVER['argc'] and $_SERVER['argv'] from the query string by TimWolla · Pull Request #19606 · php/php-src

TimWolla · 2025-08-27T12:56:54Z

RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_register_argc_argv_ini_directive

main/php_variables.c

…ilable

This INI is ignored since the previous commit, which makes the hardcoded setting obsolete.

…he query string RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_register_argc_argv_ini_directive

nicolas-grekas · 2025-09-01T15:43:07Z

main/php_variables.c

+				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['argc'] and $_SERVER['argv'] from $_SERVER['QUERY_STRING'] is deprecated, configure register_argc_argv=0 to suppress this message and access the query parameters via $_SERVER['QUERY_STRING'] or $_GET");


Trying to make the message shorter while still accurate.
Accessing the query parameters might be a bad suggestion (as that might reintroduce the security issue)

Suggested change

zend_error(E_DEPRECATED, "Deriving $_SERVER['argc'] and $_SERVER['argv'] from $_SERVER['QUERY_STRING'] is deprecated, configure register_argc_argv=0 to suppress this message and access the query parameters via $_SERVER['QUERY_STRING'] or $_GET");

zend_error(E_DEPRECATED, "Deriving $_SERVER['argv'] from the query string is deprecated, configure register_argc_argv=0 to turn this off.");

I would go with thisshortened variant, but also split it up in two sentences as I originally suggested, and maybe "this" -> "this message" ?

zend_error(E_DEPRECATED, "Deriving $_SERVER['argv'] from the query string is deprecated. Configure register_argc_argv=0 to turn this message off");

(make sure not to include the . at the end!)

"this message", but also this deriving from behavior; that's why I didn't add message in my suggestion (but I understand it makes things a bit more implicit, while accurate :)

I've used Derick's suggestion and used UPGRADING to put a little more explanation there.

Girgias

The implementation looks OK. I will let others judge the deprecation message content.

nicolas-grekas · 2025-09-08T11:26:22Z

UPGRADING

+  . 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.


DanielEScherzer

RM approval, technical review not performed
Scanned the code and saw some extra whitespace in tests

DanielEScherzer · 2025-09-09T08:20:53Z

tests/basic/011_empty_query.phpt

+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");


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");

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

DanielEScherzer · 2025-09-09T08:21:57Z

tests/basic/011_register_argc_argv_disabled.phpt

+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");


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");

bukka

Just few NITs

bukka · 2025-09-09T08:16:12Z

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

+$response = $tester->request();
+echo "=====", PHP_EOL;
+$response->printBody();
+echo "=====", PHP_EOL;


What's the point of this echo..?

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.

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.

bukka · 2025-09-09T08:23:24Z

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

@@ -0,0 +1,68 @@
+--TEST--


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.

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

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.

TimWolla force-pushed the argc-argv-from-query branch from 6900d7e to c1f22ff Compare August 27, 2025 12:59

derickr reviewed Aug 27, 2025

View reviewed changes

main/php_variables.c Outdated Show resolved Hide resolved

TimWolla force-pushed the argc-argv-from-query branch from c1f22ff to 090baf3 Compare August 27, 2025 13:58

TimWolla mentioned this pull request Aug 27, 2025

main: Change the register_argc_argv INI default to Off #19473

Merged

TimWolla force-pushed the argc-argv-from-query branch 2 times, most recently from 90a3ccd to 6086512 Compare September 1, 2025 15:10

github-actions bot added the SAPI: fpm label Sep 1, 2025

TimWolla added 3 commits September 1, 2025 17:25

main: Ignore register_argc_argv when SG(request_info).argc is ava…

6ea0eb9

…ilable

sapi: Remove hardcoded register_argc_argv for CLI SAPIs

026d470

This INI is ignored since the previous commit, which makes the hardcoded setting obsolete.

main: Deprecate deriving $_SERVER['argc'] and $_SERVER['argv'] from t…

85f7cb1

…he query string RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_register_argc_argv_ini_directive

TimWolla force-pushed the argc-argv-from-query branch from 6086512 to 85f7cb1 Compare September 1, 2025 15:27

github-actions bot added Extension: standard SAPI: cli SAPI: phpdbg labels Sep 1, 2025

nicolas-grekas reviewed Sep 1, 2025

View reviewed changes

TimWolla marked this pull request as ready for review September 2, 2025 08:46

TimWolla requested a review from bukka as a code owner September 2, 2025 08:46

TimWolla requested a review from Girgias September 2, 2025 08:46

TimWolla mentioned this pull request Sep 2, 2025

[Tracking] Pending PHP 8.5 deprecations #19468

Closed

17 tasks

Girgias approved these changes Sep 3, 2025

View reviewed changes

TimWolla added 3 commits September 8, 2025 12:50

Merge branch 'master' into argc-argv-from-query

d97f297

main: Adjust deprecation message for register_argc_argv

3556021

NEWS/UPGRADING

767ea7f

TimWolla requested review from a team and nicolas-grekas and removed request for nicolas-grekas September 8, 2025 10:58

nicolas-grekas reviewed Sep 8, 2025

View reviewed changes

alexandre-daubois approved these changes Sep 9, 2025

View reviewed changes

DanielEScherzer approved these changes Sep 9, 2025

View reviewed changes

bukka approved these changes Sep 9, 2025

View reviewed changes

TimWolla merged commit 37bf0ec into php:master Sep 9, 2025
9 checks passed

TimWolla deleted the argc-argv-from-query branch September 9, 2025 08:36

jrfnl mentioned this pull request Oct 8, 2025

Account for changes introduced in PHP 8.5 PHPCompatibility/PHPCompatibility#1849

Open

70 tasks

TimWolla mentioned this pull request Oct 24, 2025

Fix GH-20279: register_argc_argv set at 0 on sapi cli after deprecation. #20280

Closed

jrfnl mentioned this pull request Oct 27, 2025

PHP 8.5 | RemovedIniDirectives: detect use of register_argc_argv (RFC) PHPCompatibility/PHPCompatibility#1954

Merged

	zend_error(E_DEPRECATED, "Deriving $_SERVER['argc'] and $_SERVER['argv'] from $_SERVER['QUERY_STRING'] is deprecated, configure register_argc_argv=0 to suppress this message and access the query parameters via $_SERVER['QUERY_STRING'] or $_GET");
	zend_error(E_DEPRECATED, "Deriving $_SERVER['argv'] from the query string is deprecated, configure register_argc_argv=0 to turn this off.");

	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");

Conversation

TimWolla commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielEScherzer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bukka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

TimWolla commented Aug 27, 2025 •

edited

Loading