-
Notifications
You must be signed in to change notification settings - Fork 210
fix: normalize same-origin absolute URLs for client-side navigation #336
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import React, { forwardRef, useRef, useEffect, useCallback, useContext, createCo | |
| // so this resolves both via the Vite plugin and in direct vitest imports) | ||
| import { toRscUrl, getPrefetchedUrls, storePrefetchResponse } from "./navigation.js"; | ||
| import { isDangerousScheme } from "./url-safety.js"; | ||
| import { toSameOriginPath } from "./url-utils.js"; | ||
|
|
||
| interface NavigateEvent { | ||
| url: URL; | ||
|
|
@@ -146,10 +147,15 @@ function scrollToHash(hash: string): void { | |
| function prefetchUrl(href: string): void { | ||
| if (typeof window === "undefined") return; | ||
|
|
||
| const fullHref = withBasePath(href); | ||
| // Normalize same-origin absolute URLs to local paths before prefetching | ||
| let prefetchHref = href; | ||
| if (href.startsWith("http://") || href.startsWith("https://") || href.startsWith("//")) { | ||
| const localPath = toSameOriginPath(href); | ||
| if (localPath == null) return; // truly external — don't prefetch | ||
| prefetchHref = localPath; | ||
| } | ||
|
|
||
| // Don't prefetch external URLs | ||
| if (fullHref.startsWith("http://") || fullHref.startsWith("https://") || fullHref.startsWith("//")) return; | ||
| const fullHref = withBasePath(prefetchHref); | ||
|
|
||
| // Don't prefetch the same URL twice (keyed by rscUrl so the browser | ||
| // entry can clear the key when a cache entry is consumed) | ||
|
|
@@ -318,13 +324,18 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link( | |
| const node = internalRef.current; | ||
| if (!node) return; | ||
|
|
||
| // Don't prefetch external URLs | ||
| if (localizedHref.startsWith("http://") || localizedHref.startsWith("https://") || localizedHref.startsWith("//")) return; | ||
| // Normalize same-origin absolute URLs; skip truly external ones | ||
| let hrefToPrefetch = localizedHref; | ||
| if (localizedHref.startsWith("http://") || localizedHref.startsWith("https://") || localizedHref.startsWith("//")) { | ||
| const localPath = toSameOriginPath(localizedHref); | ||
| if (localPath == null) return; // truly external | ||
| hrefToPrefetch = localPath; | ||
| } | ||
|
|
||
| const observer = getSharedObserver(); | ||
| if (!observer) return; | ||
|
|
||
| observerCallbacks.set(node, () => prefetchUrl(localizedHref)); | ||
| observerCallbacks.set(node, () => prefetchUrl(hrefToPrefetch)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: |
||
| observer.observe(node); | ||
|
|
||
| return () => { | ||
|
|
@@ -353,13 +364,18 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link( | |
| return; | ||
| } | ||
|
|
||
| // External links: let the browser handle it | ||
| // External links: let the browser handle it. | ||
| // Same-origin absolute URLs (e.g. http://localhost:3000/about) are | ||
| // normalized to local paths so they get client-side navigation. | ||
| let navigateHref = resolvedHref; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice detail: using |
||
| if ( | ||
| resolvedHref.startsWith("http://") || | ||
| resolvedHref.startsWith("https://") || | ||
| resolvedHref.startsWith("//") | ||
| ) { | ||
| return; | ||
| const localPath = toSameOriginPath(resolvedHref); | ||
| if (localPath == null) return; // truly external | ||
| navigateHref = localPath; | ||
| } | ||
Divkix marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| e.preventDefault(); | ||
|
|
@@ -395,7 +411,7 @@ const Link = forwardRef<HTMLAnchorElement, LinkProps>(function Link( | |
| } | ||
|
|
||
| // Resolve relative hrefs (#hash, ?query) against current URL | ||
| const absoluteHref = resolveRelativeHref(resolvedHref); | ||
| const absoluteHref = resolveRelativeHref(navigateHref); | ||
| const absoluteFullHref = withBasePath(absoluteHref); | ||
|
|
||
| // Hash-only change: update URL and scroll to target, skip RSC fetch | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /** | ||
| * Shared URL utilities for same-origin detection. | ||
| * | ||
| * Used by link.tsx, navigation.ts, and router.ts to normalize | ||
| * same-origin absolute URLs to local paths for client-side navigation. | ||
| */ | ||
|
|
||
| /** | ||
| * If `url` is an absolute same-origin URL, return the local path | ||
| * (pathname + search + hash). Returns null for truly external URLs | ||
| * or on the server (where origin is unknown). | ||
| */ | ||
| export function toSameOriginPath(url: string): string | null { | ||
| if (typeof window === "undefined") return null; | ||
| try { | ||
| const parsed = url.startsWith("//") | ||
| ? new URL(url, window.location.origin) | ||
| : new URL(url); | ||
| if (parsed.origin === window.location.origin) { | ||
| return parsed.pathname + parsed.search + parsed.hash; | ||
| } | ||
| } catch { | ||
| // not a valid absolute URL — ignore | ||
| } | ||
| return null; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.