diff --git a/.changeset/khaki-dragons-build.md b/.changeset/khaki-dragons-build.md new file mode 100644 index 000000000..8a2dad341 --- /dev/null +++ b/.changeset/khaki-dragons-build.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Fix middleware search params with multiple values diff --git a/examples/app-pages-router/app/rewrite-destination/page.tsx b/examples/app-pages-router/app/rewrite-destination/page.tsx index c87e4e646..d4d4c9ff6 100644 --- a/examples/app-pages-router/app/rewrite-destination/page.tsx +++ b/examples/app-pages-router/app/rewrite-destination/page.tsx @@ -1,11 +1,12 @@ export default async function RewriteDestination(props: { - searchParams: Promise<{ a: string }>; + searchParams: Promise<{ a: string; multi?: string[] }>; }) { const searchParams = await props.searchParams; return (
Rewritten Destination
a: {searchParams.a}
+
multi: {searchParams.multi?.join(", ")}
); } diff --git a/examples/app-pages-router/middleware.ts b/examples/app-pages-router/middleware.ts index 4905be8c0..edc8677a5 100644 --- a/examples/app-pages-router/middleware.ts +++ b/examples/app-pages-router/middleware.ts @@ -27,6 +27,14 @@ export function middleware(request: NextRequest) { u.searchParams.set("a", "b"); return NextResponse.rewrite(u); } + if (path === "/rewrite-multi-params") { + const u = new URL("/rewrite-destination", `${protocol}://${host}`); + u.searchParams.append("multi", "0"); + u.searchParams.append("multi", "1"); + u.searchParams.append("multi", "2"); + u.searchParams.set("a", "b"); + return NextResponse.rewrite(u); + } if (path === "/api/middleware") { return new NextResponse(JSON.stringify({ hello: "middleware" }), { status: 200, diff --git a/packages/open-next/src/core/routing/middleware.ts b/packages/open-next/src/core/routing/middleware.ts index 717d7e2ec..cd4c47628 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -9,6 +9,7 @@ import { import type { InternalEvent, InternalResult } from "types/open-next.js"; import { emptyReadableStream } from "utils/stream.js"; +import { getQueryFromSearchParams } from "../../overrides/converters/utils.js"; import { localizePath } from "./i18n/index.js"; import { convertBodyToReadableStream, @@ -142,7 +143,7 @@ export async function handleMiddleware( // NOTE: the header was added to `req` from above const rewriteUrl = responseHeaders.get("x-middleware-rewrite"); let isExternalRewrite = false; - let middlewareQueryString = internalEvent.query; + let middlewareQuery = internalEvent.query; let newUrl = internalEvent.url; if (rewriteUrl) { newUrl = rewriteUrl; @@ -151,17 +152,14 @@ export async function handleMiddleware( isExternalRewrite = true; } else { const rewriteUrlObject = new URL(rewriteUrl); + // Search params from the rewritten URL override the original search params - // Reset the query params if the middleware is a rewrite - middlewareQueryString = middlewareQueryString.__nextDataReq - ? { - __nextDataReq: middlewareQueryString.__nextDataReq, - } - : {}; + middlewareQuery = getQueryFromSearchParams(rewriteUrlObject.searchParams); - rewriteUrlObject.searchParams.forEach((v: string, k: string) => { - middlewareQueryString[k] = v; - }); + // We still need to add internal search params to the query string for pages router on older versions of Next.js + if ("__nextDataReq" in internalEvent.query) { + middlewareQuery.__nextDataReq = internalEvent.query.__nextDataReq; + } } } @@ -188,7 +186,7 @@ export async function handleMiddleware( headers: { ...internalEvent.headers, ...reqHeaders }, body: internalEvent.body, method: internalEvent.method, - query: middlewareQueryString, + query: middlewareQuery, cookies: internalEvent.cookies, remoteAddress: internalEvent.remoteAddress, isExternalRewrite, diff --git a/packages/tests-e2e/tests/appPagesRouter/middleware.rewrite.test.ts b/packages/tests-e2e/tests/appPagesRouter/middleware.rewrite.test.ts index ba09abd6b..1cec3e326 100644 --- a/packages/tests-e2e/tests/appPagesRouter/middleware.rewrite.test.ts +++ b/packages/tests-e2e/tests/appPagesRouter/middleware.rewrite.test.ts @@ -1,19 +1,44 @@ import { expect, test } from "@playwright/test"; -test("Middleware Rewrite", async ({ page }) => { - await page.goto("/"); - await page.locator('[href="/rewrite"]').click(); +test.describe("Middleware Rewrite", () => { + test("Simple Middleware Rewrite", async ({ page }) => { + await page.goto("/"); + await page.locator('[href="/rewrite"]').click(); - await page.waitForURL("/rewrite"); - let el = page.getByText("Rewritten Destination", { exact: true }); - await expect(el).toBeVisible(); - el = page.getByText("a: b", { exact: true }); - await expect(el).toBeVisible(); - // Loading page should also rewrite - await page.goto("/rewrite"); - await page.waitForURL("/rewrite"); - el = page.getByText("Rewritten Destination", { exact: true }); - await expect(el).toBeVisible(); - el = page.getByText("a: b", { exact: true }); - await expect(el).toBeVisible(); + await page.waitForURL("/rewrite"); + let el = page.getByText("Rewritten Destination", { exact: true }); + await expect(el).toBeVisible(); + el = page.getByText("a: b", { exact: true }); + await expect(el).toBeVisible(); + // Loading page should also rewrite + await page.goto("/rewrite"); + await page.waitForURL("/rewrite"); + el = page.getByText("Rewritten Destination", { exact: true }); + await expect(el).toBeVisible(); + el = page.getByText("a: b", { exact: true }); + await expect(el).toBeVisible(); + }); + + test("Middleware Rewrite with multiple search params", async ({ page }) => { + await page.goto("/rewrite-multi-params"); + let el = page.getByText("Rewritten Destination", { exact: true }); + await expect(el).toBeVisible(); + el = page.getByText("a: b", { exact: true }); + await expect(el).toBeVisible(); + el = page.getByText("multi: 0, 1, 2", { exact: true }); + await expect(el).toBeVisible(); + }); + + test("Middleware Rewrite should override original search params", async ({ + page, + }) => { + await page.goto("/rewrite?a=1&multi=3"); + let el = page.getByText("Rewritten Destination", { exact: true }); + await expect(el).toBeVisible(); + el = page.getByText("a: b", { exact: true }); + await expect(el).toBeVisible(); + el = page.getByText("multi:", { exact: true }); + await expect(el).toBeVisible(); + expect(el).toHaveText("multi:"); + }); });