Skip to content

zlib: strictly validate Zstd pledgedSrcSize - #64604

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Archkon:zlib3
Jul 23, 2026
Merged

zlib: strictly validate Zstd pledgedSrcSize#64604
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Archkon:zlib3

Conversation

@Archkon

@Archkon Archkon commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Fixes: #64603

@nodejs-github-bot

nodejs-github-bot commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@Archkon
Archkon marked this pull request as draft July 19, 2026 16:18
@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. stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem. labels Jul 19, 2026
@codecov

codecov Bot commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.13%. Comparing base (efc0a14) to head (801112a).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
src/node_zlib.cc 75.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #64604      +/-   ##
==========================================
+ Coverage   90.11%   90.13%   +0.02%     
==========================================
  Files         741      741              
  Lines      242196   242106      -90     
  Branches    45606    45587      -19     
==========================================
- Hits       218246   218219      -27     
+ Misses      15438    15383      -55     
+ Partials     8512     8504       -8
Files with missing lines Coverage Δ
lib/internal/streams/iter/transform.js 95.73% <ø> (-0.06%) ⬇️
lib/zlib.js 98.07% <100.00%> (ø)
src/node_zlib.cc 78.84% <75.00%> (+0.49%) ⬆️

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

@Archkon
Archkon marked this pull request as ready for review July 20, 2026 04:50
@Archkon Archkon mentioned this pull request Jul 20, 2026
Merged
@Archkon

Archkon commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@hybrist @jasnell Could you take look at this? Thanks!

hybrist
hybrist reviewed Jul 22, 2026
View reviewed changes
Comment thread lib/zlib.js Outdated
Comment thread src/node_zlib.cc
@Archkon
zlib: validate pledgedSrcSize as a safe integer
801112a 
pledgedSrcSize represents an exact byte count. Reject values that are
not non-negative safe integers instead of silently ignoring or coercing
them through IntegerValue().

Apply the same validation to zlib/iter and retain a native validation
check for internal callers.

Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
@Archkon
Archkon requested a review from hybrist July 22, 2026 17:29
Ethan-Arrowood
Ethan-Arrowood approved these changes Jul 23, 2026
View reviewed changes

@Ethan-Arrowood Ethan-Arrowood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - since this is an experimental API I think its safe to be a patch.

@Ethan-Arrowood Ethan-Arrowood added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 23, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 23, 2026
@nodejs-github-bot

nodejs-github-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/75035/

@Archkon

Archkon commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@Ethan-Arrowood I can't access jenkins ci test so could you review what is wrong with these failed tests when available.Should I resolve these problem or just flaky test?

@hybrist

hybrist commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

This looks like an infra issue?

01:00:12 hudson.plugins.git.GitException: Command "git reset --hard" returned status code 128:
01:00:12 stdout: 
01:00:12 stderr: fatal: Unable to create '/home/iojs/build/workspace/node/.git/index.lock': File exists.
01:00:12 
01:00:12 Another git process seems to be running in this repository, e.g.
01:00:12 an editor opened by 'git commit'. Please make sure all processes
01:00:12 are terminated then try again. If it still fails, a git process
01:00:12 may have crashed in this repository earlier:
01:00:12 remove the file manually to continue.

@nodejs-github-bot

nodejs-github-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/75062/

@hybrist hybrist added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 23, 2026
@Ethan-Arrowood Ethan-Arrowood added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 23, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 23, 2026
@nodejs-github-bot
nodejs-github-bot merged commit de6eeec into nodejs:main Jul 23, 2026
75 checks passed
@nodejs-github-bot

nodejs-github-bot commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Landed in de6eeec

@Archkon
Archkon deleted the zlib3 branch July 24, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hybrist hybrist hybrist approved these changes
@Ethan-Arrowood Ethan-Arrowood Ethan-Arrowood approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

zlib: strictly validate Zstd pledgedSrcSize

4 participants

@Archkon @nodejs-github-bot @hybrist @Ethan-Arrowood