fix(router): limit UrlParser recursion depth to prevent stack overflow

Deeply nested parentheses in URLs (e.g. `(a/(b/(c...)))`) trigger recursive calls in `UrlParser`, which can lead to a `RangeError: Maximum call stack size exceeded`. While such errors are generally caught by the framework, relying on the runtime's stack limit is unpredictable across different environments and engine states (e.g. varying stack sizes in different browsers or Node.js versions).

The deeply nested parentheses  can cause a stack overflow. While such URLs can be valid (e.g., `(a/(b/(c...)))`) and serialize to simple paths (e.g., /a/b/c), excessive nesting is unreasonable and likely malicious or accidental.

Linear paths (e.g. /a/b/c/d) are parsed iteratively and do NOT trigger recursion. Only parentheses trigger recursion.

This commit introduces a recursion depth limit of 50. If parsing exceeds this depth, the router will now throw a specific `UNPARSABLE_URL` error with the message "URL is too deep". This ensures a deterministic failure mode that is easier for applications to handle than a crash or generic RangeError.

The limit of 50 is chosen as it should accommodate any reasonable application URL structure (including complex named outlets) while providing a safe upper bound against abusive payloads.

This is essentially a refactor of the error state:

* Before: RangeError (System says "I'm out of stack memory")
* After: RuntimeError (Validator says "Input is invalid")

This provides:

* Semantic Correctness: The error now correctly blames the input ("URL too deep"), not the environment ("Stack full").
* Cross-Platform Consistency: The limit is the same in Chrome, Firefox, Node, and Deno, regardless of their internal recursion limits.
* Fast Failure: We stop at depth 50 instead of depth ~15,000, saving those cycles (though CPU cost is negligible either way).

"wide" URLs are now theoretically more expensive than "deep" URLs (because deep ones fail fast), but both are well within safe bounds for any reasonable input size.

(cherry picked from commit 458bc4a2c8)
This commit is contained in:
Andrew Scott
2026-01-26 11:40:04 -08:00
committed by Leon Senft
parent 0a4c647ef0
commit d6268c0bbb
2 changed files with 27 additions and 5 deletions
+11 -5
View File
@@ -612,7 +612,13 @@ class UrlParser {
return this.consumeOptional('#') ? decodeURIComponent(this.remaining) : null;
}
private parseChildren(): {[outlet: string]: UrlSegmentGroup} {
private parseChildren(depth = 0): {[outlet: string]: UrlSegmentGroup} {
if (depth > 50) {
throw new RuntimeError(
RuntimeErrorCode.UNPARSABLE_URL,
(typeof ngDevMode === 'undefined' || ngDevMode) && 'URL is too deep',
);
}
if (this.remaining === '') {
return {};
}
@@ -632,12 +638,12 @@ class UrlParser {
let children: {[outlet: string]: UrlSegmentGroup} = {};
if (this.peekStartsWith('/(')) {
this.capture('/');
children = this.parseParens(true);
children = this.parseParens(true, depth);
}
let res: {[outlet: string]: UrlSegmentGroup} = {};
if (this.peekStartsWith('(')) {
res = this.parseParens(false);
res = this.parseParens(false, depth);
}
if (segments.length > 0 || Object.keys(children).length > 0) {
@@ -723,7 +729,7 @@ class UrlParser {
}
// parse `(a/b//outlet_name:c/d)`
private parseParens(allowPrimary: boolean): {[outlet: string]: UrlSegmentGroup} {
private parseParens(allowPrimary: boolean, depth: number): {[outlet: string]: UrlSegmentGroup} {
const segments: {[key: string]: UrlSegmentGroup} = {};
this.capture('(');
@@ -750,7 +756,7 @@ class UrlParser {
outletName = PRIMARY_OUTLET;
}
const children = this.parseChildren();
const children = this.parseChildren(depth + 1);
segments[outletName ?? PRIMARY_OUTLET] =
Object.keys(children).length === 1 && children[PRIMARY_OUTLET]
? children[PRIMARY_OUTLET]
@@ -425,6 +425,22 @@ describe('url serializer', () => {
it('should throw when missing closing )', () => {
expect(() => url.parse('/one/(left')).toThrowError();
});
it('should throw when the URL is too deeply nested', () => {
let urlStr = 'a';
for (let i = 0; i < 60; i++) {
urlStr = `p/(${urlStr})`;
}
expect(() => url.parse(`/${urlStr}`)).toThrowError(/URL is too deep/);
});
it('should not throw when the URL is nested within the limit', () => {
let urlStr = 'a';
for (let i = 0; i < 40; i++) {
urlStr = `p/(${urlStr})`;
}
expect(() => url.parse(`/${urlStr}`)).not.toThrow();
});
});
});