parse body if empty and has media type detected #100
Conversation
| @@ -162,13 +162,13 @@ public function getParsedBody() | |||
| { | |||
| $parsedBody = $this->serverRequest->getParsedBody(); | |||
|
|
|||
| if ($parsedBody !== null) { | |||
| if (!empty($parsedBody)) { | |||
| return $parsedBody; | |||
There was a problem hiding this comment.
Honestly, I don't think this is the correct solution. nyholm/psr7 should be updated to not automatically set the parsed body, per PSR-7 spec:
* If the request Content-Type is either application/x-www-form-urlencoded
* or multipart/form-data, and the request method is POST, this method MUST
* return the contents of $_POST.
*
* Otherwise, this method may return any results of deserializing
* the request body content; as parsing returns structured content, the
* potential types MUST be arrays or objects only. A null value indicates
* the absence of body content.
There was a problem hiding this comment.
As I understand it, nyholm/psr7 is correct implementation. In the case of a json $_POST they should not send null they should send empty array. See the discussion in #99.
There was a problem hiding this comment.
We could do a null check based on content type then. If it is application/x-www-form-urlencoded or multipart/form-data we do a null check and return the implementation’s parsed body if not null otherwise parse the body.
There was a problem hiding this comment.
@l0gicgate Slim doesn't need to solve this problem, IMO. It is a bug in nyholm/psr7, which is clearly not following the spec by blindly setting withParsedBody($post) without checking the request Content-Type.
There was a problem hiding this comment.
It’s also a problem with Zend-Diactoros.
There was a problem hiding this comment.
Alright, if the problem exists beyond one implementation then I agree this is a reasonable solution.
There was a problem hiding this comment.
It seems that the spec wasn't really considering the possibility that multiple layers would attempt to parse the body. There's no way to pass information on whether body parsing was attempted already.
| @@ -162,13 +162,13 @@ public function getParsedBody() | |||
| { | |||
| $parsedBody = $this->serverRequest->getParsedBody(); | |||
|
|
|||
| if ($parsedBody !== null) { | |||
| if (!empty($parsedBody)) { | |||
| return $parsedBody; | |||
There was a problem hiding this comment.
Alright, if the problem exists beyond one implementation then I agree this is a reasonable solution.
|
As an aside, it's nice that Slim-Psr7's |
|
Is there anything else I should do with this PR to get it merged? |
Fixes #99