From 1211f5e13c00c3fa02a2ac0d0eb911057aa74b5e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 4 Apr 2024 12:28:09 -0600 Subject: [PATCH] fix: reused existing parse-relative-url method --- packages/next/src/build/utils.ts | 2 +- ...tatic-generation-async-storage.external.ts | 6 ++++- packages/next/src/lib/metadata/metadata.tsx | 3 +-- packages/next/src/lib/url.ts | 10 ------- .../next/src/server/app-render/app-render.tsx | 26 +++++++++++-------- .../app-render/create-component-tree.tsx | 4 +-- .../server/app-render/dynamic-rendering.ts | 7 +++-- .../server/app-render/validate-url.test.ts | 13 ---------- .../src/server/app-render/validate-url.ts | 18 ------------- ...static-generation-async-storage-wrapper.ts | 16 +++++++++--- .../future/route-modules/app-route/module.ts | 2 +- packages/next/src/server/lib/patch-fetch.ts | 23 ++++++++-------- .../server/web/spec-extension/revalidate.ts | 5 +--- .../web/spec-extension/unstable-cache.ts | 2 +- .../router/utils/parse-relative-url.test.ts | 20 ++++++++++++++ .../lib/router/utils/parse-relative-url.ts | 21 ++++++++++++--- .../lib/router/utils/resolve-rewrites.ts | 4 +-- 17 files changed, 94 insertions(+), 88 deletions(-) delete mode 100644 packages/next/src/server/app-render/validate-url.test.ts delete mode 100644 packages/next/src/server/app-render/validate-url.ts create mode 100644 packages/next/src/shared/lib/router/utils/parse-relative-url.test.ts diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 6e1c9c06bc6bf..9e260ccfb1f60 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -1360,7 +1360,7 @@ export async function buildAppStaticPaths({ return StaticGenerationAsyncStorageWrapper.wrap( ComponentMod.staticGenerationAsyncStorage, { - urlPathname: page, + url: { pathname: page }, renderOpts: { originalPathname: page, incrementalCache, diff --git a/packages/next/src/client/components/static-generation-async-storage.external.ts b/packages/next/src/client/components/static-generation-async-storage.external.ts index 4433992a8f331..8b4a7d0ae6fb9 100644 --- a/packages/next/src/client/components/static-generation-async-storage.external.ts +++ b/packages/next/src/client/components/static-generation-async-storage.external.ts @@ -10,7 +10,11 @@ import { createAsyncLocalStorage } from './async-local-storage' export interface StaticGenerationStore { readonly isStaticGeneration: boolean readonly pagePath?: string - readonly urlPathname: string + /** + * The URL of the request. This only specifies the pathname and the search + * part of the URL. The other parts aren't accepted so they shouldn't be used. + */ + readonly url: { readonly pathname: string; readonly search: string } readonly incrementalCache?: IncrementalCache readonly isOnDemandRevalidate?: boolean readonly isPrerendering?: boolean diff --git a/packages/next/src/lib/metadata/metadata.tsx b/packages/next/src/lib/metadata/metadata.tsx index 1be2eb2e20717..3f1acb71cf3aa 100644 --- a/packages/next/src/lib/metadata/metadata.tsx +++ b/packages/next/src/lib/metadata/metadata.tsx @@ -58,8 +58,7 @@ export function createMetadataComponents({ ) => ParsedUrlQuery }): [React.ComponentType, React.ComponentType] { const metadataContext = { - // Make sure the pathname without query string - pathname: pathname.split('?')[0], + pathname, trailingSlash, } diff --git a/packages/next/src/lib/url.ts b/packages/next/src/lib/url.ts index 32aea56082490..4f812328a2798 100644 --- a/packages/next/src/lib/url.ts +++ b/packages/next/src/lib/url.ts @@ -1,13 +1,3 @@ -const DUMMY_ORIGIN = 'http://n' - -function getUrlWithoutHost(url: string) { - return new URL(url, DUMMY_ORIGIN) -} - -export function getPathname(url: string) { - return getUrlWithoutHost(url).pathname -} - export function isFullStringUrl(url: string) { return /https?:\/\//.test(url) } diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 0ca1e501a4a18..f56f9ff0b1a2b 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -66,7 +66,6 @@ import { import { getSegmentParam } from './get-segment-param' import { getScriptNonceFromHeader } from './get-script-nonce-from-header' import { parseAndValidateFlightRouterState } from './parse-and-validate-flight-router-state' -import { validateURL } from './validate-url' import { createFlightRouterStateFromLoaderTree } from './create-flight-router-state-from-loader-tree' import { handleAction } from './action-handler' import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr' @@ -107,6 +106,7 @@ import { wrapClientComponentLoader, } from '../client-component-renderer-logger' import { createServerModuleMap } from './action-utils' +import { parseRelativeUrl } from '../../shared/lib/router/utils/parse-relative-url' export type GetDynamicParamFromSegment = ( // [slug] / [[slug]] / [...slug] @@ -280,7 +280,7 @@ async function generateFlight( }, getDynamicParamFromSegment, appUsingSizeAdjustment, - staticGenerationStore: { urlPathname }, + staticGenerationStore: { url }, query, requestId, flightRouterState, @@ -289,7 +289,7 @@ async function generateFlight( if (!options?.skipFlight) { const [MetadataTree, MetadataOutlet] = createMetadataComponents({ tree: loaderTree, - pathname: urlPathname, + pathname: url.pathname, trailingSlash: ctx.renderOpts.trailingSlash, query, getDynamicParamFromSegment, @@ -402,7 +402,7 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { GlobalError, createDynamicallyTrackedSearchParams, }, - staticGenerationStore: { urlPathname }, + staticGenerationStore: { url }, } = ctx const initialTree = createFlightRouterStateFromLoaderTree( tree, @@ -413,7 +413,7 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { const [MetadataTree, MetadataOutlet] = createMetadataComponents({ tree, errorType: asNotFound ? 'not-found' : undefined, - pathname: urlPathname, + pathname: url.pathname, trailingSlash: ctx.renderOpts.trailingSlash, query, getDynamicParamFromSegment: getDynamicParamFromSegment, @@ -449,7 +449,7 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) { { - const { pathname } = validateURL(req.url) + if (!req.url) { + throw new Error('Invalid URL') + } + + const url = parseRelativeUrl(req.url, undefined, false) return RequestAsyncStorageWrapper.wrap( renderOpts.ComponentMod.requestAsyncStorage, @@ -1435,7 +1439,7 @@ export const renderToHTMLOrFlight: AppPageRender = ( StaticGenerationAsyncStorageWrapper.wrap( renderOpts.ComponentMod.staticGenerationAsyncStorage, { - urlPathname: pathname, + url, renderOpts, }, (staticGenerationStore) => diff --git a/packages/next/src/server/app-render/create-component-tree.tsx b/packages/next/src/server/app-render/create-component-tree.tsx index 0ea17745987dc..5da5ddc9fc57a 100644 --- a/packages/next/src/server/app-render/create-component-tree.tsx +++ b/packages/next/src/server/app-render/create-component-tree.tsx @@ -229,7 +229,7 @@ async function createComponentTreeInternal({ if (typeof layoutOrPageMod?.revalidate !== 'undefined') { validateRevalidate( layoutOrPageMod?.revalidate, - staticGenerationStore.urlPathname + staticGenerationStore.url.pathname ) } @@ -534,7 +534,7 @@ async function createComponentTreeInternal({ , loadingData, ], diff --git a/packages/next/src/server/app-render/dynamic-rendering.ts b/packages/next/src/server/app-render/dynamic-rendering.ts index 148d095309f35..995cf4997a1fa 100644 --- a/packages/next/src/server/app-render/dynamic-rendering.ts +++ b/packages/next/src/server/app-render/dynamic-rendering.ts @@ -26,7 +26,6 @@ import React from 'react' import type { StaticGenerationStore } from '../../client/components/static-generation-async-storage.external' import { DynamicServerError } from '../../client/components/hooks-server-context' import { StaticGenBailoutError } from '../../client/components/static-generation-bailout' -import { getPathname } from '../../lib/url' const hasPostpone = typeof React.unstable_postpone === 'function' @@ -76,7 +75,7 @@ export function markCurrentScopeAsDynamic( store: StaticGenerationStore, expression: string ): void { - const pathname = getPathname(store.urlPathname) + const { pathname } = store.url if (store.isUnstableCacheCallback) { // inside cache scopes marking a scope as dynamic has no effect because the outer cache scope // creates a cache boundary. This is subtly different from reading a dynamic data source which is @@ -123,7 +122,7 @@ export function trackDynamicDataAccessed( store: StaticGenerationStore, expression: string ): void { - const pathname = getPathname(store.urlPathname) + const { pathname } = store.url if (store.isUnstableCacheCallback) { throw new Error( `Route ${pathname} used "${expression}" inside a function cached with "unstable_cache(...)". Accessing Dynamic data sources inside a cache scope is not supported. If you need this data inside a cached function use "${expression}" oustide of the cached function and pass the required dynamic data in as an argument. See more info here: https://nextjs.org/docs/app/api-reference/functions/unstable_cache` @@ -181,7 +180,7 @@ export function trackDynamicFetch( expression: string ) { if (store.prerenderState) { - postponeWithTracking(store.prerenderState, expression, store.urlPathname) + postponeWithTracking(store.prerenderState, expression, store.url.pathname) } } diff --git a/packages/next/src/server/app-render/validate-url.test.ts b/packages/next/src/server/app-render/validate-url.test.ts deleted file mode 100644 index 4415965c8fb74..0000000000000 --- a/packages/next/src/server/app-render/validate-url.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { validateURL } from './validate-url' - -describe('validateUrl', () => { - it('should return valid pathname', () => { - expect(validateURL('/').pathname).toBe('/') - expect(validateURL('/abc').pathname).toBe('/abc') - }) - - it('should throw for invalid pathname', () => { - expect(() => validateURL('//**y/\\')).toThrow() - expect(() => validateURL('//google.com')).toThrow() - }) -}) diff --git a/packages/next/src/server/app-render/validate-url.ts b/packages/next/src/server/app-render/validate-url.ts deleted file mode 100644 index 723442ec4f9fb..0000000000000 --- a/packages/next/src/server/app-render/validate-url.ts +++ /dev/null @@ -1,18 +0,0 @@ -const DUMMY_ORIGIN = 'http://n' -const INVALID_URL_MESSAGE = 'Invalid request URL' - -export function validateURL(url: string | undefined): URL { - if (!url) { - throw new Error(INVALID_URL_MESSAGE) - } - try { - const parsed = new URL(url, DUMMY_ORIGIN) - // Avoid origin change by extra slashes in pathname - if (parsed.origin !== DUMMY_ORIGIN) { - throw new Error(INVALID_URL_MESSAGE) - } - return parsed - } catch { - throw new Error(INVALID_URL_MESSAGE) - } -} diff --git a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts index 2f11e7cf85097..c5a0e050965d1 100644 --- a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts +++ b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts @@ -8,7 +8,11 @@ import { createPrerenderState } from '../../server/app-render/dynamic-rendering' import type { FetchMetric } from '../base-http' export type StaticGenerationContext = { - urlPathname: string + /** + * The URL of the request. This only specifies the pathname and the search + * part of the URL. The other parts aren't accepted so they shouldn't be used. + */ + url: { pathname: string; search?: string } renderOpts: { incrementalCache?: IncrementalCache isOnDemandRevalidate?: boolean @@ -50,7 +54,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< > = { wrap( storage: AsyncLocalStorage, - { urlPathname, renderOpts }: StaticGenerationContext, + { url, renderOpts }: StaticGenerationContext, callback: (store: StaticGenerationStore) => Result ): Result { /** @@ -82,7 +86,13 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< const store: StaticGenerationStore = { isStaticGeneration, - urlPathname, + // Rather than just using the whole `url` here, we pull the parts we want + // to ensure we don't use parts of the URL that we shouldn't. This also + // lets us avoid requiring an empty string for `search` in the type. + url: { + pathname: url.pathname, + search: url.search ?? '', + }, pagePath: renderOpts.originalPathname, incrementalCache: // we fallback to a global incremental cache for edge-runtime locally diff --git a/packages/next/src/server/future/route-modules/app-route/module.ts b/packages/next/src/server/future/route-modules/app-route/module.ts index 2edb287eda3a7..c1d0280afb650 100644 --- a/packages/next/src/server/future/route-modules/app-route/module.ts +++ b/packages/next/src/server/future/route-modules/app-route/module.ts @@ -256,7 +256,7 @@ export class AppRouteRouteModule extends RouteModule< // Get the context for the static generation. const staticGenerationContext: StaticGenerationContext = { - urlPathname: rawRequest.nextUrl.pathname, + url: rawRequest.nextUrl, renderOpts: context.renderOpts, } diff --git a/packages/next/src/server/lib/patch-fetch.ts b/packages/next/src/server/lib/patch-fetch.ts index 9cbc4e45d2e5c..32bcb641bea2a 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -129,7 +129,10 @@ const getDerivedTags = (pathname: string): string[] => { export function addImplicitTags(staticGenerationStore: StaticGenerationStore) { const newTags: string[] = [] - const { pagePath, urlPathname } = staticGenerationStore + const { + pagePath, + url: { pathname }, + } = staticGenerationStore if (!Array.isArray(staticGenerationStore.tags)) { staticGenerationStore.tags = [] @@ -147,10 +150,8 @@ export function addImplicitTags(staticGenerationStore: StaticGenerationStore) { } } - if (urlPathname) { - const parsedPathname = new URL(urlPathname, 'http://n').pathname - - const tag = `${NEXT_CACHE_IMPLICIT_TAG_ID}${parsedPathname}` + if (pathname) { + const tag = `${NEXT_CACHE_IMPLICIT_TAG_ID}${pathname}` if (!staticGenerationStore.tags?.includes(tag)) { staticGenerationStore.tags.push(tag) } @@ -298,7 +299,7 @@ function createPatchedFetcher( // we only want to warn if the user is explicitly setting a cache value if (!(isRequestInput && _cache === 'default')) { Log.warn( - `fetch for ${fetchUrl} on ${staticGenerationStore.urlPathname} specified "cache: ${_cache}" and "revalidate: ${curRevalidate}", only one should be specified.` + `fetch for ${fetchUrl} on ${staticGenerationStore.url.pathname} specified "cache: ${_cache}" and "revalidate: ${curRevalidate}", only one should be specified.` ) } _cache = undefined @@ -321,7 +322,7 @@ function createPatchedFetcher( revalidate = validateRevalidate( curRevalidate, - staticGenerationStore.urlPathname + staticGenerationStore.url.pathname ) const _headers = getRequestMeta('headers') @@ -647,8 +648,8 @@ function createPatchedFetcher( if (!staticGenerationStore.forceStatic && cache === 'no-store') { const dynamicUsageReason = `no-store fetch ${input}${ - staticGenerationStore.urlPathname - ? ` ${staticGenerationStore.urlPathname}` + staticGenerationStore.url.pathname + ? ` ${staticGenerationStore.url.pathname}` : '' }` @@ -678,8 +679,8 @@ function createPatchedFetcher( next.revalidate === 0 ) { const dynamicUsageReason = `revalidate: 0 fetch ${input}${ - staticGenerationStore.urlPathname - ? ` ${staticGenerationStore.urlPathname}` + staticGenerationStore.url.pathname + ? ` ${staticGenerationStore.url.pathname}` : '' }` diff --git a/packages/next/src/server/web/spec-extension/revalidate.ts b/packages/next/src/server/web/spec-extension/revalidate.ts index ef10e466fe07f..9c192f4e35bf2 100644 --- a/packages/next/src/server/web/spec-extension/revalidate.ts +++ b/packages/next/src/server/web/spec-extension/revalidate.ts @@ -4,7 +4,6 @@ import { NEXT_CACHE_IMPLICIT_TAG_ID, NEXT_CACHE_SOFT_TAG_MAX_LENGTH, } from '../../../lib/constants' -import { getPathname } from '../../../lib/url' import { staticGenerationAsyncStorage } from '../../../client/components/static-generation-async-storage.external' /** @@ -51,9 +50,7 @@ function revalidate(tag: string, expression: string) { if (store.isUnstableCacheCallback) { throw new Error( - `Route ${getPathname( - store.urlPathname - )} used "${expression}" inside a function cached with "unstable_cache(...)" which is unsupported. To ensure revalidation is performed consistently it must always happen outside of renders and cached functions. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` + `Route ${store.url.pathname} used "${expression}" inside a function cached with "unstable_cache(...)" which is unsupported. To ensure revalidation is performed consistently it must always happen outside of renders and cached functions. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` ) } diff --git a/packages/next/src/server/web/spec-extension/unstable-cache.ts b/packages/next/src/server/web/spec-extension/unstable-cache.ts index ca98abeb11964..a880d1117cf57 100644 --- a/packages/next/src/server/web/spec-extension/unstable-cache.ts +++ b/packages/next/src/server/web/spec-extension/unstable-cache.ts @@ -302,7 +302,7 @@ export function unstable_cache( // when the unstable_cache call is revalidated fetchCache: 'force-no-store', isUnstableCacheCallback: true, - urlPathname: '/', + url: { pathname: '/', search: '' }, isStaticGeneration: false, prerenderState: null, }, diff --git a/packages/next/src/shared/lib/router/utils/parse-relative-url.test.ts b/packages/next/src/shared/lib/router/utils/parse-relative-url.test.ts new file mode 100644 index 0000000000000..8c71acfda0b83 --- /dev/null +++ b/packages/next/src/shared/lib/router/utils/parse-relative-url.test.ts @@ -0,0 +1,20 @@ +import { parseRelativeUrl } from './parse-relative-url' + +describe('relative urls', () => { + it('should return valid pathname', () => { + expect(parseRelativeUrl('/').pathname).toBe('/') + expect(parseRelativeUrl('/abc').pathname).toBe('/abc') + }) + + it('should throw for invalid pathname', () => { + expect(() => parseRelativeUrl('//**y/\\')).toThrow() + expect(() => parseRelativeUrl('//google.com')).toThrow() + }) +}) + +describe('query parsing', () => { + it('should parse query string', () => { + expect(parseRelativeUrl('/?a=1&b=2').query).toEqual({ a: '1', b: '2' }) + expect(parseRelativeUrl('/').query).toEqual({}) + }) +}) diff --git a/packages/next/src/shared/lib/router/utils/parse-relative-url.ts b/packages/next/src/shared/lib/router/utils/parse-relative-url.ts index eb3ccedf6d1f2..bb1b757225578 100644 --- a/packages/next/src/shared/lib/router/utils/parse-relative-url.ts +++ b/packages/next/src/shared/lib/router/utils/parse-relative-url.ts @@ -18,8 +18,19 @@ export interface ParsedRelativeUrl { */ export function parseRelativeUrl( url: string, - base?: string -): ParsedRelativeUrl { + base?: string, + parseQuery?: true +): ParsedRelativeUrl +export function parseRelativeUrl( + url: string, + base: string | undefined, + parseQuery: false +): Omit +export function parseRelativeUrl( + url: string, + base?: string, + parseQuery = true +): ParsedRelativeUrl | Omit { const globalBase = new URL( typeof window === 'undefined' ? 'http://n' : getLocationOrigin() ) @@ -34,14 +45,16 @@ export function parseRelativeUrl( url, resolvedBase ) + if (origin !== globalBase.origin) { throw new Error(`invariant: invalid relative URL, router received ${url}`) } + return { pathname, - query: searchParamsToUrlQuery(searchParams), + query: parseQuery ? searchParamsToUrlQuery(searchParams) : undefined, search, hash, - href: href.slice(globalBase.origin.length), + href: href.slice(origin.length), } } diff --git a/packages/next/src/shared/lib/router/utils/resolve-rewrites.ts b/packages/next/src/shared/lib/router/utils/resolve-rewrites.ts index c886a718d6004..4c1bb545147b6 100644 --- a/packages/next/src/shared/lib/router/utils/resolve-rewrites.ts +++ b/packages/next/src/shared/lib/router/utils/resolve-rewrites.ts @@ -5,7 +5,7 @@ import { matchHas, prepareDestination } from './prepare-destination' import { removeTrailingSlash } from './remove-trailing-slash' import { normalizeLocalePath } from '../../i18n/normalize-locale-path' import { removeBasePath } from '../../../../client/remove-base-path' -import { parseRelativeUrl } from './parse-relative-url' +import { parseRelativeUrl, type ParsedRelativeUrl } from './parse-relative-url' export default function resolveRewrites( asPath: string, @@ -20,7 +20,7 @@ export default function resolveRewrites( locales?: string[] ): { matchedPage: boolean - parsedAs: ReturnType + parsedAs: ParsedRelativeUrl asPath: string resolvedHref?: string externalDest?: boolean