Fix stream filter seeking resetting write filter state on read seeks#21587
Fix stream filter seeking resetting write filter state on read seeks#21587nicolas-grekas wants to merge 1 commit intophp:masterfrom
Conversation
278d7f2 to
2d9907b
Compare
2d9907b to
2cf3b0e
Compare
|
I thought about this and I'm not sure this is the right solution. I wonder how this is going to work for read + write (STREAM_FILTER_ALL mode) because from code it looks like the same state is used. This might need some testing. But mainly I'm not sure this is always safe for all filters. For example it could keep in the state buffer just a partial write (e.g. just part of UTF-8 character) and when sought back, it will just include this buffer rubbish to the next write which might result to some unexpected result. I will need to check if any such filter is impacted and if so, we should introduce new seekable type. I also don't like the fact that that the write seek is completely skipped but that would be addressed by addressing the above anyway as I don't think we can skip it for all. I need to think and look into it more. |
|
I'm not saying this should not be addressed (the use case is valid), just need to think about proper solution. That test is helpful though. |
|
Thanks! The only alternative would be for me to reimplement dechunking in userland, which would make this a hard BC break. |
PR #19981 (bug #49874 fix) introduced stream filter seeking, which correctly resets read filter state on
fseek()/ftell(). However, it also resets write filter state on seek, which breaks real-world usage patterns where a stream with a write filter is seeked for reading.The case where this issue happened:
php://tempwith adechunkwrite filter, as used by Symfony HttpClient'sNativeResponseto decode chunked HTTP transfers. The buffer is written to (through the filter) and then seeked to position 0 to read back decoded output viastream_get_contents($buffer, -1, 0). After the PR, this seek resets the dechunk filter's state machine toCHUNK_SIZE_START, so the nextfwrite()of continuation chunk data is misparsed, causing "Transfer closed with outstanding data remaining from chunked response" errors.The attached patch does this:
fseek(). Since write filters are no longer reset on seek, there is no reason to prevent seeking based on write filter seekability.This restores the pre-#19981 behavior for write filters (flush-only on seek) while preserving the new read filter seeking behavior.
Any code that attaches a stateful write filter (dechunk, zlib.deflate, etc.) to a
php://tempor file stream and alternates between writing and reading viastream_get_contents()with an offset is affected by #19981 and fixed here.