Skip to content

fix: prevent precision loss for large integer strings exceeding MAX_SAFE_INTEGER (#152) - #156

Open
guoyangzhen wants to merge 1 commit into
unjs:mainfrom
guoyangzhen:fix-large-integer-precision
Open

guoyangzhen wants to merge 1 commit into
unjs:mainfrom
guoyangzhen:fix-large-integer-precision

Conversation

@guoyangzhen

@guoyangzhen guoyangzhen commented Mar 23, 2026

Copy link
Copy Markdown

🔗 Linked issue

Closes #152

📝 Description

destr() converts numeric-looking strings to JavaScript Numbers even when they exceed Number.MAX_SAFE_INTEGER (9007199254740991), causing silent precision loss:

destr('9007199254740993') // returns 9007199254740992 (wrong!)

This is because JsonSigRx matches integers with up to 16 digits (\d{1,16}), and MAX_SAFE_INTEGER is a 16-digit number. Any 16-digit integer ≥ 9007199254740992 gets corrupted.

Fix

Reduce the integer digit limit from 16 to 15 in JsonSigRx:

- const JsonSigRx = /^\s*["[{]|^\s*-?\d{1,16}(\.\d{1,17})?([Ee][+-]?\d+)?\s*$/;
+ const JsonSigRx = /^\s*["[{]|^\s*-?\d{1,15}(\.\d{1,17})?([Ee][+-]?\d+)?\s*$/;

With 15 digits, the maximum matched integer is 999999999999999 (< MAX_SAFE_INTEGER), so all matched integers are safe.

Impact

  • Decimals: unaffected (e.g., "9007199254740993.5" still has decimal handling)
  • Scientific notation: unaffected (e.g., "9.007e15" still handled)
  • 16+ digit pure integers: now returned as strings instead of being silently corrupted
  • 0-15 digit integers: unchanged behavior

This is technically a behavior change for 16-digit integer strings, but the current behavior is data corruption — returning a string is always preferable to returning an incorrect number.

Summary by CodeRabbit

  • Bug Fixes
    • Refined numeric input validation to improve consistency and accuracy in handling large numeric values during JSON processing.

@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9c5ace1c-2d42-4594-8ebb-1588340f60b8

📥 Commits

Reviewing files that changed from the base of the PR and between 469ec7f and 2862c0b.

📒 Files selected for processing (1)
  • src/index.ts

📝 Walkthrough

Walkthrough

Modified the JsonSigRx regular expression in src/index.ts to restrict integer digit length from 16 to 15 digits, preventing large numeric strings beyond JavaScript's safe integer limit from being parsed as Numbers and suffering precision loss.

Changes

Cohort / File(s) Summary
Numeric Validation Regex
src/index.ts
Reduced maximum integer digit length in JsonSigRx from \d{1,16} to \d{1,15} to prevent coercion of unsafe large numbers that exceed Number.MAX_SAFE_INTEGER (9007199254740991).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A regex tweak, so small yet mighty true,
Fifteen digits now, not sixteen through and through,
No more precision lost in numeric disguise,
Safe integers reign—precision satisfies! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: reducing the integer digit limit in JsonSigRx from 16 to 15 to prevent unsafe Number conversion of large integer strings exceeding MAX_SAFE_INTEGER.
Linked Issues check ✅ Passed The PR fully addresses issue #152 by changing JsonSigRx to limit integer digits to 15, ensuring 16+ digit pure integers remain strings instead of unsafe Numbers, while maintaining correct behavior for decimals and scientific notation.
Out of Scope Changes check ✅ Passed The single-line regex change in src/index.ts is directly scoped to fixing the precision loss issue described in #152, with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

destr incorrectly parses large numeric strings into unsafe Numbers, causing precision loss

1 participant