Fixed GH-21376: gzfile and readgzfile no longer detect corrupted gzip…#21381
Conversation
…zip data When gzip data is corrupted, gzread() can return positive byte count while gzerror() indicates Z_DATA_ERROR. The zlib stream wrapper now checks gzerror() after each gzread() and returns -1 so that gzfile() returns an empty array and readgzfile() returns -1 as in previous PHP versions. readgzfile() also now uses ssize_t for php_stream_passthru() result so that -1 is correctly returned on error instead of being cast to a large size_t.
GabrielCordeiroBarrosoTeles
force-pushed
the
fix-21376-gzfile-corrupt-gzip
branch
from
22440ad to
aa61385
Compare
On Windows, zlib may return 0 bytes for corrupted gzip while setting Z_DATA_ERROR. Check gzerror() for read >= 0 so that case is treated as failure and readgzfile() returns -1 (phpGH-21376).
GabrielCordeiroBarrosoTeles
force-pushed
the
fix-21376-gzfile-corrupt-gzip
branch
from
aa61385 to
7e33691
Compare
| } | ||
| size = php_stream_passthru(stream); | ||
| /* On Windows, corrupt gzip may yield 0 from passthru; stream eof is already set so a further read returns 0 (not -1). Treat 0 bytes read from a non-empty file as error. */ | ||
| if (size == 0) { |
There was a problem hiding this comment.
hmmm I do not know that well gz extension however this block does not give me that much confidence (reliability wise). cc @ndossche wdyt ?
There was a problem hiding this comment.
Thanks for the review @devnexen . This block is a workaround for the Windows case where the wrapper returns 0 instead of -1 for corrupt gzip. I'm happy to implement a more direct fix in the wrapper/stream if you think that's a better approach.
There was a problem hiding this comment.
No worries I figured from your comments :) but just asking more knowledgable people which direction is best. Cheers
|
Something more complex is going on.
However, I do get an empty array on both versions, and both return value 0 on both versions. I also don't see relevant code changes between the two versions. So I wonder what actually changed. Is it possible that the zlib library also got upgraded when you upgraded PHP and somehow this library behaves differently now? |
3 participants
Fixes #21376
Summary
From PHP 8.5.3,
gzfile()andreadgzfile()no longer treat corrupted gzip data as an error. They may return decompressed data (garbage) instead of an empty array and-1respectively, as in previous versions.This change restores the previous behaviour when the zlib stream reports a data or stream error.
Changes
ext/zlib/zlib_fopen_wrapper.c
After each
gzread()call (including when it returns0), the wrapper now checksgzerror(). If the error isZ_DATA_ERRORorZ_STREAM_ERROR, the read is treated as failed (return-1), so callers receive error semantics instead of invalid data. Checking whenread >= 0covers Windows, where zlib may return 0 bytes while setting the error.ext/zlib/zlib.c
readgzfile()now stores the return value ofphp_stream_passthru()in anssize_tand returns it withRETURN_LONG((zend_long) size), so a stream error (-1) is returned to PHP as-1instead of being converted to a large positive value when stored in asize_t.When
passthru()returns0, a one-byte read is performed; if that read fails (returns< 0), the function returns-1. This fallback fixes corrupt gzip handling on Windows wheregzerror()may not be set whengzread()returns 0.ext/zlib/tests/bug21376_corrupt_gz.phpt
New test that uses a minimal invalid gzip file and asserts that
gzfile()returns an empty array andreadgzfile()returns-1.NEWS
Entry added for this fix under the Zlib section.
Testing
The new regression test was run locally (see screenshot below).
What the screenshot shows: Output of
run-tests.php ext/zlib/tests/bug21376_corrupt_gz.phptinside Docker (Ubuntu 24.04). One test is executed — the one that checks corrupted gzip handling — and it PASSes. The summary at the bottom reports: 1 test, 1 passed, 0 failed.