-
-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not imply having parsed the request body #27
Conversation
Note: this didn’t break any tests, probably because we aren’t doing a lot of testing on POST requests. Do we need a better way to test for the results of specific HTTP requests? How would we introduce testing for the changes I propose here? |
84c26af
to
f39c4b8
Compare
Turns out StyleCI’s Symfony presets include a bunch of “blank line before” rules, while PHP CS Fixer actually disables all of them except for before Changed the rules to match StyleCI. @Nyholm let me know if you’d rather change StyleCI settings than PHP CS Fixer settings. I am not sure which one is seen as leading here—or by Symfony. This will always be BC breaking. Although on the parameter side I merely widened the allowed input, which in itself will not break BC (and is in line with LSP), there may be minor breakage in the output as Although this is a BC break from the PSR7 Server perspective, the |
a2f9ee1
to
798e852
Compare
Strictly follow the PSR-7 guidance where $_POST is used for a request’s parsed body only for very specific requests. All other requests should result in a null value and leave other parsing up to application logic.
798e852
to
6ee9d73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. Thank you
\strtolower(\trim(\explode(';', $headerValue, 2)[0])), | ||
['application/x-www-form-urlencoded', 'multipart/form-data'] | ||
)) { | ||
$post = $_POST; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will $_POST contain if it isnt a POST request with content type application/x-www-form-urlencoded or multipart/form-data
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is just an empty array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. From my reading of PSR-7, getParsedBody
should return null
when no (parseable) request body was provided. If we pass along the empty array from $_POST
that would be the same as saying we successfully parsed the body content, which may not have been try at al. E.g. we make no attempt to parse JSON bodies.
That is why I wanted to establish a clear difference between []
and null
😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
Thank you |
Strictly follow the PSR-7 guidance where
$_POST
is used for a request’s parsed body only for very specific requests. All other requests should result in a null value and leave other parsing up to application logic.The current practice of putting an empty array in may imply we have done parsing and found an empty request body. See also Nyholm/psr7#100.