-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Does not handle fgets returning an error gracefully #68
Comments
Thanks for reporting this. Is there a ticket in Drupal's issue tracker? I'm just trying to get some more scenario and context details. Anyway, I'll try to reproduce that behavior soonish. That's the position that triggers the If possible, please provide the version of |
Okay... independent from PharStreamWrapper I could reproduce I think it's fine to add an additional guard for this case. Still, finding out what really happens with |
Sorry! I linked this GitHub issue to the Drupal issue, but not here to there. Drupal issue: https://www.drupal.org/project/upgrade_status/issues/3232011 |
I had PHP 8.0, locally maybe that's why I couldn't reproduce. I'll see if I can work the user to get a reproduction and more data. |
Let's have a look, when that actually happens - thus, when PHP has problems to read from a stream, without issuing https://github.com/php/php-src/blob/PHP-7.4.20/ext/standard/file.c#L1122-L1127
https://github.com/php/php-src/blob/PHP-7.4.20/main/streams/streams.c#L991-L996
tl;dr: I have not able to reproduce this behavior with DDEV and PHP 7.4.20 on macOS. |
If
fgets
returnsfalse
due to an error, this package will crash as it passes abool
tostrpos
:I'm still not certain how we got an error from
fgets
. We were trying to launch the phpstan.phar:Relevant issue on Drupal.org. This gets triggered with:
That causes the PHPStan\PharAutoloader to execute:
And we end up broken. I don't know how many lines it takes to get an error, but it's before
feof
returns true.The text was updated successfully, but these errors were encountered: