zend-diactoros
zend-diactoros copied to clipboard
`ServerRequestFactory::fromGlobals()` checks if args are truthy rather than set
In ServerRequestFactory::fromGlobals()
, most of the code checks if the arguments are truthy, rather than set. For example:
https://github.com/zendframework/zend-diactoros/blob/89d471cc09850c7d47839cf16ba7a1fed54d45e5/src/ServerRequestFactory.php#L57-L58
https://github.com/zendframework/zend-diactoros/blob/89d471cc09850c7d47839cf16ba7a1fed54d45e5/src/ServerRequestFactory.php#L72-L74
This becomes an issue during testing (or a long-running process that accepts multiple requests). For example:
// In a previous test
$_POST = ['foo' => 'bar'];
// In current test
$request = ServerRequestFactory::fromGlobals(null, null, []);
echo serialize($request->getParsedBody());
// Expected: a:0:{}
// Actual: a:1:{s:3:"foo";s:3:"bar";}
Improved versions would be:
$server = static::normalizeServer($server ?? $_SERVER);
$files = static::normalizeFiles($files ?? $_FILES);
$cookies ?? $_COOKIE,
$query ?? $_GET,
$body ?? $_POST,
Or for under PHP 7:
$server = static::normalizeServer(isset($server) ? $server : $_SERVER);
$files = static::normalizeFiles(isset($files) ? $files : $_FILES);
isset($cookies) ? $cookies : $_COOKIE,
isset($query) ? $query : $_GET,
isset($body) ? $body : $_POST,
(This code should fix the issues, but I didn't have time to write any tests right now, so I'm opening an issue instead of a pr.)
fromGlobals()
is meant to be used for requests parsed by php and injected into process as global variables. That rules out multiple requests in long running processes or requests assembled for testing.
Other than apparent misuse, this looks like a legit issue.
I would definitely consider a patch that uses the isset-ternary approach outlined above. That said, as noted by @Xerkus, this is a rare possibility in standard usage.
This repository has been closed and moved to laminas/laminas-diactoros; a new issue has been opened at https://github.com/laminas/laminas-diactoros/issues/12.