lib: coerce -0 to +0 in various APIs by panva · Pull Request #63556 · nodejs/node

panva · 2026-05-25T08:46:33Z

Followup to #63531

This is not an exhaustive list of these occurences, just the ones I can remotely imagine being exposed to an untrusted user input through a web-based or CLI interface.

The tests demonstrate the process crashing before the ./lib changes.

I used Codex to find and confirm these before diving in.

nodejs-github-bot · 2026-05-25T08:46:38Z

Review requested:

@nodejs/crypto
@nodejs/net

aduh95 · 2026-05-25T09:03:27Z

  validateUint32(interval, 'interval');
+  // Coerce -0 to +0.
+  interval += 0;


Shouldn't we update validateUint32 to reject -0 instead?

I have no particular sympathy for either or.

Having it part of the helper is probably more robust than relying on humans to remember for checking for -0

Having it part of the helper is probably more robust than relying on humans to remember for checking for -0

Maybe even have it part of all uint conversion helpers?

we can certainly start blanket rejecting -0 from these validators in a semver-major followup if you feel like picking it up. #63531 already went the same route of this PR and is in the commit-queue

@panva check out this comment: #63531 (comment)

codecov · 2026-05-25T10:14:40Z

Codecov Report

❌ Patch coverage is 97.36842% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (cebe424) to head (d3df485).
⚠️ Report is 47 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/dns/promises.js	50.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #63556   +/-   ##
=======================================
  Coverage   90.32%   90.33%           
=======================================
  Files         730      730           
  Lines      234653   234723   +70     
  Branches    43940    43960   +20     
=======================================
+ Hits       211953   212027   +74     
  Misses      14414    14414           
+ Partials     8286     8282    -4

Files with missing lines	Coverage Δ
lib/dns.js	`98.65% <100.00%> (+0.01%)`	⬆️
lib/internal/blocklist.js	`91.39% <100.00%> (+0.05%)`	⬆️
lib/internal/crypto/cipher.js	`97.98% <100.00%> (+0.03%)`	⬆️
lib/internal/crypto/diffiehellman.js	`97.77% <100.00%> (+0.01%)`	⬆️
lib/internal/crypto/hash.js	`99.02% <100.00%> (+0.01%)`	⬆️
lib/internal/crypto/hkdf.js	`100.00% <100.00%> (+1.15%)`	⬆️
lib/internal/crypto/keygen.js	`93.49% <100.00%> (+0.47%)`	⬆️
lib/internal/crypto/random.js	`96.09% <100.00%> (+0.02%)`	⬆️
lib/internal/dns/utils.js	`99.44% <100.00%> (+<0.01%)`	⬆️
lib/internal/fs/utils.js	`98.44% <100.00%> (+<0.01%)`	⬆️
... and 4 more

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

ljharb

i think fixing the C++ side also is a good idea, but can/should be done separately

nodejs-github-bot · 2026-05-27T06:57:30Z

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

Signed-off-by: Filip Skokan <[email protected]>

nodejs-github-bot · 2026-05-27T08:02:13Z

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

nodejs-github-bot · 2026-05-27T15:18:25Z

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

atlowChemi · 2026-05-31T13:31:51Z

+  try {
+    fn();
+  } catch {
+    // Ignore expected file system errors from the missing path.


Shouldn't we assert we actually got here? (ie. common must call etc)

nodejs-github-bot · 2026-05-31T14:25:33Z

Landed in 040c51c...9e58d9d

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Backport-PR-URL: #64629 Reviewed-By: Antoine du Hamel <[email protected]>

panva requested a review from ljharb May 25, 2026 08:46

panva added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label May 25, 2026

panva changed the title ~~lib: various coerce -0 to +0 in various APIs~~ lib: coerce -0 to +0 in various APIs May 25, 2026

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 25, 2026

aduh95 reviewed May 25, 2026

View reviewed changes

panva mentioned this pull request May 26, 2026

crypto: coerce -0 keylen to +0 in pbkdf2 and scrypt #63531

Closed

ljharb approved these changes May 26, 2026

View reviewed changes

panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 26, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 26, 2026

This comment was marked as outdated.

Sign in to view

panva added 5 commits May 27, 2026 10:00

crypto: coerce -0 to +0 before native calls

ac93201

Signed-off-by: Filip Skokan <[email protected]>

dns: coerce -0 to +0 in lookup and resolver inputs

30a9263

Signed-off-by: Filip Skokan <[email protected]>

fs: coerce -0 to +0 in mode flags and watch intervals

80b1401

Signed-off-by: Filip Skokan <[email protected]>

net: coerce -0 to +0 in BlockList prefixes

844056b

Signed-off-by: Filip Skokan <[email protected]>

zlib: coerce -0 to +0 for crc32 seeds

ac9d613

Signed-off-by: Filip Skokan <[email protected]>

panva force-pushed the negative-zero-fixes branch from bb206e1 to ae9e27c Compare May 27, 2026 08:01

panva removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 27, 2026

panva force-pushed the negative-zero-fixes branch from 1b1223e to df6c598 Compare May 27, 2026 13:19

This comment was marked as outdated.

Sign in to view

fixup! crypto: coerce -0 to +0 before native calls

d3df485

panva force-pushed the negative-zero-fixes branch from f954c07 to d3df485 Compare May 27, 2026 15:17

panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. review wanted PRs that need reviews. labels May 27, 2026

atlowChemi reviewed May 31, 2026

View reviewed changes

atlowChemi approved these changes May 31, 2026

View reviewed changes

panva added the commit-queue Add this label to land a pull request using GitHub Actions. label May 31, 2026

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 31, 2026

nodejs-github-bot pushed a commit that referenced this pull request May 31, 2026

crypto: coerce -0 to +0 before native calls

9b7e761

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

nodejs-github-bot pushed a commit that referenced this pull request May 31, 2026

net: coerce -0 to +0 in BlockList prefixes

8bab410

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

nodejs-github-bot pushed a commit that referenced this pull request May 31, 2026

zlib: coerce -0 to +0 for crc32 seeds

9e58d9d

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

nodejs-github-bot closed this May 31, 2026

panva deleted the negative-zero-fixes branch May 31, 2026 14:35

aduh95 pushed a commit that referenced this pull request Jun 18, 2026

crypto: coerce -0 to +0 before native calls

9984b05

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 18, 2026

dns: coerce -0 to +0 in lookup and resolver inputs

0cf8342

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 18, 2026

fs: coerce -0 to +0 in mode flags and watch intervals

ceba08a

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 18, 2026

net: coerce -0 to +0 in BlockList prefixes

3988efa

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 18, 2026

zlib: coerce -0 to +0 for crc32 seeds

692215d

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

github-actions Bot mentioned this pull request Jun 22, 2026

2026-06-23, Version 26.4.0 (Current) #64058

Merged

aduh95 pushed a commit that referenced this pull request Jun 25, 2026

dns: coerce -0 to +0 in lookup and resolver inputs

0200292

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 25, 2026

fs: coerce -0 to +0 in mode flags and watch intervals

a7bdf3d

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 25, 2026

net: coerce -0 to +0 in BlockList prefixes

e69f82c

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

aduh95 pushed a commit that referenced this pull request Jun 25, 2026

zlib: coerce -0 to +0 for crc32 seeds

be4b3fe

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

panva added the backport-open-v24.x Indicate that the PR has an open backport label Jul 20, 2026

panva mentioned this pull request Jul 20, 2026

[v24.x backport] more (web)crypto #64629

Open

github-actions Bot mentioned this pull request Jul 21, 2026

2026-07-22, Version 24.19.0 'Krypton' (LTS) #64654

Open

aduh95 pushed a commit that referenced this pull request Jul 21, 2026

crypto: coerce -0 to +0 before native calls

4ce77bb

Signed-off-by: Filip Skokan <[email protected]> PR-URL: #63556 Backport-PR-URL: #64629 Reviewed-By: Antoine du Hamel <[email protected]>

Uh oh!

Uh oh!

Conversation

panva commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented May 25, 2026

Uh oh!

aduh95 May 25, 2026

Choose a reason for hiding this comment

Uh oh!

panva May 25, 2026

Choose a reason for hiding this comment

Uh oh!

aduh95 May 25, 2026

Choose a reason for hiding this comment

Uh oh!

TheOneTheOnlyJJ May 25, 2026

Choose a reason for hiding this comment

Uh oh!

panva May 26, 2026

Choose a reason for hiding this comment

Uh oh!

akshatgit May 28, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented May 27, 2026

Uh oh!

nodejs-github-bot commented May 27, 2026

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented May 27, 2026

Uh oh!

atlowChemi May 31, 2026

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

panva commented May 25, 2026 •

edited

Loading

codecov Bot commented May 25, 2026 •

edited

Loading