Skip to content

Warn on http_response_code() after header('HTTP/...') #18962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
+48 −0
Closed

Warn on http_response_code() after header('HTTP/...') #18962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc15db4
Warn on http_response_code() after header('HTTP/...')
iluuu1994 Jun 27, 2025
8a9bdcf
[skip ci] Keep code for PHP 9.0 in comment
iluuu1994 Jul 22, 2025
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
8 changes: 8 additions & 0 deletions ext/standard/head.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,14 @@ PHP_FUNCTION(http_response_code)
}
RETURN_FALSE;
}

if (SG(sapi_headers).http_status_line) {
php_error_docref(NULL, E_WARNING, "Calling http_response_code() after header('HTTP/...') has no effect. This will change in PHP 9.0");
// TODO: Uncomment in PHP 9.0
// efree(SG(sapi_headers).http_status_line);
// SG(sapi_headers).http_status_line = NULL;
Comment on lines +380 to +383
Copy link
Member

Choose a reason for hiding this comment

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

That should be deprecation if it should change in 9.0 and it would also require RFC because the upcoming BC break. I think I would just add NOTICE for now and then leave it to anyone who want to do the RFC...

Copy link
Member Author

Choose a reason for hiding this comment

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

Your stance on this issue is seriously baffling to me. Almost every bug fix has some risk of breakage. That's why we delay more risky fixes to master, including myself in this case. If that's not enough, delaying to the next major with a migration path should certainly be enough. I even offered a mailing list discussion. But if the fixing of obvious issues now requires RFCs, I don't know how we can get anything done. This is not only wasteful to the RFC author, but everyone who has to read it.

That should be deprecation

I made this a warning because such code is almost guaranteed to be misbehaving, rather than this being a feature that is being removed.

}

zend_long old_response_code;

old_response_code = SG(sapi_headers).http_response_code;
Expand Down
40 changes: 40 additions & 0 deletions sapi/cli/tests/gh18582.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
--TEST--
GH-18582: Allow http_response_code() to clear HTTP start-line
--SKIPIF--
<?php
include "skipif.inc";
?>
--FILE--
<?php
include "php_cli_server.inc";

php_cli_server_start(<<<'PHP'
http_response_code(401);
header('HTTP/1.1 404 Not Found');
$is_404 = http_response_code(403);
$should_be_404_but_is_403 = http_response_code();
echo $is_404 . PHP_EOL;
echo $should_be_404_but_is_403 . PHP_EOL;
PHP);

$host = PHP_CLI_SERVER_HOSTNAME;
$fp = php_cli_server_connect();
if (fwrite($fp, "GET / HTTP/1.1\nHost: {$host}\n\n")) {
while (!feof($fp)) {
echo fgets($fp);
}
}
fclose($fp);
?>
--EXPECTF--
HTTP/1.1 404 Not Found
Host: %s
Date: %s
Connection: close
X-Powered-By: %s
Content-type: text/html; charset=UTF-8

<br />
<b>Warning</b>: http_response_code(): Calling http_response_code() after header('HTTP/...') has no effect. This will change in PHP 9.0 in <b>%s</b> on line <b>3</b><br />
404
403