Request::getParsedBody returns false but must null|array|object

[p-golovin]

I send POST request from jQuery via XMLHttpRequest. In case of empty request body there is no Content-Type header. In such case Request::getMediaType returns null and Request::getParsedBody returns false. This was broke after 3.2.0 update (https://github.com/slimphp/Slim/pull/1734)

[akrabat]

Can you explain what you're trying to do please?

[p-golovin]

I send POST request from jQuery. It may contain body or not, depending on user data:

$.ajax({url: '/' + CHAMP.vote.id + '/vote/send/' + voteId + '/', method: 'POST', data: requestData, dataType: 'json'});

In controller I want to validate data from this request and use getParsedBody function for this:

public function sendVote(Request $request, Response $response, array $args)
{
        $validator = [
            'g-recaptcha-response' => Validator::optional(Validator::regex("/^[\w-]*$/"))
        ];

        try {
            $params = Functions::validate($validator, $request->getParsedBody());
        } catch (NestedValidationException $e) {
            throw new NotFoundException($request, $response);
        }

I expect to receive array or null, but receive false in case of empty POST body and no Content-Type header.

[tuupola]

Tested with 3.2.2 and it returns false. PSR-7 says getParsedBody() says return value should be null|array|object. Seems behaviour is wrong at the moment.

$app->post("/test", function ($request, $response, $args) {
    var_dump($request->getParsedBody());
});

%curl --include --request POST http://127.0.0.1:8080/test
HTTP/1.1 200 OK
Host: 127.0.0.1:8080
Connection: close
X-Powered-By: PHP/5.6.17
Content-Type: text/html; charset=UTF-8
Content-Length: 103

<pre class='xdebug-var-dump' dir='ltr'><small>boolean</small> <font color='#75507b'>false</font>
</pre>

[geggleto]

What content-type would that request be @tuupola ?

I am guessing there's a logic error in here.

https://github.com/slimphp/Slim/blob/3.x/Slim/Http/Request.php#L972-L982

We should have a unit test for this as well.

edit We could add a guard around line 980,

if ($parsedBody === false) {
    $parsedBody = [];
}

[p-golovin]

May be add else for this (https://github.com/slimphp/Slim/blob/3.x/Slim/Http/Request.php#L972-L982):

else {
    return null;
}

[mathmarques]

I think we could return null, or set parsedBody to null.

The unit test that will fail:

public function testGetParsedBodyNoContentType()
    {
        $method = 'GET';
        $uri = new Uri('https', 'example.com', 443, '/foo/bar', 'abc=123', '', '');
        $headers = new Headers();
        $cookies = [];
        $serverParams = [];
        $body = new RequestBody();
        $request = new Request($method, $uri, $headers, $cookies, $serverParams, $body);

        $this->assertNull($request->getParsedBody());
    }

[geggleto]

@p-golovin The if can't have an else that returns null, then nothing would get parsed ;)

[geggleto]

:+1: @mathmarques

//cc @akrabat @silentworks @codeguy

I think either an empty array or null is fine in this situation. I would prefer the array but that is more for my preference than what it actually needs to be. Unfortunately the spec is vague in this area.

[p-golovin]

@geggleto

The if can't have an else that returns null, then nothing would get parsed ;)

Yes, in case of non-existent mediaType. But I agree, that it is bad solution.

[akrabat]

I would appreciate review and test of #1808 to see if this solves the problem.

[p-golovin]

Works fine now. Thanks.