Edit report at https://bugs.php.net/bug.php?id=51749&edit=1
ID: 51749
Updated by: mike@php.net
Reported by: theimp at iinet dot net dot au
Summary: header("Location:") changing HTTP status
-Status: Open
+Status: Not a bug
Type: Bug
Package: HTTP related
Operating System: Debian Lenny
PHP Version: 5.3.2
Block user comment: N
Private report: N
New Comment:
So, nothing new here, closing.
Previous Comments:
------------------------------------------------------------------------
[2010-05-30 00:41:43] theimp at iinet dot net dot au
The patch no-http-status-code-overwrite-on-location-header (last revision 2010-05-29 20:00 UTC) is a
patch to main/SAPI.c from 5.3.2 (04 Mar 2010).
The relevant changes are lines 661 to 665 (661 to 668 in the original), and 675 to 678 (678 in the
original).
I deliberately chose the simplest method I could; it simply checks to see if the status is currently
200, and if so, then it updates the status code as before. Otherwise, it does not.
As mentioned, the behavior that is the subject of this bug has been a non-problem for a very long
time. I am more and more inclined to agree with mike at php dot net that "fixing" it just
might not be worth it, when compared against the possibility of failures related to
backwards-compatibility expectations. But aharvey at php dot net does have a valid point too.
A method I considered, but rejected as too inefficient (considering the benefit), involved creating
a new variable to store the status when set, and only write it just before the headers are sent.
Another similar method that I considered, which was more efficient, but that I rejected for being
too dangerous and fault-prone, was making the default HTTP response value 0 (or any other invalid
value) and, again, updating it to 200 if it was still set to 0 (meaning that it hadn't been
updated) before the headers were about to be sent.
These changes involve much more significant modification for very little additional benefit. The
only case where they would improve the patch code is if you set the HTTP code to some number, and
then set the HTTP code back to 200, and then sent a Location or similar header; you might expect it
not to change (because you've explicitly set it), but it would change (because it's
currently 200, and that's the only condition that is checked).
(Also, the code on lines 665-666 of the original is redundant in the current code because it would
always be performed again at 752).
Please note that I am not necessarily saying that I think this patch should be applied. In fact,
after reading the source code, I think that - if anything - my complaint really should simply be a
documentation bug (ie; that setting a Status header, last, is the preferred method of setting the
response code, and clarifications related to the correct use of http_response_code).
(Should have read the source code first, I guess. For example, it's now painfully clear that
PHP follows RFC 3875 as closely as possible).
Thank you for your consideration.
------------------------------------------------------------------------
[2010-05-20 09:17:43] mike@php.net
Heh, you're quite patient either ;)
I think the most SAPI-portable way to set the HTTP status is:
header("Status: NNN", true, NNN);
Still, special headers like Location and WWW-Authenticate might override it again, so it's best
to set the HTTP status at last.
If you want to know more about the "hack", visit sapi_header_op() in main/SAPI.c
Cheers
PS: patches are always welcome, even if the just cause a discussion without following changes
------------------------------------------------------------------------
[2010-05-20 08:51:06] theimp at iinet dot net dot au
> header("HTTP/1.1 XXX") is just a hack
I did not realize this. So does that mean that, per RFC 3875, we're only supposed to set the
Status header and hope that the server does what we expect?
The documentation specifically lists this "hack" as the correct way to supply the
Status-Line and, therefore, the Response Code. But I'm not going to argue with you about this.
> I see no hard reason to introduce other hacks to support a hack in the first place.
Well, that's fair enough.
> You are writing *pages*
I apologize. I tend to use far more words than I have to, in anticipation of explaining myself
poorly otherwise. I will try to be more concise. Many of the details I felt were essential for
understanding how the fix SHOULD be handled, distinct from my personal preferences.
> about code that's *years* old and worked that way for a long long time, and there's
> very little chance to become that changed.
I understand from this, that you do not want to fix this in the way I have suggested, which is fair
enough; it doesn't seem to bother anyone else and has trivial workarounds, and its very status
as a bug is more an matter of opinion than fact.
I'm bad at interpreting subtlety, though, and so I am not clear if I should also conclude that:
1. This bug is closed, we've got more important things to fix, and this is technically not even
broken. Stop bothering me!
or:
2. If you want it fixed so bad, then submit a patch yourself and we'll see if it's not too
convoluted to keep.
I am not trying to be offensive, and understand that you weren't, either.
Thank you for your patience.
------------------------------------------------------------------------
[2010-05-18 09:47:14] mike@php.net
Maybe, but header("HTTP/1.1 XXX") is just a hack and I see no hard reason to introduce
other hacks to support a hack in the first place.
You are writing *pages* about code that's *years* old and worked that way for a long long time,
and there's very little chance to become that changed. You know, PHP is an acronym for BC, or
was it the other way round...
------------------------------------------------------------------------
[2010-05-12 14:19:48] theimp at iinet dot net dot au
@ mike at php dot net
I did more testing, and yes, if you use the additional parameters on the occasion that you send the
location header, the special handling of the Location header does not override the explicit
behavior.
So:
header("HTTP/1.1 503 Service Unavailable", true, 503);
header("Location: http://www.php.net/");
Does not work; but:
header("HTTP/1.1 503 Service Unavailable");
header("Location: http://www.php.net/", true,
503);
Does work.
Obvious, in hindsight. But very confusing at first. The documentation for http_response_code simply
says: "Forces the HTTP response code to the specified value"; I read that as forcing the
response code irrespective of any other later action other than another http_response_code.
It's not quite a documentation bug, since it's not strictly wrong, but it should probably
be changed, because it is easy to read other than as intended. I would accept changing this bug to a
documentation bug.
@ aharvey at php dot net
The functionality I expected exists; I simply did not understand it. But I do agree with what you
say also; it is questionable whether it should behave the way that it does even aside from other
functionality.
To really know how this should be treated requires, I think, consideration of the points I have
previously mentioned. Presumably, this specific behavior was put into PHP for a reason, and it was
not changed much when the opportunity last arose. I do not know the specific goals of the PHP
project in this respect.
I would not have written this bug report if I had realized the correct way to use the
http_response_code parameter.
Also, the workaround mentioned in bug #25044 is still possible.
Finally, I had not properly considered RFC 3875 when I first created this bug report. If I had, I
would probably have decided that the behavior is deliberate and was not expected to be fixed.
The http_response_code is confusing, since it can be set on unrelated headers, making it difficult
to track down the code that sets it since it could be a place other than where you set the Response
Line itself (in principle, any header; practically, any Location header in addition to the Response
Line header).
Also, the HTTP Response Code that you want to set must be known at the time you want to set the
Location header, which might not be known at that time. It might have already been set in another
function; there is no way to retrieve the response code that you have set, so you have to remember
it yourself by assigning it to a variable and re-using that variable at the time you set the
Location header.
For example:
...
if ($_SERVER["HTTPS"] != "on") {
setstatus(426);
setlocation("https");
}
...
function setstatus($status)
{
switch ($status)
{
...
case 426:
header("HTTP/1.1 426 Upgrade Required");
break;
...
}
}
...
function setlocation($scheme)
{
switch ($scheme)
{
...
case "https":
header('Location: $scheme://$url');
break;
...
}
}
A better solution may have been, rather than to add the http_response_code parameter, to have added
a force_response() function to optionally set the HTTP Response Code, which would not be
overwritten. But we are long past the point that it makes sense to add such a function; it would add
no new functionality and would deprecate existing uses of unrelated code.
While I do think that PHP should not set the Response Code after a Location header, there are still
reasons to consider this behavior appropriate (standards compliance and backwards-compatibility),
which I have already discussed at length.
On the other hand; fixing this "bug" in a way similar to the one I suggested would make
PHP more robust and make it much more likely that people get the behavior that they expect after
they read all of the relevant documentation. It may also help with bug-finding in the case of
incorrect header output, and simplify some functions, depending on how they have been designed.
Thank you all for taking the time to consider this bug.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=51749
--
Edit this bug report at https://bugs.php.net/bug.php?id=51749&edit=1