From 0b8b5995bc844f6d20e8ab0963b7440085ae7e96 Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Sun, 4 Oct 2026 11:06:53 +0530 Subject: [PATCH] zlibWrapper: don't start an empty gzip member after Z_FINISH 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. --- zlibWrapper/Makefile | 12 +++- zlibWrapper/examples/gzfinishtest.c | 91 +++++++++++++++++++++++++++++ zlibWrapper/gzguts.h | 1 + zlibWrapper/gzlib.c | 1 + zlibWrapper/gzwrite.c | 11 +++- 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 zlibWrapper/examples/gzfinishtest.c diff --git a/zlibWrapper/Makefile b/zlibWrapper/Makefile index 830b294bb6f..ca3d8e1abfd 100644 --- a/zlibWrapper/Makefile +++ b/zlibWrapper/Makefile @@ -46,9 +46,11 @@ release : STDFLAGS = release : DEBUGFLAGS = release : all -all: fitblk example zwrapbench minigzip +all: fitblk example zwrapbench minigzip gzfinishtest -test: example fitblk example_zstd fitblk_zstd zwrapbench minigzip minigzip_zstd +test: example fitblk example_zstd fitblk_zstd zwrapbench minigzip minigzip_zstd gzfinishtest gzfinishtest_zstd + ./gzfinishtest + ./gzfinishtest_zstd ./example ./example_zstd ./fitblk 10240 <$(TEST_FILE) @@ -83,6 +85,12 @@ test-valgrind: clean example fitblk example_zstd fitblk_zstd zwrapbench #.c.o: # $(CC) $(CFLAGS) $(CPPFLAGS) -c $< -o $@ +gzfinishtest: gzfinishtest.o zstd_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) + $(LINK.o) $^ $(LDLIBS) $(OUTPUT_OPTION) + +gzfinishtest_zstd: gzfinishtest.o zstdTurnedOn_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) + $(LINK.o) $^ $(LDLIBS) $(OUTPUT_OPTION) + minigzip: minigzip.o zstd_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) minigzip_zstd: minigzip.o zstdTurnedOn_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) diff --git a/zlibWrapper/examples/gzfinishtest.c b/zlibWrapper/examples/gzfinishtest.c new file mode 100644 index 00000000000..8209e1dc0ff --- /dev/null +++ b/zlibWrapper/examples/gzfinishtest.c @@ -0,0 +1,91 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under both the BSD-style license (found in the + * LICENSE file in the root directory of this source tree) and the GPLv2 (found + * in the COPYING file in the root directory of this source tree). + * You may select, at your option, one of the above-listed licenses. + */ + +/* gzfinishtest : check that gzflush(Z_FINISH) does not start an empty member. + * + * Writes one small buffer, finishes the member with gzflush(), flushes again + * with no data in between, then closes the file. The result must contain + * exactly one gzip member : the file size must equal the size obtained when + * writing the same data without the intermediate flushes. + */ + +#include +#include +#include +#define ZLIB_CONST +#define Z_PREFIX +#include "zlib.h" + +static int writeMode(const char* path, int mode) +{ + gzFile file = gzopen(path, "wb"); + if (file == NULL) { + fprintf(stderr, "gzfinishtest: gzopen failed\n"); + return 1; + } + if (gzwrite(file, "hello", 5) != 5) { + fprintf(stderr, "gzfinishtest: gzwrite failed\n"); + return 1; + } + if (mode >= 1) { + if (gzflush(file, Z_FINISH) != Z_OK) { + fprintf(stderr, "gzfinishtest: gzflush failed\n"); + return 1; + } + } + if (mode >= 2) { + /* second finish with no data in between : must not emit a member */ + if (gzflush(file, Z_FINISH) != Z_OK) { + fprintf(stderr, "gzfinishtest: second gzflush failed\n"); + return 1; + } + } + if (gzclose(file) != Z_OK) { + fprintf(stderr, "gzfinishtest: gzclose failed\n"); + return 1; + } + return 0; +} + +static long fileSize(const char* path) +{ + FILE* const f = fopen(path, "rb"); + long size; + if (f == NULL) return -1; + if (fseek(f, 0, SEEK_END) != 0) { fclose(f); return -1; } + size = ftell(f); + fclose(f); + return size; +} + +int main(int argc, char** argv) +{ + const char* name = (argc > 1) ? argv[1] : "gzfinishtest.tmp"; + long plain, flushed, twice; + + if (writeMode(name, 0)) return 1; + plain = fileSize(name); + if (writeMode(name, 1)) return 1; + flushed = fileSize(name); + if (writeMode(name, 2)) return 1; + twice = fileSize(name); + + if (plain < 0 || flushed != plain || twice != plain) { + fprintf(stderr, "gzfinishtest: file sizes differ : plain=%ld flushed=%ld twice=%ld " + "(spurious empty gzip member after Z_FINISH)\n", + plain, flushed, twice); + remove(name); + return 1; + } + + printf("gzfinishtest OK : %ld bytes in all cases\n", plain); + remove(name); + return 0; +} diff --git a/zlibWrapper/gzguts.h b/zlibWrapper/gzguts.h index 70a609d724c..cd64d01f2d7 100644 --- a/zlibWrapper/gzguts.h +++ b/zlibWrapper/gzguts.h @@ -197,6 +197,7 @@ typedef struct { /* just for writing */ int level; /* compression level */ int strategy; /* compression strategy */ + int reset; /* true if a reset is pending after a Z_FINISH */ /* seek request */ z_off64_t skip; /* amount to skip (already rewound if backwards) */ int seek; /* true if seek request pending */ diff --git a/zlibWrapper/gzlib.c b/zlibWrapper/gzlib.c index c726515113c..bbe7385b552 100644 --- a/zlibWrapper/gzlib.c +++ b/zlibWrapper/gzlib.c @@ -81,6 +81,7 @@ local void gz_reset(gz_statep state) { state.state->how = LOOK; /* look for gzip header */ } state.state->seek = 0; /* no seek request pending */ + state.state->reset = 0; /* no deflateReset pending */ gz_error(state, Z_OK, NULL); /* clear error */ state.state->x.pos = 0; /* no uncompressed data yet */ state.state->strm.avail_in = 0; /* no input data yet */ diff --git a/zlibWrapper/gzwrite.c b/zlibWrapper/gzwrite.c index 85b776a94f7..a007e469eb0 100644 --- a/zlibWrapper/gzwrite.c +++ b/zlibWrapper/gzwrite.c @@ -98,6 +98,15 @@ local int gz_comp(gz_statep state, int flush) { return 0; } + /* check for a pending reset */ + if (state.state->reset) { + /* don't start a new gzip member unless there is data to write */ + if (strm->avail_in == 0) + return 0; + deflateReset(strm); + state.state->reset = 0; + } + /* run deflate() on provided input until it produces no more output */ ret = Z_OK; do { @@ -135,7 +144,7 @@ local int gz_comp(gz_statep state, int flush) { /* if that completed a deflate stream, allow another to start */ if (flush == Z_FINISH) - deflateReset(strm); + state.state->reset = 1; /* all done, no errors */ return 0;