search

slimphp / Slim-Psr7

PSR-7 implementation for use with Slim 4
MIT License
131 stars 45 forks source link

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

Closed odan closed 4 months ago

odan commented 10 months ago

The current implementation of stream_get_contents() swallows errors, leading to 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

Proposed Solution

To address the issue, @nicolas-grekas proposes replacing the usage of stream_get_contents() with fread(). This change will ensure that errors are not concealed, and they will be thrown as expected.

Using fread(), should fix this issue.

See here: https://github.com/Nyholm/psr7/pull/252

Thanks to @nicolas-grekas

nicolas-grekas commented 10 months ago

Thanks for the follow up. Please don't merge now as I might change the patch a bit depending on my tests.

nicolas-grekas commented 10 months ago

Upstream PR updated, see https://github.com/Nyholm/psr7/pull/252#issuecomment-1806036059

odan commented 10 months ago

@nicolas-grekas Ok thanks, we are waiting.

nicolas-grekas commented 10 months ago

Merged upstream, you can now sync the patch.

l0gicgate commented 9 months ago

@odan it appears CI is failing on 8.2. The changes look good to me otherwise