From 8c511d476069c569ab80210e27d4b7fc4d05134e 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 | 16 ++++++++++-- .../src/shared/lib/router/utils/parse-url.ts | 11 +++++++- 17 files changed, 99 insertions(+), 85 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 6e1c9c06bc6bf6..9e260ccfb1f604 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 4433992a8f331f..8b4a7d0ae6fb9f 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 1be2eb2e207177..3f1acb71cf3aa0 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 32aea560824901..4f812328a2798f 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 0ca1e501a4a185..f1521045d57839 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) 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 0ea17745987dc7..5da5ddc9fc57aa 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 148d095309f356..995cf4997a1fa8 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 4415965c8fb740..00000000000000 --- 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 723442ec4f9fb2..00000000000000 --- 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 2f11e7cf850973..c5a0e050965d18 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 2edb287eda3a71..c1d0280afb6504 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 bba75df1ca594c..b59d4e0469ee46 100644 --- a/packages/next/src/server/lib/patch-fetch.ts +++ b/packages/next/src/server/lib/patch-fetch.ts @@ -115,7 +115,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 = [] @@ -133,10 +136,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) } @@ -292,7 +293,7 @@ export function patchFetch({ // 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 @@ -315,7 +316,7 @@ export function patchFetch({ revalidate = validateRevalidate( curRevalidate, - staticGenerationStore.urlPathname + staticGenerationStore.url.pathname ) const _headers = getRequestMeta('headers') @@ -641,8 +642,8 @@ export function patchFetch({ if (!staticGenerationStore.forceStatic && cache === 'no-store') { const dynamicUsageReason = `no-store fetch ${input}${ - staticGenerationStore.urlPathname - ? ` ${staticGenerationStore.urlPathname}` + staticGenerationStore.url.pathname + ? ` ${staticGenerationStore.url.pathname}` : '' }` @@ -672,8 +673,8 @@ export function patchFetch({ 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 600c3fd495b7be..4a48dd847accdb 100644 --- a/packages/next/src/server/web/spec-extension/revalidate.ts +++ b/packages/next/src/server/web/spec-extension/revalidate.ts @@ -8,7 +8,6 @@ import { NEXT_CACHE_IMPLICIT_TAG_ID, NEXT_CACHE_SOFT_TAG_MAX_LENGTH, } from '../../../lib/constants' -import { getPathname } from '../../../lib/url' /** * This function allows you to purge [cached data](https://nextjs.org/docs/app/building-your-application/caching) on-demand for a specific cache tag. @@ -60,9 +59,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 66c887b23a64f2..5ac54c2cba3a86 100644 --- a/packages/next/src/server/web/spec-extension/unstable-cache.ts +++ b/packages/next/src/server/web/spec-extension/unstable-cache.ts @@ -308,7 +308,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 00000000000000..8c71acfda0b837 --- /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 eb3ccedf6d1f2f..438a4f5ab252c3 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 @@ -34,14 +34,26 @@ export function parseRelativeUrl( url, resolvedBase ) + if (origin !== globalBase.origin) { throw new Error(`invariant: invalid relative URL, router received ${url}`) } + + let query: ParsedUrlQuery | undefined + return { pathname, - query: searchParamsToUrlQuery(searchParams), + // Lazily transform the search parameters into a query object, caching the + // result. + get query() { + if (!query) { + query = searchParamsToUrlQuery(searchParams) + } + + return query + }, search, hash, - href: href.slice(globalBase.origin.length), + href: href.slice(origin.length), } } diff --git a/packages/next/src/shared/lib/router/utils/parse-url.ts b/packages/next/src/shared/lib/router/utils/parse-url.ts index 67a78286b221b0..82ec8dd3b5c7d4 100644 --- a/packages/next/src/shared/lib/router/utils/parse-url.ts +++ b/packages/next/src/shared/lib/router/utils/parse-url.ts @@ -20,6 +20,9 @@ export function parseUrl(url: string): ParsedUrl { } const parsedURL = new URL(url) + + let query: ParsedUrlQuery | undefined + return { hash: parsedURL.hash, hostname: parsedURL.hostname, @@ -27,7 +30,13 @@ export function parseUrl(url: string): ParsedUrl { pathname: parsedURL.pathname, port: parsedURL.port, protocol: parsedURL.protocol, - query: searchParamsToUrlQuery(parsedURL.searchParams), + get query() { + if (!query) { + query = searchParamsToUrlQuery(parsedURL.searchParams) + } + + return query + }, search: parsedURL.search, } }