Skip to content

fix(query): don't corrupt query keys that collide with Object.prototype - #2797

Closed
contactjawad wants to merge 1 commit into
vuejs:mainfrom
contactjawad:fix-parsequery-prototype-keys
Closed

fix(query): don't corrupt query keys that collide with Object.prototype#2797
contactjawad wants to merge 1 commit into
vuejs:mainfrom
contactjawad:fix-parsequery-prototype-keys

Conversation

@contactjawad

@contactjawad contactjawad commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

A query key that happens to be an Object.prototype member gets mangled:

parseQuery('toString=foo')
// { toString: [ƒ toString, 'foo'] }, expected { toString: 'foo' }

parseQuery accumulates into a plain {} and checks for repeated keys with key in query. Since in walks the prototype chain, toString (and valueOf, hasOwnProperty, …) look like they already exist on the first occurrence, so the value gets wrapped in an array with the inherited function.

I switched the accumulator to Object.create(null) — which is what the TODO in the file and the experimental implementation already point at. Added a test.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed query parsing for parameter names that overlap with built-in object properties, such as toString, valueOf, and hasOwnProperty.

@netlify

netlify Bot commented Sep 2, 2026

Copy link
Copy Markdown

Deploy Preview for vue-router canceled.

Name Link
🔨 Latest commit 2ade4dd
🔍 Latest deploy log https://app.netlify.com/projects/vue-router/deploys/6a9838f8610516000812c7dc

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: b7871a18-cc06-4877-a871-0067c54de3b3

📥 Commits

Reviewing files that changed from the base of the PR and between 3944b93 and 2ade4dd.

📒 Files selected for processing (2)
  • packages/router/__tests__/parseQuery.spec.ts
  • packages/router/src/query.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

parseQuery now returns a null-prototype object. Tests verify that query keys named toString, valueOf, and hasOwnProperty retain their values.

Changes

Query parsing

Layer / File(s) Summary
Null-prototype query results
packages/router/src/query.ts, packages/router/__tests__/parseQuery.spec.ts
parseQuery now creates a null-prototype result object. Tests cover query keys that overlap with Object.prototype names.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 2ade4

This change prevents query keys matching inherited object properties from being corrupted while preserving repeated-key handling. It is localized and merge-ready after normal checks, with no actionable merge-blocking risk remaining.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing query keys that collide with Object.prototype properties from being corrupted.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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.

@posva

posva commented Sep 2, 2026

Copy link
Copy Markdown
Member

Thanks but see #2658

@posva posva closed this Sep 2, 2026
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.

2 participants