Skip to content

fix(router-core): prevent lossy string coercion in defaultParseSearch (#7650)#7679

Draft
sanjibani wants to merge 1 commit into
TanStack:mainfrom
sanjibani:fix/default-parse-search-no-lossy-coercion
Draft

fix(router-core): prevent lossy string coercion in defaultParseSearch (#7650)#7679
sanjibani wants to merge 1 commit into
TanStack:mainfrom
sanjibani:fix/default-parse-search-no-lossy-coercion

Conversation

@sanjibani

Copy link
Copy Markdown

Summary

defaultParseSearch currently runs JSON.parse on every value, which silently coerces opaque strings that happen to be valid JSON into JSON values:

Input Before After
?codAut=662E41 { codAut: 6.62e+43 } { codAut: '662E41' }
?id=723421968459640832 { id: 723421968459640800 } (precision lost) { id: '723421968459640832' }
?sig=abcdef0123456789 { sig: 4.2269...e+15 } (mangled) { sig: 'abcdef0123456789' }
?ulid=01H8XGJWBWBAQ4ZEXAMPLE { ulid: 1.34... (mangled) } { ulid: '01H8XGJWBWBAQ4ZEXAMPLE' }

These values commonly come from upstream systems: identity provider codes, social auth IDs, ULID/UUIDv7, large Snowflake/ClickHouse keys, hex signatures, etc. They parse to valid JSON because the JSON grammar accepts hex-digit-only strings as numbers, but the parsed value cannot be re-serialized back to the original string.

The default parser now uses a roundtrip check: a parsed value is only accepted when re-serializing it with stringify reproduces the original string. The existing behavior for the documented JSON-shaped values (?foo=1 → number, ?foo={"a":1} → object, etc.) is preserved because those values roundtrip cleanly.

Changes

Notes

  • parseSearchWith is part of the public surface (re-exported from router-core, react-router, solid-router, vue-router). The new stringify parameter is optional with a JSON.stringify default, so existing callers are unaffected.
  • I considered moving the check inside defaultParseSearch only, but parseSearchWith is the documented customization point for this behavior, so the roundtrip lives at that boundary.
  • Alternative considered: only check when the original string is >= 13 chars of all digits/hex. Rejected because it would be a workaround for JSON.parse rather than a correctness fix and would miss the 662E416.62e+43 case (6 chars).

Fixes #7650

Test plan

  • pnpm exec vitest run packages/router-core/tests/searchParams.test.ts — 44/44 pass (40 existing + 4 new)
  • pnpm exec tsc --noEmit -p packages/router-core — no errors
  • Reviewer: run the test command above; verify the new opaque string roundtrip block fails on main and passes on this branch.

defaultParseSearch ran JSON.parse on every string value, which silently
coerced opaque strings that happen to be valid JSON into JSON values:

  ?codAut=662E41                 -> { codAut: 6.62e+43 }
  ?id=723421968459640832         -> { id: 723421968459640800 } (precision lost)
  ?sig=abcdef0123456789          -> { sig: 4.22696e+15 } (mangled)
  ?ulid=01H8XGJWBWBAQ4ZEXAMPLE   -> { ulid: 134... (mangled)

The default parser now uses a roundtrip check: a parsed value is only
accepted when re-serializing it with stringify reproduces the original
string. This rejects any lossy conversion while preserving the
existing behavior for the documented JSON-shaped values.

parseSearchWith also grows an optional second 'stringify' argument so
users with a non-JSON serializer can pass a matching re-serializer.
Default is JSON.stringify, so the existing defaultParseSearch export
keeps the same call shape.

Fixes TanStack#7650
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e0d33e34-a776-4dd8-ae84-28a1337dc6bf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ 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.

@sanjibani

Copy link
Copy Markdown
Author

Opened as draft to work around a cross-fork GraphQL createPullRequest token-rotation issue on this side. The branch is ready for review — flipping the PR out of draft can be done from either side.

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.

defaultParseSearch corrupts string search params that look like JSON numbers (e.g. "662E41", large integers) — lossy and unrecoverable on inbound URLs

1 participant