Skip to content

fix: restrict client external navigation to http(s) and protocol-relative URLs #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JaredStowell wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: restrict client external navigation to http(s) and protocol-relative URLs #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/vinext/src/shims/navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ export function useParams<
}

/**
* Check if a href is an external URL (any URL scheme per RFC 3986, or protocol-relative).
* Check if a href is an external URL (http/https, or protocol-relative).
*/
function isExternalUrl(href: string): boolean {
return /^[a-z][a-z0-9+.-]*:/i.test(href) || href.startsWith("//");
return /^https?:\/\//i.test(href) || href.startsWith("//");
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 7, 2026

Choose a reason for hiding this comment

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

P2 Badge Treat non-HTTP absolute schemes as external in App Router

Restricting isExternalUrl to only http(s) and // makes values like mailto:, tel:, and sms: fall into the internal navigation path, so useRouter().push()/replace() reaches history.pushState with a non-HTTP absolute URL and can throw SecurityError in browsers instead of delegating to native navigation. This is a regression from the previous behavior and breaks legitimate external protocol links; only dangerous schemes should be blocked.

Useful? React with 👍 / 👎.

}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/vinext/src/shims/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ export function applyNavigationLocale(url: string, locale?: string): string {
return `/${locale}${url.startsWith("/") ? url : `/${url}`}`;
}

/** Check if a URL is external (any URL scheme per RFC 3986, or protocol-relative) */
/** Check if a URL is external (http/https, or protocol-relative) */
export function isExternalUrl(url: string): boolean {
return /^[a-z][a-z0-9+.-]*:/i.test(url) || url.startsWith("//");
return /^https?:\/\//i.test(url) || url.startsWith("//");
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 7, 2026

Choose a reason for hiding this comment

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

P2 Badge Keep non-HTTP schemes external in Pages Router helper

The same narrowing in the Pages Router isExternalUrl means next/router push/replace now treats mailto:/tel: URLs as internal, then executes window.history.pushState/replaceState on those values and can throw at runtime (or misroute when a basePath is set). This breaks existing external-protocol navigation semantics; the helper should still classify non-dangerous absolute schemes as external.

Useful? React with 👍 / 👎.

}

/** Check if a href is only a hash change relative to the current URL */
Expand Down
6 changes: 6 additions & 0 deletions tests/link.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ describe("isExternalUrl", () => {
it("hash-only is not external", () => {
expect(isExternalUrl("#section")).toBe(false);
});

it("blocks dangerous schemes from being treated as external", () => {
expect(isExternalUrl("javascript:alert(1)")).toBe(false);
expect(isExternalUrl("data:text/html,<h1>x</h1>")).toBe(false);
});

});

// ─── isHashOnlyChange ───────────────────────────────────────────────────
Expand Down
6 changes: 6 additions & 0 deletions tests/shims.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5528,6 +5528,12 @@ describe("Pages Router router helpers", () => {
it("returns false for query-only", () => {
expect(isExternalUrl("?foo=1")).toBe(false);
});

it("returns false for dangerous schemes", () => {
expect(isExternalUrl("javascript:alert(1)")).toBe(false);
expect(isExternalUrl("data:text/html,<h1>x</h1>")).toBe(false);
});

});

describe("isHashOnlyChange", () => {
Expand Down