Skip to content

Fix stream filter seeking resetting write filter state on read seeks#21587

Open
nicolas-grekas wants to merge 1 commit intophp:masterfrom
nicolas-grekas:fix-write-filter
Open

Fix stream filter seeking resetting write filter state on read seeks#21587
nicolas-grekas wants to merge 1 commit intophp:masterfrom
nicolas-grekas:fix-write-filter

Conversation

@nicolas-grekas
Copy link
Copy Markdown
Contributor

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://temp with a dechunk write filter, as used by Symfony HttpClient's NativeResponse to decode chunked HTTP transfers. The buffer is written to (through the filter) and then seeked to position 0 to read back decoded output via stream_get_contents($buffer, -1, 0). After the PR, this seek resets the dechunk filter's state machine to CHUNK_SIZE_START, so the next fwrite() of continuation chunk data is misparsed, causing "Transfer closed with outstanding data remaining from chunked response" errors.

The attached patch does this:

  • Do not seek/reset write filters on stream seek. Write filter state tracks the transformation of data being written through the filter. It is independent of the stream's read/write position. Seeking repositions where output is stored or read from, not where input comes from.
  • Remove the write filter seekability check that could block fseek(). Since write filters are no longer reset on seek, there is no reason to prevent seeking based on write filter seekability.
  • Read filter seeking is unchanged and works correctly.

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://temp or file stream and alternates between writing and reading via stream_get_contents() with an offset is affected by #19981 and fixed here.

@bukka
Copy link
Copy Markdown
Member

bukka commented Mar 31, 2026

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.

@bukka
Copy link
Copy Markdown
Member

bukka commented Mar 31, 2026

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.

@nicolas-grekas
Copy link
Copy Markdown
Contributor Author

Thanks! The only alternative would be for me to reimplement dechunking in userland, which would make this a hard BC break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants