Skip to content

fix: use safe hasOwnProperty when parsing query params#69259

Open
rootvector2 wants to merge 1 commit into
angular:mainfrom
rootvector2:query-param-hasownproperty
Open

fix: use safe hasOwnProperty when parsing query params#69259
rootvector2 wants to merge 1 commit into
angular:mainfrom
rootvector2:query-param-hasownproperty

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

PR Checklist

PR Type

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other

What is the current behavior?

Issue Number: N/A

parseQueryParam in the router and parseKeyValue in @angular/common/upgrade check for an existing key by calling hasOwnProperty directly on the object they accumulate parsed query params into, whose keys come straight from the URL. A query string like ?hasOwnProperty=x&a=b overwrites that method with a string, so the existence check for the next param throws TypeError: params.hasOwnProperty is not a function and the whole parse fails on otherwise valid input reachable from any crafted link.

What is the new behavior?

Both call sites use the non-clobberable Object.prototype.hasOwnProperty.call(...), which router/src/shared.ts already uses for param lookups, so a hasOwnProperty query key parses like any other key and the repeated-key array handling is unchanged. Spotted while auditing the query-string parsers for the unsafe obj.hasOwnProperty(userKey) shape.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove Bot requested a review from crisbeto June 9, 2026 12:32
Comment thread packages/router/src/url_tree.ts Outdated
const decodedVal = decodeQuery(value);

if (params.hasOwnProperty(decodedKey)) {
if (Object.prototype.hasOwnProperty.call(params, decodedKey)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use Object.hasOwn

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call, switched both call sites to Object.hasOwn.

@JeanMeche JeanMeche requested review from atscott and removed request for crisbeto June 9, 2026 12:34
`parseQueryParam` and the AngularJS-compat `parseKeyValue` accumulate query params into a plain object and check key presence with `obj.hasOwnProperty`, so a `hasOwnProperty` query key clobbers the method and the next lookup throws `TypeError`. Switch both to `Object.hasOwn`, which can't be shadowed by a query key.
@rootvector2 rootvector2 force-pushed the query-param-hasownproperty branch from baae253 to 04004ab Compare June 9, 2026 18:08
@ngbot ngbot Bot added this to the Backlog milestone Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants