Edit report at https://bugs.php.net/bug.php?id=51749&edit=1
ID: 51749
Updated by: m...@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] m...@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] m...@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
