From 646ba5824351d70482afd943a866e988080f42d1 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Wed, 30 Apr 2025 11:30:05 +0200 Subject: [PATCH 1/3] fix middleware search params with multiple value --- .../app/rewrite-destination/page.tsx | 3 +- examples/app-pages-router/middleware.ts | 8 +++ .../open-next/src/core/routing/middleware.ts | 18 +++--- .../appPagesRouter/middleware.rewrite.test.ts | 55 ++++++++++++++----- 4 files changed, 59 insertions(+), 25 deletions(-) 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..927dd65d6 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -15,6 +15,7 @@ import { getMiddlewareMatch, isExternal, } from "./util.js"; +import { getQueryFromSearchParams } from "../../overrides/converters/utils.js"; const middlewareManifest = MiddlewareManifest; const functionsConfigManifest = FunctionsConfigManifest; @@ -151,17 +152,16 @@ export async function handleMiddleware( isExternalRewrite = true; } else { const rewriteUrlObject = new URL(rewriteUrl); + // If we have a rewrite, search params from the rewrite URL should be used and override the original search params - // Reset the query params if the middleware is a rewrite - middlewareQueryString = middlewareQueryString.__nextDataReq - ? { - __nextDataReq: middlewareQueryString.__nextDataReq, - } - : {}; + middlewareQueryString = 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) { + middlewareQueryString.__nextDataReq = internalEvent.query.__nextDataReq; + } } } 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:"); + }); }); From 4a53331b3defd2cbbaabd21f1cf039c4b8322ade Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Wed, 30 Apr 2025 11:31:20 +0200 Subject: [PATCH 2/3] changeset and lint --- .changeset/khaki-dragons-build.md | 5 +++++ packages/open-next/src/core/routing/middleware.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/khaki-dragons-build.md diff --git a/.changeset/khaki-dragons-build.md b/.changeset/khaki-dragons-build.md new file mode 100644 index 000000000..35c114935 --- /dev/null +++ b/.changeset/khaki-dragons-build.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Fix middleware search params with multiple value diff --git a/packages/open-next/src/core/routing/middleware.ts b/packages/open-next/src/core/routing/middleware.ts index 927dd65d6..bed7245d7 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -9,13 +9,13 @@ 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, getMiddlewareMatch, isExternal, } from "./util.js"; -import { getQueryFromSearchParams } from "../../overrides/converters/utils.js"; const middlewareManifest = MiddlewareManifest; const functionsConfigManifest = FunctionsConfigManifest; From 026730d0a89dc2708c70a72e407f489033f95d1e Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Wed, 30 Apr 2025 11:46:59 +0200 Subject: [PATCH 3/3] review fix --- .changeset/khaki-dragons-build.md | 2 +- packages/open-next/src/core/routing/middleware.ts | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.changeset/khaki-dragons-build.md b/.changeset/khaki-dragons-build.md index 35c114935..8a2dad341 100644 --- a/.changeset/khaki-dragons-build.md +++ b/.changeset/khaki-dragons-build.md @@ -2,4 +2,4 @@ "@opennextjs/aws": patch --- -Fix middleware search params with multiple value +Fix middleware search params with multiple values diff --git a/packages/open-next/src/core/routing/middleware.ts b/packages/open-next/src/core/routing/middleware.ts index bed7245d7..cd4c47628 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -143,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; @@ -152,15 +152,13 @@ export async function handleMiddleware( isExternalRewrite = true; } else { const rewriteUrlObject = new URL(rewriteUrl); - // If we have a rewrite, search params from the rewrite URL should be used and override the original search params + // Search params from the rewritten URL override the original search params - middlewareQueryString = getQueryFromSearchParams( - rewriteUrlObject.searchParams, - ); + middlewareQuery = getQueryFromSearchParams(rewriteUrlObject.searchParams); // 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) { - middlewareQueryString.__nextDataReq = internalEvent.query.__nextDataReq; + 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,