Skip to content

zlibWrapper: don't start an empty gzip member after Z_FINISH - #4818

Open
ManoharPaturi wants to merge 1 commit into
meta:devfrom
ManoharPaturi:zlibwrapper-gz-finish-empty-member
Open

ManoharPaturi wants to merge 1 commit into
meta:devfrom
ManoharPaturi:zlibwrapper-gz-finish-empty-member

Conversation

@ManoharPaturi

Copy link
Copy Markdown

gz_comp() in zlibWrapper/gzwrite.c calls deflateReset() as soon as a Z_FINISH completes a member. The next flush, which is usually the one gzclose_w() issues, then runs deflate() on a freshly reset stream with no input, and that emits a complete empty gzip member at the end of the file. A file written as gzwrite, gzflush(Z_FINISH), gzclose ends up 20 bytes longer than the same file written without the intermediate flush, and every further gzflush(Z_FINISH) with no data in between appends another 20 byte empty member.

Measured on current dev, writing five bytes with the wrapper's own gz layer (gzip mode):

gzwrite + gzclose                     25 bytes
gzwrite + gzflush(Z_FINISH) + gzclose 45 bytes   (trailing empty member)
... + one more empty gzflush          65 bytes   (two empty members)

The same program linked against system zlib 1.3.2 produces 25 bytes in all three cases. zlib fixed this in 1.3 by keeping a reset-pending flag in the gz state and only resetting the deflate stream once new data arrives (upstream change introducing state->reset in gz_comp).

This ports that fix: gzguts.h gains the reset flag, gz_comp() defers the deflateReset until it has input to compress, and gz_reset() clears the flag when a stream is recycled. With the change, the wrapper produces byte for byte the same output as system zlib for the sequences above, and files that legitimately contain several members (write, finish, write, finish, close) are unaffected.

A new test, examples/gzfinishtest.c, covers this in both zlib and zstd compression modes through the make test target: it writes one buffer with zero, one, and two intervening Z_FINISH flushes and requires all three files to have the same size. On unfixed code it fails with plain=25 flushed=45 twice=65 in gzip mode and plain=14 flushed=23 twice=32 in zstd mode; with the fix all sizes match. The rest of the wrapper test suite passes.

Copilot AI balanced review requested due to automatic review settings October 4, 2026 04:22
@meta-cla meta-cla Bot added the CLA Signed label Oct 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

gz_comp() in zlibWrapper/gzwrite.c calls deflateReset() as soon as a
Z_FINISH completes a member. The next flush, which is usually the one
gzclose_w() issues, then runs deflate() on a freshly reset stream with
no input, and that emits a complete empty gzip member at the end of the
file. A file written as gzwrite, gzflush(Z_FINISH), gzclose ends up 20
bytes longer than the same file written without the intermediate flush,
and every further gzflush(Z_FINISH) with no data in between appends
another 20 byte empty member.

Measured on current dev, writing five bytes with the wrapper's own gz
layer (gzip mode):

    gzwrite + gzclose                     25 bytes
    gzwrite + gzflush(Z_FINISH) + gzclose 45 bytes   (trailing empty member)
    ... + one more empty gzflush          65 bytes   (two empty members)

The same program linked against system zlib 1.3.2 produces 25 bytes in
all three cases. zlib fixed this by keeping a reset-pending flag in the
gz state and only resetting the deflate stream once new data arrives.

Port that fix: gzguts.h gains the reset flag, gz_comp() defers the
deflateReset until it has input to compress, and gz_reset() clears the
flag when a stream is recycled.

Add a test, examples/gzfinishtest.c, run in both zlib and zstd
compression modes through the make test target: it writes one buffer
with zero, one, and two intervening Z_FINISH flushes and requires all
three files to have the same size. On unfixed code it fails with
plain=25 flushed=45 twice=65 in gzip mode and plain=14 flushed=23
twice=32 in zstd mode; with the fix all sizes match.
@ManoharPaturi
ManoharPaturi force-pushed the zlibwrapper-gz-finish-empty-member branch from 4453359 to 0b8b599 Compare October 4, 2026 05:36
@ManoharPaturi

Copy link
Copy Markdown
Author

@Cyan4973 these legacy decoder fixes (#4818 among the v0.2 through v0.7 batch) have been green on all checks for a few weeks now with no reviewer assigned yet. Each one is a narrow correctness fix in one legacy format with a round-trip regression test in tests/legacy.c. Could you or another maintainer take a look when the queue allows?

This branch has not been deployed

No deployments
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.

2 participants