Respect explicit response status code when Location response header is given (PHP SAPI)

[jkrzefski]

Let's say, this is your code:

$app->get('/foo', function () {
    return new \React\Http\Message\Response(202, [
        'Location' => '/bar',
    ]);
});

• When you use ReactPHP as webserver, the status code will be 202 (as expected).
• When you use PHP's built-in webserver, the status code will be 302 (unexpected).

So, why does this happen?

When you use header(...) to set the Location header, PHP will update the status code to 302. This behavior is even mentioned in the official documentation: PHP: header - Manual

The second special case is the "Location:" header. Not only does it send this header back to the browser, but it also returns a REDIRECT (302) status code to the browser unless the 201 or a 3xx status code has already been set.

Here is the corresponding code in php/php-src: https://github.com/php/php-src/blob/php-8.1.11/main/SAPI.c#L806-L821

Should this happen?

I think, this behavior is more confusing than it is helpful. It should not be the decision of PHP to override my status code. According to RFC 9110, the Location header has a defined behavior for the status codes 201 and 3xx.

But that is not an exclusive definition. Any HTTP response may contain a Location header, but the expected behavior is not defined by this RFC. That means, it can be used by a proprietary API definition for other status codes.

What does this change do?

The status code is now set after setting all other headers. This will override any status code that has priorly been set by PHP. So now my choice of status code is respected.

[jkrzefski]

Would you mind, adding the hacktoberfest topic to this repository?

[SimonFrings]

Hey @jkrzefski, thanks for bringing this up 👍

First up I want to take the time to tell you that I really appreciate the amount of information you presented, really helps understanding where you're coming from and what your intentions are, nice job 💪

Can you also add tests to confirm that your changes work as expected. In this case you need to add some acceptance tests inside the tests/acceptance.sh in combination with the examples/index.php, you can look into a260ce7 for reference or check the history of examples/index.php for more information.

[jkrzefski]

Hi @SimonFrings,

thank you for your feedback. I will have a look at the tests and add my case there.

[jkrzefski]

@SimonFrings I added some test cases and ran them with and without my change. Prior to my change, the case for status 202 fails. With my change it passes. Is this what you had in mind for the tests?

[SimonFrings]

@jkrzefski I tested this with PHP's development web server and agree with you that if you explicitly define the status code, it shouldn't be changed by PHP in this case. I can also confirm that this is covered by your tests, you got my approval 👍

[clue]

@jkrzefski Thank you very much for this high-quality PR! This is indeed a nasty one and I'm glad we could come up with some test cases to confirm that rearranging a single line of code actually addresses this subtle behavior in PHP. Keep it up! 👍