php-http / discovery

Finds installed clients and message factories
http://php-http.org
MIT License
1.27k stars 49 forks source link

Psr17Factory::buildServerRequestFromGlobals() throws TypeError when encountering nested array of file uploads #226

Closed mfb closed 1 year ago

mfb commented 1 year ago

PHP version: 8.2.5

Description

PHP allows file uploads to be nested in an arbitrarily deep array structure (see example markup below). For requests using this feature, Psr17Factory::buildServerRequestFromGlobals() throws TypeError: Http\Discovery\Psr17Factory::createStreamFromFile(): Argument ($filename) must be of type string, array given, called in /vendor/php-http/discovery/src/Psr17Factory.php on line 271 in Http\Discovery\Psr17Factory->createStreamFromFile() (line 120 of /vendor/php-http/discovery/src/Psr17Factory.php)

How to reproduce Uploading a file to this script should throw the TypeError:

<pre>
<form method="post" enctype="multipart/form-data">
  <input multiple="multiple" type="file" name="files[upload_0][]">
  <input type="submit" name="submit">
</form>

<?php
use Http\Discovery\Psr17Factory;
require './vendor/autoload.php';
print_r($_FILES);
(new Psr17Factory())->createServerRequestFromGlobals();

Possible Solution

Additional context Discovered in https://github.com/getsentry/sentry-php/issues/1517

dbu commented 1 year ago

from the stack trace, the problem is at the bottom of the factory: https://github.com/php-http/discovery/blob/1ce09e799044cb5255d735991217408611c67ec4/src/Psr17Factory.php#L269 and is discovered thanks to strict typing. we should add a recursion there to dig into the array until we get to a string.

nicolas-grekas commented 1 year ago

Can you please provide a test case and a fix maybe in a PR?

vjik commented 1 year ago

Same problem.

Http\Discovery\Psr17Factory::createStreamFromFile(): Argument #1 ($filename) must be of type string, array given, called in /app/vendor/php-http/discovery/src/Psr17Factory.php on line 271

mfb commented 1 year ago

Can you please provide a test case and a fix maybe in a PR?

As far as I can tell, we could simply copy the working logic from guzzlehttp/psr7's ServerRequest class, as there is already a comment mentioning that package and its copyright information.

nicolas-grekas commented 1 year ago

Fixed in v1.16.0, thanks for the PR