Skip to content

lib: fix evenRound sign for fractional values in (-1, 1) - #62837

Closed
deepview-autofix wants to merge 1 commit into
nodejs:mainfrom
deepview-autofix:deepview/500aa18f3b
Closed

lib: fix evenRound sign for fractional values in (-1, 1)#62837
deepview-autofix wants to merge 1 commit into
nodejs:mainfrom
deepview-autofix:deepview/500aa18f3b

Conversation

@deepview-autofix

@deepview-autofixdeepview-autofix commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

evenRound derived the rounding direction from MathSign(i) where i = integerPart(x). For any x whose truncated integer part is zero (e.g. 0.7, -0.7), sign was 0, so the non-halfway branch returned i + sign === 0 instead of rounding away from zero.

This produced incorrect [Clamp] WebIDL integer conversions for fractional inputs in (-1, 0) ∪ (0, 1) that are not exactly ±0.5

Observable e.g. via new Blob([Uint8Array.of(1,2,3)]).slice(-0.9999999) returning the whole blob instead of a 1-byte slice.

Use MathSign(x) so the rounding direction follows the sign of the original value.

`evenRound` derived the rounding direction from `MathSign(i)` where
`i = integerPart(x)`. For any `x` whose truncated integer part is zero
(e.g. `0.7`, `-0.7`), `sign` was `0`, so the non-halfway branch returned
`i + sign === 0` instead of rounding away from zero. This produced
incorrect `[Clamp]` WebIDL integer conversions for fractional inputs in
`(-1, 0) ∪ (0, 1)` that are not exactly `±0.5`, observable e.g. via
`new Blob([Uint8Array.of(1,2,3)]).slice(-0.9999999)` returning the whole
blob instead of a 1-byte slice. Use `MathSign(x)` so the rounding
direction follows the sign of the original value.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com>
Co-Authored-By: Nikita Skovoroda <chalkerx@gmail.com>
Signed-off-by: Nikita Skovoroda <chalkerx@gmail.com>
@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 20, 2026
@ChALkeRChALkeR added the web-standards Issues and PRs related to Web APIs label Apr 20, 2026

@ChALkeRChALkeR left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is mine (scanner + autofixer)


Node.js, prior to the patch:

image

Chrome:

image

@ChALkeR
ChALkeR marked this pull request as ready for review April 20, 2026 03:55
@codecov

codecovBot commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.62%. Comparing base (14e16db) to head (cc729e8).
⚠️ Report is 112 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #62837 +/- ##
=======================================
Coverage 89.62% 89.62% =======================================
Files 706 706 Lines 219136 219136 Branches 41987 41981 -6 =======================================
+ Hits 196404 196410 +6 - Misses 14611 14614 +3 + Partials 8121 8112 -9 
Files with missing linesCoverage Δ
lib/internal/webidl.js96.62% <100.00%> (ø)

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

@ChALkeR

Copy link
Copy Markdown
Member

Closing in favor of #62979

@ChALkeRChALkeR closed this Apr 27, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.web-standardsIssues and PRs related to Web APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@deepview-autofix@ChALkeR@ljharb@nodejs-github-bot