Skip to content

Fix #75776: Flushing streams with compression filter is broken #6703

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

Closed
wants to merge 1 commit into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Feb 16, 2021

First, the bzip2.compress filter has the same issue as zlib.deflate
so we port the respective fix[1] to ext/bz2.

Second, there is still an issue, if a stream with an attached
compression filter is flushed before it is closed, without any writes
in between. In that case, the compression is never finalized. We fix
this by enforcing a _php_stream_flush() with the closing flag set
in _php_stream_free(), whenever a write filter is attached. This
call is superfluous for most write filters, but does not hurt, even
when it is unnecessary.

[1] http://git.php.net/?p=php-src.git;a=commit;h=20e75329f2adb11dd231852c061926d0e4080929

First, the `bzip2.compress` filter has the same issue as `zlib.deflate`
so we port the respective fix[1] to ext/bz2.

Second, there is still an issue, if a stream with an attached
compression filter is flushed before it is closed, without any writes
in between.  In that case, the compression is never finalized.  We fix
this by enforcing a `_php_stream_flush()` with the `closing` flag set
in `_php_stream_free()`, whenever a write filter is attached.  This
call is superfluous for most write filters, but does not hurt, even
when it is unnecessary.

[1] <http://git.php.net/?p=php-src.git;a=commit;h=20e75329f2adb11dd231852c061926d0e4080929>
@cmb69 cmb69 added the Bug label Feb 16, 2021
@php-pulls php-pulls closed this in 963e50c Feb 22, 2021
@cmb69 cmb69 deleted the cmb/75776 branch February 22, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants