Skip to content

zlib: avoid an extra empty zstd frame - #66161

Open
marcopiraccini wants to merge 1 commit into
nodejs:mainfrom
marcopiraccini:zstd-empty-frame
Open

marcopiraccini wants to merge 1 commit into
nodejs:mainfrom
marcopiraccini:zstd-empty-frame

Conversation

@marcopiraccini

@marcopiraccini marcopiraccini commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

createZstdCompress() could append an empty frame when end() was called while writes were still queued. The last queued chunk could finish the frame, then the stream's final flush sent ZSTD_e_end again with no input.

Track when the frame has ended and skip empty calls until more input arrives. This avoids the extra frame while still allowing a later write to start a new one.

Added test/parallel/test-zlib-zstd-compress-single-frame.js to cover queued writes, explicit frame endings, an empty flush, an empty stream, and a later frame.

Fixes: #66078

Note

PR #66091 proposed the same C++ fix but was closed without merging. This PR expands the regression coverage to include an empty flush after a completed frame, an empty stream, and a deliberate second frame, alongside queued writes and an explicit end flush. The empty flush completes before end() so the test exercises ZSTD_e_flush separately.

Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies. labels Sep 20, 2026
@marcopiraccini
marcopiraccini marked this pull request as ready for review September 20, 2026 15:22
@codecov

codecov Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (25e6c15) to head (b0ce1f4).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66161      +/-   ##
==========================================
+ Coverage   90.27%   90.28%   +0.01%     
==========================================
  Files         790      790              
  Lines      271981   271988       +7     
  Branches    51913    51920       +7     
==========================================
+ Hits       245531   245568      +37     
+ Misses      16945    16912      -33     
- Partials     9505     9508       +3     
Files with missing lines Coverage Δ
src/node_zlib.cc 79.88% <100.00%> (-0.23%) ⬇️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zlib: createZstdCompress appends an empty frame when end() is called with writes still queued

2 participants