Skip to content

zlib: reject reset after gzip emitted incomplete output - #66179

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
xia-chao:zlib-reject-reset-incomplete-output
Sep 26, 2026
Merged

nodejs-github-bot merged 2 commits into
nodejs:mainfrom
xia-chao:zlib-reject-reset-incomplete-output

Conversation

@xia-chao

@xia-chao xia-chao commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

gzip writes a header on the first write. Calling reset after that starts a new member while that header is already in the output, so gunzip fails.

Same idea as the zstd incomplete-frame case in #66088, but only for gzip. zlib deflate can still reset after a flush: the dictionary test throws the first output away and reuses the compressor. Raw deflate is unchanged.

reset before any write still works. flush then end, without reset, still works.

I ran the new gzip reset test, the dictionary test, the existing reset tests, test-zlib.js, and the flush tests on a Release build.

deflateReset starts a new member. Bytes already written out cannot be
taken back, so gunzip/inflate see a truncated member followed by a new
header. Refuse reset in that case. Raw deflate has no wrapper header
and is unchanged.

Signed-off-by: Xia Chao <shapirolutts@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 21, 2026
zlib-wrapped deflate still allows reset after flush. The dictionary
test discards the first member and reuses the compressor.

Signed-off-by: Xia Chao <shapirolutts@gmail.com>
@xia-chao xia-chao changed the title zlib: reject reset after gzip/deflate emitted incomplete output zlib: reject reset after gzip emitted incomplete output Sep 21, 2026
@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.29%. Comparing base (513981d) to head (96593b0).
⚠️ Report is 100 commits behind head on main.

Files with missing lines Patch % Lines
src/node_zlib.cc 88.88% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66179   +/-   ##
=======================================
  Coverage   90.28%   90.29%           
=======================================
  Files         790      790           
  Lines      272043   272062   +19     
  Branches    51945    51945           
=======================================
+ Hits       245619   245650   +31     
+ Misses      16931    16916   -15     
- Partials     9493     9496    +3     
Files with missing lines Coverage Δ
src/node_zlib.cc 80.00% <88.88%> (+0.20%) ⬆️

... and 27 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.

@xia-chao

xia-chao commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Narrowed this to gzip only in a follow-up commit — could you take another look?

@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 22, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 22, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Sep 25, 2026
@nodejs-github-bot nodejs-github-bot added the lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. label Sep 25, 2026
@nodejs-github-bot
nodejs-github-bot merged commit ca0810f into nodejs:main Sep 26, 2026
93 of 94 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in ca0810f

@nodejs-github-bot nodejs-github-bot removed commit-queue PRs queued for automated landing through the Commit Queue. lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. labels Sep 26, 2026
aduh95 pushed a commit that referenced this pull request Sep 27, 2026
deflateReset starts a new member. Bytes already written out cannot be
taken back, so gunzip/inflate see a truncated member followed by a new
header. Refuse reset in that case. Raw deflate has no wrapper header
and is unchanged.

Signed-off-by: Xia Chao <shapirolutts@gmail.com>
PR-URL: #66179
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
HoonDongKang pushed a commit to HoonDongKang/node that referenced this pull request Sep 28, 2026
deflateReset starts a new member. Bytes already written out cannot be
taken back, so gunzip/inflate see a truncated member followed by a new
header. Refuse reset in that case. Raw deflate has no wrapper header
and is unchanged.

Signed-off-by: Xia Chao <shapirolutts@gmail.com>
PR-URL: nodejs#66179
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request Sep 28, 2026
deflateReset starts a new member. Bytes already written out cannot be
taken back, so gunzip/inflate see a truncated member followed by a new
header. Refuse reset in that case. Raw deflate has no wrapper header
and is unchanged.

Signed-off-by: Xia Chao <shapirolutts@gmail.com>
PR-URL: #66179
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. 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.

4 participants