Everything

php

header() allows arbitrary status codes

php.internals

Christoph Becker

4 years ago

Hi all,

a while ago it has been reported[1] that our header() function actually
allows arbitrary status codes, which may even overflow.  Of course, that
makes no sense, since the status code is supposed to be a three digit
code.  So this ticket has been followed up by a pull request[2], and
Jakub suggested to further restrict the status code to be in range 100 -
599.

Since this could break some pathological cases, I wanted to ask whether
anybody objects to this change for the master branch (i.e. PHP 8.2).

[1] <https://bugs.php.net/bug.php?id=81645>
[2] <https://github.com/php/php-src/pull/7676>

Christoph

Ayesh - PHP.Watch

4 years ago

>
> Hi all,
>
> a while ago it has been reported[1] that our header() function actually
> allows arbitrary status codes, which may even overflow.  Of course, that
> makes no sense, since the status code is supposed to be a three digit
> code.  So this ticket has been followed up by a pull request[2], and
> Jakub suggested to further restrict the status code to be in range 100 -
> 599.
>
> Since this could break some pathological cases, I wanted to ask whether
> anybody objects to this change for the master branch (i.e. PHP 8.2).
>
> [1] <https://bugs.php.net/bug.php?id=81645>
> [2] <https://github.com/php/php-src/pull/7676>

I think it is a useful improvement.  Should we adjust to
http_response_code to match this behavior?

Christoph Becker

4 years ago

On 21.12.2021 at 20:09, Ayesh Karunaratne wrote:

>> a while ago it has been reported[1] that our header() function actually
>> allows arbitrary status codes, which may even overflow.  Of course, that
>> makes no sense, since the status code is supposed to be a three digit
>> code.  So this ticket has been followed up by a pull request[2], and
>> Jakub suggested to further restrict the status code to be in range 100 -
>> 599.
>>
>> Since this could break some pathological cases, I wanted to ask whether
>> anybody objects to this change for the master branch (i.e. PHP 8.2).
>>
>> [1] <https://bugs.php.net/bug.php?id=81645>
>> [2] <https://github.com/php/php-src/pull/7676>
>
> I think it is a useful improvement.  Should we adjust to
> http_response_code to match this behavior?

Oh, good catch!  I think we should.

Christoph

David Gebler

4 years ago

On Tue, Dec 21, 2021 at 6:59 PM Christoph M. Becker <cmbecker69@gmx.de>
wrote:

> Hi all,
>
> a while ago it has been reported[1] that our header() function actually
> allows arbitrary status codes, which may even overflow.  Of course, that
> makes no sense, since the status code is supposed to be a three digit
> code.  So this ticket has been followed up by a pull request[2], and
> Jakub suggested to further restrict the status code to be in range 100 -
> 599.
>

Personally, I don't like restricting the status code to a number in the
100-599 range. As far as I know, RFC 7230 doesn't mandate anything beyond
the requirement of 3 digits and while 7231 may only specify semantics for
1xx-5xx, that doesn't mean there isn't a legitimate use-case in custom or
internal applications for status codes outside the usual semantics.

The overflow part is a legit bug which can and should be fixed, but I'd at
least question whether a user should be obliged to stick to conventional
HTTP semantics via the header() function, or even a strictly conformant
implementation of the standards defined 7320. Maybe this behaviour could be
default but overridable via a new, fourth optional parameter or something,
I don't know...but I can easily imagine someone having a legitimate context
in which they want to send status codes outside the usual range
representing custom semantics.

Paul Dragoonis

4 years ago

On Thu, 23 Dec 2021, 00:06 David Gebler, <davidgebler@gmail.com> wrote:

> On Tue, Dec 21, 2021 at 6:59 PM Christoph M. Becker <cmbecker69@gmx.de>
> wrote:
>
> > Hi all,
> >
> > a while ago it has been reported[1] that our header() function actually
> > allows arbitrary status codes, which may even overflow.  Of course, that
> > makes no sense, since the status code is supposed to be a three digit
> > code.  So this ticket has been followed up by a pull request[2], and
> > Jakub suggested to further restrict the status code to be in range 100 -
> > 599.
> >
>
> Personally, I don't like restricting the status code to a number in the
> 100-599 range. As far as I know, RFC 7230 doesn't mandate anything beyond
> the requirement of 3 digits and while 7231 may only specify semantics for
> 1xx-5xx, that doesn't mean there isn't a legitimate use-case in custom or
> internal applications for status codes outside the usual semantics.
>
> The overflow part is a legit bug which can and should be fixed, but I'd at
> least question whether a user should be obliged to stick to conventional
> HTTP semantics via the header() function, or even a strictly conformant
> implementation of the standards defined 7320. Maybe this behaviour could be
> default but overridable via a new, fourth optional parameter or something,
> I don't know...but I can easily imagine someone having a legitimate context
> in which they want to send status codes outside the usual range
> representing custom semantics.
>

I think its safe to say we should restrict the overflow parts.

As for boundaries; I don't know who is or isn't using their own custom
status codes. It's unlikely, but entirely possible. As such, this is
considered a BC break.

If we apply restrictions to a minor release, then upgrading will be harder
for 8.2 if we include this in the next release.

You could say 98% of people are using standard ranges for status codes, but
we do have to always take into account the 2% on our decisions.

Thanks.

Hans Henrik Bergan

4 years ago

sometime in the future HTTP 6xx will be defined, and we'll have to add a
big warning to the header()/http_respons_code() pages like
"Warning: HTTP 6.x.x is only supported in PHP >= x.x.x and PHP <=8.1.x",
and library developers have to add fugly code like
`if(PHP_VERSION_MAJOR >= X || (PHP_VERSION_MAJOR <=8 && PHP_VERSION_MINOR
<= 1){http_response_code(6xx);}else{
trigger_error("your php version cannot use http 6xx");
}

i'd prefer if we didn't restrict the header ranges

On Thu, 23 Dec 2021 at 13:47, Paul Dragoonis <dragoonis@gmail.com> wrote:

Hans Henrik Bergan

4 years ago

another thing, it wouldn't surprise me if someone at some point want to
emulate some IIS-specific codes in PHP, like
header("HTTP/1.1 401.3 Unauthorized due to ACL on resource.");

it'd be a shame if PHP literally cannot send IIS-errorcodes

On Thu, 23 Dec 2021 at 16:40, Hans Henrik Bergan <divinity76@gmail.com>
wrote:

Christoph Becker

4 years ago

On 23.12.2021 at 16:53, Hans Henrik Bergan wrote:

> another thing, it wouldn't surprise me if someone at some point want to
> emulate some IIS-specific codes in PHP, like
> header("HTTP/1.1 401.3 Unauthorized due to ACL on resource.");
>
> it'd be a shame if PHP literally cannot send IIS-errorcodes

I don't think that this works right now, and I'm not absolute convinced
that we should add support for this.  It seems to me that this violates
RFC 7230, section 3.1.2[1].

After consideration, I agree, though, that it's better to not restrict
the range to 100-599, and maybe we should allow for more or less than 3
digits, and just catch potential overflow.

[1] <https://datatracker.ietf.org/doc/html/rfc7230#section-3.1.2>

Christoph