Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.16k stars 75 forks source link

Inconsistent handling of stringable bodies #171

Closed AndreKR closed 1 year ago

AndreKR commented 3 years ago

Here are three ways to create a Nyholm\Psr7\Stream that I would expect to yield the same result:

This one works (because there's a type hint that casts to string):

$psr17Factory = new \Nyholm\Psr7\Factory\Psr17Factory();
$psr17Factory->createStream(3);

This one gives Uncaught InvalidArgumentException: First argument to Stream::create() must be a string, resource or StreamInterface.:

Stream::create(3);

This one gives Uncaught TypeError: fwrite() expects parameter 2 to be string, int given:

$stream = Stream::create('');
$stream->write(3);

Not a big deal, but I thought you might want to know. I encountered it when I implemented an API endpoint that was trying to return just the number of search results (3).

Zegnat commented 3 years ago

It only works if you do not declare strict types and allow PHP to do the silent casting.

If you use declare(strict_types=1); like we do for every class implemented in this library (e.g. on the Psr17Factory) then your first code example will not work either:

Fatal error: Uncaught TypeError: Nyholm\Psr7\Factory\Psr17Factory::createStream(): Argument #1 ($content) must be of type string, int given, […]

I am actually not sure if PHP-FIG has a stance on this. Do you know, @Nyholm?

That said the last error looks like an actual bug. According to PSR-7 Psr\Http\Message\StreamInterface::write can only throw a \RuntimeException. I would have to look at other implementations to see what the expected result for passing in a non-string is.

Nyholm commented 3 years ago

Thank you @AndreKR.

@Zegnat Yes, you are correct.

I am actually not sure if PHP-FIG has a stance on this. Do you know, @Nyholm?

No, I dont, sorry.

That said the last error looks like an actual bug

Yes, I think so too. We should add a cast to string there.

Nyholm commented 3 years ago

Btw, I know that PSR-7 will soon be updated. (Just like PSR-6). They will add type hints so this error will be solved.

Btw, Static analysers like psalm och phpstan will warn if you write Stream::create(3); or $stream->write(3);

nicolas-grekas commented 1 year ago

This is not something that we can fix at our level. This should be fixed by the PSR first.