Respect response's status code #273
Conversation
In order to be able to verify that we're calling the function with the correct values.
Although we have tests covering multiple `Set-Cookie` headers, they are not verifying if the function `header()` is being called correctly.
|
In looking at this, I think there may be a better approach. Each of the two SAPI emitter implementations call the following: $this->emitStatusLine($response);
$this->emitHeaders($response);I'm thinking that perhaps those lines should be reversed, and Alternately, we keep your suggested solution, but also make those changes, to be 100% certain in situations where custom implementations call the methods in a different order. Thoughts? |
|
@weierophinney reversing the calls also fixes it indeed. However IMO is a bit fragile, specially having the possibility of creating custom emitters that shares the trait, and not really "natural" - I expect that the header stack would be status line and then the headers. I'd suggest to only force the response code when calling |
Well, Let's do the following:
This will cover all bases, I think. Thanks, @lcobucci ! |
If we don't pass the response status code to the function `header()` PHP might silently change the response code, which is not really nice since it should be up for the developers to define how they design the application. This silent change usually happens when one uses "Location" with a status code that's not 201 or 3xx, and some people already discussed a lot and essencially they've said that PHP will not modify its behaviour regarding this because it allows high level code to modify (fix) this. References: - https://bugs.php.net/bug.php?id=70273 - https://bugs.php.net/bug.php?id=51749 - https://bugs.php.net/bug.php?id=74535
To ensure that the emitted response will always be correct according with the response object.
lcobucci
force-pushed
the
respect-response-status-code
branch
from
4fad88e
to
35f5af1
Compare
|
@weierophinney done, thanks! |
Respect response's status code
Forward port #273 Conflicts: CHANGELOG.md
|
Thanks, @lcobucci! Merged, and released with 1.6.1. |
This basically ensures that
zend-diactorossends the status code toheader()while emitting the response headers, preventing PHP from overriding it silently.IMO this is a bug fix, since the emitted response is different from the created response.
We have a similar solution in
symfony/http-foundation: https://github.com/symfony/http-foundation/blob/dd97248b62153457d0716738d0dbcc0c2b31e285/Response.php#L331-L338More info: