search

Nyholm / psr7

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

Fix error handling in Stream::getContents() #252

Closed nicolas-grekas closed 9 months ago

nicolas-grekas commented 9 months ago

stream_get_contents() swallows errors. This creates issues like https://github.com/symfony/symfony/issues/52490, where errors reported by the stream layer aren't propagated to user-land.

This is even in the C source: https://github.com/php/php-src/blob/311cae03e730c76aed343312319ed8cf1c37ade0/main/streams/streams.c#L1512

nicolas-grekas commented 9 months ago

guzzle/psr7 looks OK BTW: https://github.com/guzzle/psr7/blame/2.6/src/Utils.php#L404

nicolas-grekas commented 9 months ago

laminas/diactoros is affected: https://github.com/laminas/laminas-diactoros/blob/3.4.x/src/Stream.php#L282 /cc @weierophinney

stof commented 9 months ago

@nicolas-grekas is this a case for which we could write a test in php-http/psr7-integration-tests so that all PSR-7 implementations could know whether they handle that case properly by running the shared testsuite ? Or is it not testable this way ?

nicolas-grekas commented 9 months ago

slim/psr7 is affected: https://github.com/slimphp/Slim-Psr7/blob/4299f1650acad6df77fbaeb6f18c59e46af761b9/src/Stream.php#L357 /cc @l0gicgate

nicolas-grekas commented 9 months ago

ringcentral/psr7 affected too: https://github.com/ringcentral/psr7/blob/360faaec4b563958b673fb52bbe94e37f14bc686/src/Stream.php#L102 /cc @mtdowling

nicolas-grekas commented 9 months ago

workerman/psr7 affected: https://github.com/walkor/psr7/blob/e21521b72b5a891bd1a7a51376da12801863ce5b/src/Stream.php#L99 /cc @walkor

nicolas-grekas commented 9 months ago

httpsoft/http-message affected too: https://github.com/httpsoft/http-message/blob/6d86446b9d8f92cc06c60375d3e67cf3407a57a5/src/StreamTrait.php#L332 /cc @devanych

nicolas-grekas commented 9 months ago

I added a test case at https://github.com/php-http/psr7-integration-tests/pull/74 and updated the implementation to cover more situations (like guzzle does).

vbartusevicius commented 9 months ago

Hi @nicolas-grekas, thanks for fixing such a lurking bug! @Nyholm, if possible, when can we expect a release with the fix?

Nyholm commented 9 months ago

It was released in 1.8.1.

See https://github.com/Nyholm/psr7/releases/tag/1.8.1

shadowhand commented 9 months ago

It might also be helpful to have a test case for this in http-factory-tests.

devanych commented 9 months ago

Hi @nicolas-grekas, thanks :+1: