Skip to content
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

Remove stream filters in Stream destructor only if they are still valid resources #544

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

crocodele
Copy link
Contributor

When a Reader or Writer has a stream filter and the handle to the stream is closed manually in code, there's a fatal error in the Stream destructor.

To reproduce: in current master branch, the test case in the PR fails with a fatal error in the Stream destructor:

PHP Fatal error:  Uncaught TypeError: stream_filter_remove(): supplied resource is not a valid stream filter resource in /path/to/example/vendor/league/csv/src/Stream.php:84
Stack trace:
#0 /path/to/example/vendor/league/csv/src/Stream.php(84): stream_filter_remove()
#1 [internal function]: League\Csv\Stream->League\Csv\{closure}()
#2 /path/to/example/vendor/league/csv/src/Stream.php(84): array_walk_recursive()
#3 [internal function]: League\Csv\Stream->__destruct()
#4 {main}
  thrown in /path/to/example/vendor/league/csv/src/Stream.php on line 84

@crocodele crocodele changed the title Remove stream filters only if they are still valid Remove stream filters in Stream destructor only if they are still valid resources Dec 17, 2024
@nyamsprod nyamsprod merged commit b6dfea1 into thephpleague:master Dec 18, 2024
9 checks passed
@nyamsprod
Copy link
Member

Thanks for finding and fixing the bug. Nice catch. I will merge the PR but I update or improve if needed your fix.

@crocodele crocodele deleted the fix/StreamFilterDestruct branch December 18, 2024 08:48
@nyamsprod
Copy link
Member

version 9.20.1 is released with you fix included. Thanks again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants