POST data missing when uploading a file

[akrabat]

Consider this:

$app->get('/fileupload', function ($request, $response) {
    $html = <<<EOT
    <form method="POST" enctype="multipart/form-data">

        Title: <input type="text" name="title">
        <br>
        File: <input type="file" name="this_file">
        <br>
        <input type="submit" value="Upload">
    </form>
EOT;

    return $response->write($html);
});
$app->post('/fileupload', function ($request, $response) {
    var_dump($request->getParsedBody());
    exit;
});

We know that the file upload won't be picked up until #1263 is merged.

However, the rest of the form fields in the POST data are also missing!

Reading http://php.net/manual/en/wrappers.php.php#wrappers.php.input, I found this nugget:

php://input is not available with enctype="multipart/form-data"

So... getParsedBody() has to use $_POST in this scenario. However, we don't have access to $_POST as it's not passed in via the constructor.

From the PSR-7 interface docblock for getParsedBody() there's this too:

• 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.

We currently use the php://input stream rather than $_POST so that we can auto-decode JSON and XML bodies which is quite a cool feature.

How should we fix this?

The two options I can see are:

1. Update the factory method in Container to pass in $_POST as the $body parameter when the content-type is application/x-www-form-urlencoded or multipart/form-data.
2. Add another parameter to the Request's constructor so that we have the php://input stream and $_POST available.

I don't like 1 at all as it's moving something that's intrinsic to the Request to outside of it and now the created of the Request object has to be aware of the content-type which just seems odd.

to implement 2, we'd have to change the Request's signature from:

public function __construct($method, UriInterface $uri, HeadersInterface $headers,
    array $cookies, array $serverParams, StreamInterface $body)

to say:

public function __construct($method, UriInterface $uri, HeadersInterface $headers, 
    array $cookies, array $serverParams, StreamInterface $body, array $postData)

We can then update the mediaType parsers to do use $postData or $body as required.

Does anyone have a better idea?

[akrabat]

Playing with code, these are two options:

1. Implement Request::createFromEnvironment() See: https://github.com/slimphp/Slim/compare/slimphp:develop...akrabat:request-create-from-environment?expand=1

   This simplifies the creation of the Request object in the Container, which may be a good thing in and of itself.

2. Update Request's constructor to accept $_POST data, See https://github.com/slimphp/Slim/compare/develop...akrabat:post-data?expand=1

[opengeek]

:+1: for option 1 — Request::createFromEnvironment()

[akrabat]

I tend to agree, so have raised PR: #1278

[zither]

for option 1, POST values are also part of HTTP body.

[geggleto]

:+1: for option 1.

[gabriel403]

@akrabat is this fixed now?

[akrabat]

Yes.