Fix #48725: Support for flushing in zlib stream#3320
Closed
aboks wants to merge 2 commits intophp:PHP-7.2from
Closed
Fix #48725: Support for flushing in zlib stream#3320aboks wants to merge 2 commits intophp:PHP-7.2from
aboks wants to merge 2 commits intophp:PHP-7.2from
Conversation
There are two tests: one specifically for the issue applied to the zlib stream filter, one for the generic issue of stream filters not being called with the CLOSING flag.
It seems that data streams turn EOF directly after reading the last few bytes, to that the stream filter is never called with the CLOSING flag. By ensuring that we make an extra pass through the stream filter if the last read was not empty, we can guarantee that the stream filter is always called with the CLOSING flag in the last pass.
Member
|
While working on a fix for bug #48725, I've found this PR. Thanks for it! However, bug you have found, and which this PR addresses is not the one bug 48725 is about, but is rather bug 79984. It seems to me, though, that bug 79984 and bug 77069 have the same root cause, but the latter issue would not be fixed with this PR, but rather with PR #6001. However, PR #6001 currently causes a SOAP test to fail, which this PR would not. I'm still looking for a solution; maybe you have an idea? |
Member
|
@cmb69 Can this one be closed after the changes you have done? |
Member
|
Ah, good catch, this can indeed be closed now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Actually this bug is not limited to zlib, but more about stream filters not being called with the
CLOSINGflag for data-, temp- and memory-streams. The more generic testcase aims to illustrate this.This issue was fixed in 5.5.21 by 86f1875, but resurfaced in 7.2.0 due to 5060fc2. Apparently the fix for another break in 0a45e8f was not enough to fix this issue.
This PR takes another approach by ensuring that a final pass through the stream filter with the
CLOSING-flag is made, even if the stream turns EOF directly after reading the last bytes.