Conversation
The loop used a fixed 8192-byte C buffer with php_stream_gets() and stored lines via add_index_string() (strlen-based). This caused two bugs: - NUL bytes truncated the stored string mid-line (data loss) - Lines longer than 8190 bytes were split into multiple entries Replace with php_stream_get_line(stream, NULL, 0, &len) + add_index_stringl(), matching how file() handles the same cases. The buffer is now allocated by the stream layer as needed, removing both the NUL and the size limit. Fixes: #925
Sjord
reviewed
Sep 28, 2026
Sjord
left a comment
Sjord
left a comment
Contributor
There was a problem hiding this comment.
Looks good, but the test could use some polishing.
| $tmp = tempnam(sys_get_temp_dir(), 'gzfile'); | ||
|
|
||
| // --- NUL byte preservation --- | ||
| $data_nul = "avant\0apres\nligne2\n"; |
Contributor
There was a problem hiding this comment.
It would make more sense to use English test strings.
| $lines = gzfile($tmp); | ||
| var_dump(count($lines)); // 2 | ||
| var_dump(strlen($lines[0])); // 12 (avant\0apres\n) | ||
| var_dump(bin2hex($lines[0])); // contains NUL |
Contributor
There was a problem hiding this comment.
addslashes could be useful to assert this while still being human-readable.
| var_dump(count($lines)); // 1 | ||
| var_dump(strlen($lines[0])); // 20001 | ||
|
|
||
| unlink($tmp); |
Contributor
There was a problem hiding this comment.
Should this go into a --CLEAN-- block?
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
2 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.
gzfile() reads each line into a fixed 8192-byte buffer with php_stream_gets() and stores it with add_index_string(), which recomputes the length with strlen(). This causes two bugs:
file() already avoids both issues by using php_stream_get_line() with a NULL/0 buffer (dynamically sized) and add_index_stringl() (explicit length). This applies the same approach to gzfile().
Reproducer:
A regression test is included covering NUL preservation (checked against gzgets() output) and a 20001-byte line staying as a single array entry.