Skip to content

fix(router): strip null characters from serialized URLs#68175

Open
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/router_issue_47264
Open

fix(router): strip null characters from serialized URLs#68175
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/router_issue_47264

Conversation

@arturovt
Copy link
Copy Markdown
Contributor

@arturovt arturovt commented Apr 13, 2026

Null characters (\u0000) in route path params, matrix params, or query params were encoded to %00 by encodeURIComponent. Browsers reject URLs containing %00 with a SecurityError when passed to history.pushState/replaceState, causing an unhandled promise rejection and crashing navigation.

The fix strips %00 from the output of encodeUriString, which is the shared base used by encodeUriSegment and encodeUriQuery, covering path segments, matrix params, and query params in a single change.

Closes #47264

Google Internal ref: b/239974353

Null characters (\u0000) in route path params, matrix params, or query
params were encoded to `%00` by `encodeURIComponent`. Browsers reject
URLs containing `%00` with a SecurityError when passed to
`history.pushState`/`replaceState`, causing an unhandled promise rejection
and crashing navigation.

The fix strips `%00` from the output of `encodeUriString`, which is the
shared base used by `encodeUriSegment` and `encodeUriQuery`, covering
path segments, matrix params, and query params in a single change.

Closes angular#47264
@ngbot ngbot bot added this to the Backlog milestone Apr 13, 2026
@thePunderWoman
Copy link
Copy Markdown
Contributor

Woah, looks like you've opened a lot of issues/PRs recently. While we appreciate contributions from the community, triaging and reviewing a large influx of content in a short time period takes time away from other ongoing projects. As a result, we're closing these issues/PRs to maintain the team's focus.

Note that this is not necessarily a rejection of the goals or direction of any of these contributions in particular, so much as a reflection of the team's current capacity and priorities.

You are welcome to open a smaller subset of issues/PRs in accordance with our policy focused on the most important and impactful contributions and we will do our best to prioritize a response as soon as possible.

@arturovt arturovt deleted the fix/router_issue_47264 branch April 13, 2026 16:53
@arturovt arturovt restored the fix/router_issue_47264 branch April 13, 2026 17:27
@arturovt arturovt marked this pull request as ready for review April 14, 2026 12:53
@pullapprove pullapprove bot requested a review from atscott April 14, 2026 12:53
Copy link
Copy Markdown
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

Don’t this break null characters used with hash location strategy or a strategy that falls back to location.assign it push or replace fails?

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Apr 14, 2026

From the report, this would change the navigation from a failure to go to the user page to a successful navigation to the home page, which in my opinion is wrong

@arturovt
Copy link
Copy Markdown
Contributor Author

@atscott nice catch. Right now the fix strips %00 inside encodeUriString, which is used by both strategies — so it ends up affecting HashLocationStrategy too. In the hash case, the null character just sits in the fragment (like example.com#/path%00value), and browsers don’t actually reject that. So we’re effectively “fixing” something that isn’t broken there, and in the process we silently change the value.

It probably makes more sense to move the %00 stripping into PathLocationStrategy.pushState / replaceState, right before calling history.pushState / replaceState. That keeps the fix limited to where the SecurityError actually happens and avoids touching hash-based navigation.

On the behavior change — yeah, this turns what used to be a hard failure into a silent success with a slightly corrupted URL. But compared to navigation completely breaking due to an unhandled SecurityError, that might still be the better user experience. If we do want to preserve the “fail” behavior for null-char URLs, another option would be to catch the SecurityError in PathLocationStrategy and convert it into a NavigationError. That said, it’s a bit more invasive.

Curious what direction you think makes the most sense here.

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Apr 14, 2026

Does it not already convert to NavigationError? Is this even still relevant? This seems to work, at least in Chrome, today: https://stackblitz.com/edit/stackblitz-starters-3qdalujd

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.

Navigating to url with "/:pathVariable" with null unicode symbols causes History.PushState error on route change

3 participants