fix(router-core): keep unsafe integer search param literals as strings - #7860
Open
lastMove wants to merge 1 commit into
Open
fix(router-core): keep unsafe integer search param literals as strings#7860lastMove wants to merge 1 commit into
lastMove wants to merge 1 commit into
Conversation
defaultParseSearch JSON-parses every value, so integer literals beyond Number.MAX_SAFE_INTEGER lose precision to float64 rounding (e.g. the 18-digit Meta ad id 120247103250460643 parses to 120247103250460640). Any redirect or Link that re-serializes the search then writes the corrupted value back into the URL — silently breaking ad/click id attribution for marketing landing pages using the default codec. Integer literals that fail Number.isSafeInteger now stay strings (both in defaultParseSearch and the symmetric re-serialization check in defaultStringifySearch, so they round-trip unquoted). Values that were previously parsed to numbers in this range were already corrupted, so no working behavior is lost.
Contributor
📝 WalkthroughWalkthroughChangesUnsafe integer search parameters
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/router-core/src/searchParams.ts`:
- Around line 4-18: Update integerLiteralRegex used by
parseJsonPreservingUnsafeIntegers to match JSON integer literals with leading
and trailing whitespace, while preserving the existing optional-negative-sign
and digit constraints. Keep the Number.isSafeInteger(Number(str)) validation and
JSON.parse behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ee64d62-50b5-49f7-8cbd-73a07d905d8c
📒 Files selected for processing (3)
.changeset/big-integer-search-params.mdpackages/router-core/src/searchParams.tspackages/router-core/tests/searchParams.test.ts
Comment on lines
+4
to
+18
| const integerLiteralRegex = /^-?\d+$/ | ||
|
|
||
| /** | ||
| * `JSON.parse`, except integer literals beyond `Number.MAX_SAFE_INTEGER` are | ||
| * rejected (kept as strings by the callers below): parsing them to a float64 | ||
| * silently rounds the value (e.g. "120247103250460643" becomes | ||
| * 120247103250460640), so any re-serialization corrupts the original — a | ||
| * common hazard with 18-digit ad/click ids in marketing URLs. | ||
| */ | ||
| function parseJsonPreservingUnsafeIntegers(str: string): any { | ||
| if (integerLiteralRegex.test(str) && !Number.isSafeInteger(Number(str))) { | ||
| throw new Error('Integer literal exceeds safe integer range') | ||
| } | ||
| return JSON.parse(str) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Account for leading and trailing whitespace in safety checks.
JSON.parse natively ignores leading and trailing whitespace. A value like ?foo=%20120247103250460643%20 will bypass the strict regex check due to the spaces, but will still be successfully parsed and silently rounded by JSON.parse, ultimately corrupting the value on re-serialization.
Updating the regex to allow whitespace ensures these values are properly protected. Number() natively parses whitespace-padded numbers, so the check inside the function will continue to work exactly as expected.
🛡️ Proposed fix to handle whitespace
-const integerLiteralRegex = /^-?\d+$/
+const integerLiteralRegex = /^\s*-?\d+\s*$/
/**
* `JSON.parse`, except integer literals beyond `Number.MAX_SAFE_INTEGER` are
* rejected (kept as strings by the callers below): parsing them to a float64
* silently rounds the value (e.g. "120247103250460643" becomes
* 120247103250460640), so any re-serialization corrupts the original — a
* common hazard with 18-digit ad/click ids in marketing URLs.
*/
function parseJsonPreservingUnsafeIntegers(str: string): any {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const integerLiteralRegex = /^-?\d+$/ | |
| /** | |
| * `JSON.parse`, except integer literals beyond `Number.MAX_SAFE_INTEGER` are | |
| * rejected (kept as strings by the callers below): parsing them to a float64 | |
| * silently rounds the value (e.g. "120247103250460643" becomes | |
| * 120247103250460640), so any re-serialization corrupts the original — a | |
| * common hazard with 18-digit ad/click ids in marketing URLs. | |
| */ | |
| function parseJsonPreservingUnsafeIntegers(str: string): any { | |
| if (integerLiteralRegex.test(str) && !Number.isSafeInteger(Number(str))) { | |
| throw new Error('Integer literal exceeds safe integer range') | |
| } | |
| return JSON.parse(str) | |
| } | |
| const integerLiteralRegex = /^\s*-?\d+\s*$/ | |
| /** | |
| * `JSON.parse`, except integer literals beyond `Number.MAX_SAFE_INTEGER` are | |
| * rejected (kept as strings by the callers below): parsing them to a float64 | |
| * silently rounds the value (e.g. "120247103250460643" becomes | |
| * 120247103250460640), so any re-serialization corrupts the original — a | |
| * common hazard with 18-digit ad/click ids in marketing URLs. | |
| */ | |
| function parseJsonPreservingUnsafeIntegers(str: string): any { | |
| if (integerLiteralRegex.test(str) && !Number.isSafeInteger(Number(str))) { | |
| throw new Error('Integer literal exceeds safe integer range') | |
| } | |
| return JSON.parse(str) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/router-core/src/searchParams.ts` around lines 4 - 18, Update
integerLiteralRegex used by parseJsonPreservingUnsafeIntegers to match JSON
integer literals with leading and trailing whitespace, while preserving the
existing optional-negative-sign and digit constraints. Keep the
Number.isSafeInteger(Number(str)) validation and JSON.parse behavior unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
defaultParseSearchJSON-parses every search param value. Integer literals beyondNumber.MAX_SAFE_INTEGERlose precision to float64 rounding:Any
redirector<Link>that re-serializes the search then writes the corrupted value back into the URL. This silently breaks attribution for marketing landing pages: Meta/TikTok ad and campaign ids are 18–19 digit integers passed asutm_campaign_id/utm_ad_id/etc., so a route that redirects (e.g./landing→/landing/step-1, carryingsearch) rounds every id before analytics SDKs readlocation.search. We found this in production after revenue couldn't be matched back to real ad ids — the stored ids all ended in float64-representable digits.Fix
Integer literals that fail
Number.isSafeIntegerare kept as strings. The same guard is applied to the parser used bydefaultStringifySearch's re-serialization symmetry check, so preserved literals round-trip unquoted (?id=120247103250460643, not?id=%22120247103250460643%22).No working behavior is lost: values in this range were already being silently corrupted, so nothing could have depended on the (wrong) numeric result. Safe integers (
?page=2,?ts=1716854400) parse to numbers exactly as before.Tests
MAX_SAFE_INTEGER + 2are preserved as strings and round-trip losslessly; safe integers still parse as numbers.router-coresuite: 1203 passed, 3 pre-existing expected fails, no type errors.Changeset included (
@tanstack/router-core: patch).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests