ServerRequestFactory::fromGlobals() checks if args are truthy rather than set

[weierophinney]

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

---

Originally posted by @0b10011 at zendframework/zend-diactoros#281

[weierophinney]

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.

---

Originally posted by @Xerkus at zendframework/zend-diactoros#281 (comment)

[weierophinney]

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.

---

Originally posted by @weierophinney at zendframework/zend-diactoros#281 (comment)

[weierophinney]

Fixed with #155