Skip to content

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

Open
deepview-autofix wants to merge 1 commit intonodejs:mainfrom
deepview-autofix:deepview/500aa18f3b
Open

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

Conversation

@deepview-autofix
Copy link
Copy Markdown

@deepview-autofix deepview-autofix commented Apr 20, 2026

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-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 20, 2026
@ChALkeR ChALkeR added the web-standards Issues and PRs related to Web APIs label Apr 20, 2026
Copy link
Copy Markdown
Member

@ChALkeR ChALkeR left a comment

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
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.62%. Comparing base (14e16db) to head (cc729e8).

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 lines Coverage Δ
lib/internal/webidl.js 96.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.

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

Labels

needs-ci PRs that need a full CI run. web-standards Issues and PRs related to Web APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants