diff --git a/.changeset/fluffy-lamps-confess.md b/.changeset/fluffy-lamps-confess.md new file mode 100644 index 00000000..8880b1d7 --- /dev/null +++ b/.changeset/fluffy-lamps-confess.md @@ -0,0 +1,5 @@ +--- +"open-next": patch +--- + +Fix next rewrites diff --git a/examples/pages-router/next.config.js b/examples/pages-router/next.config.js index 79370ecb..66c213a2 100644 --- a/examples/pages-router/next.config.js +++ b/examples/pages-router/next.config.js @@ -5,7 +5,20 @@ const nextConfig = { reactStrictMode: true, output: "standalone", outputFileTracing: "../sst", - rewrites: () => [{ source: "/rewrite", destination: "/" }], + rewrites: () => [ + { source: "/rewrite", destination: "/" }, + { + source: "/rewriteUsingQuery", + destination: "/:destination/", + has: [ + { + type: "query", + key: "d", + value: "(?\\w+)", + }, + ], + }, + ], trailingSlash: true, }; diff --git a/packages/open-next/src/core/routing/matcher.ts b/packages/open-next/src/core/routing/matcher.ts index 20073a37..50e8aba5 100644 --- a/packages/open-next/src/core/routing/matcher.ts +++ b/packages/open-next/src/core/routing/matcher.ts @@ -1,5 +1,11 @@ import { NextConfig } from "config/index"; -import { compile, Match, match, PathFunction } from "path-to-regexp"; +import { + compile, + Match, + match, + MatchFunction, + PathFunction, +} from "path-to-regexp"; import type { Header, PrerenderManifest, @@ -11,6 +17,7 @@ import { InternalEvent, InternalResult } from "types/open-next"; import { debug } from "../../adapters/logger"; import { + convertFromQueryString, convertToQueryString, escapeRegex, getUrlParts, @@ -71,6 +78,40 @@ function checkHas( : true; } +const getParamsFromSource = + (source: MatchFunction) => (value: string) => { + debug("value", value); + const _match = source(value); + return _match ? _match.params : {}; + }; + +const computeParamHas = + ( + headers: Record, + cookies: Record, + query: Record, + ) => + (has: RouteHas): object => { + if (!has.value) return {}; + const matcher = new RegExp(`^${has.value}$`); + const fromSource = (value: string) => { + const matches = value.match(matcher); + return matches?.groups ?? {}; + }; + switch (has.type) { + case "header": + return fromSource(headers[has.key.toLowerCase()] ?? ""); + case "cookie": + return fromSource(cookies[has.key] ?? ""); + case "query": + return Array.isArray(query[has.key]) + ? fromSource((query[has.key] as string[]).join(",")) + : fromSource((query[has.key] as string) ?? ""); + case "host": + return fromSource(headers.host ?? ""); + } + }; + function convertMatch( match: Match, toDestination: PathFunction, @@ -118,14 +159,6 @@ export function addNextConfigHeaders( debug("Error matching header ", h.key, " with value ", h.value); requestHeaders[h.key] = h.value; } - try { - const key = convertMatch(_match, compile(h.key), h.key); - const value = convertMatch(_match, compile(h.value), h.value); - requestHeaders[key] = value; - } catch { - debug("Error matching header ", h.key, " with value ", h.value); - requestHeaders[h.key] = h.value; - } }); } } @@ -138,44 +171,66 @@ export function handleRewrites( ) { const { rawPath, headers, query, cookies } = event; const matcher = routeHasMatcher(headers, cookies, query); + const computeHas = computeParamHas(headers, cookies, query); const rewrite = rewrites.find( (route) => new RegExp(route.regex).test(rawPath) && checkHas(matcher, route.has) && checkHas(matcher, route.missing, true), ); + let finalQuery = query; let rewrittenUrl = rawPath; const isExternalRewrite = isExternal(rewrite?.destination); debug("isExternalRewrite", isExternalRewrite); if (rewrite) { - const { pathname, protocol, hostname } = getUrlParts( + const { pathname, protocol, hostname, queryString } = getUrlParts( rewrite.destination, isExternalRewrite, ); - const toDestination = compile(escapeRegex(pathname ?? "") ?? ""); - const fromSource = match(escapeRegex(rewrite?.source) ?? ""); - const _match = fromSource(rawPath); - if (_match) { - const { params } = _match; - const isUsingParams = Object.keys(params).length > 0; - if (isUsingParams) { - const rewrittenPath = unescapeRegex(toDestination(params)); - rewrittenUrl = isExternalRewrite - ? `${protocol}//${hostname}${rewrittenPath}` - : `${rewrittenPath}`; - } else { - rewrittenUrl = rewrite.destination; - } - debug("rewrittenUrl", rewrittenUrl); + debug("urlParts", { pathname, protocol, hostname, queryString }); + const toDestinationPath = compile(escapeRegex(pathname ?? "") ?? ""); + const toDestinationHost = compile(escapeRegex(hostname ?? "") ?? ""); + const toDestinationQuery = compile(escapeRegex(queryString ?? "") ?? ""); + let params = { + // params for the source + ...getParamsFromSource(match(escapeRegex(rewrite?.source) ?? ""))( + rawPath, + ), + // params for the has + ...rewrite.has?.reduce((acc, cur) => { + return { ...acc, ...computeHas(cur) }; + }, {}), + // params for the missing + ...rewrite.missing?.reduce((acc, cur) => { + return { ...acc, ...computeHas(cur) }; + }, {}), + }; + const isUsingParams = Object.keys(params).length > 0; + let rewrittenQuery = queryString; + let rewrittenHost = hostname; + let rewrittenPath = pathname; + if (isUsingParams) { + rewrittenPath = unescapeRegex(toDestinationPath(params)); + rewrittenHost = unescapeRegex(toDestinationHost(params)); + rewrittenQuery = unescapeRegex(toDestinationQuery(params)); } + rewrittenUrl = isExternalRewrite + ? `${protocol}//${rewrittenHost}${rewrittenPath}` + : `/${rewrittenPath}`; + // Should we merge the query params or use only the ones from the rewrite? + finalQuery = { + ...query, + ...convertFromQueryString(rewrittenQuery), + }; + debug("rewrittenUrl", { rewrittenUrl, finalQuery, isUsingParams }); } return { internalEvent: { ...event, rawPath: rewrittenUrl, - url: `${rewrittenUrl}${convertToQueryString(query)}`, + url: `${rewrittenUrl}${convertToQueryString(finalQuery)}`, }, __rewrite: rewrite, isExternalRewrite, diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts index b978731b..616e70a0 100644 --- a/packages/open-next/src/core/routing/util.ts +++ b/packages/open-next/src/core/routing/util.ts @@ -24,28 +24,44 @@ export function isExternal(url?: string, host?: string) { return pattern.test(url); } +export function convertFromQueryString(query: string) { + if (query === "") return {}; + const queryParts = query.split("&"); + return queryParts.reduce( + (acc, part) => { + const [key, value] = part.split("="); + return { ...acc, [key]: value }; + }, + {} as Record, + ); +} + /** * * @__PURE__ */ export function getUrlParts(url: string, isExternal: boolean) { - // NOTE: when redirect to a URL that contains search query params, - // compile breaks b/c it does not allow for the '?' character - // We can't use encodeURIComponent because modal interception contains - // characters that can't be encoded - url = url.replaceAll("?", "%3F"); if (!isExternal) { + const regex = /\/([^?]*)\??(.*)/; + const match = url.match(regex); return { hostname: "", - pathname: url, + pathname: match?.[1] ?? url, protocol: "", + queryString: match?.[2] ?? "", }; } - const { hostname, pathname, protocol } = new URL(url); + + const regex = /^(https?:\/\/)?([^\/\s]+)(\/[^?]*)?(\?.*)?/; + const match = url.match(regex); + if (!match) { + throw new Error(`Invalid external URL: ${url}`); + } return { - hostname, - pathname, - protocol, + protocol: match[1] ?? "https://", + hostname: match[2], + pathname: match[3], + queryString: match[4]?.slice(1) ?? "", }; } diff --git a/packages/tests-e2e/tests/pagesRouter/rewrite.test.ts b/packages/tests-e2e/tests/pagesRouter/rewrite.test.ts index ba0c0b7c..ad0b6fa2 100644 --- a/packages/tests-e2e/tests/pagesRouter/rewrite.test.ts +++ b/packages/tests-e2e/tests/pagesRouter/rewrite.test.ts @@ -6,3 +6,10 @@ test("Single Rewrite", async ({ page }) => { let el = page.getByText("Nextjs Pages Router"); await expect(el).toBeVisible(); }); + +test("Rewrite with query", async ({ page }) => { + await page.goto("/rewriteUsingQuery?d=ssr"); + + let el = page.getByText("SSR"); + await expect(el).toBeVisible(); +});