Fix middleware search params (#844)

* fix middleware search params with multiple value

* changeset and lint

* review fix

Author: conico974

.changeset/khaki-dragons-build.md

@@ -0,0 +1,5 @@
+---
+"@opennextjs/aws": patch
+---
+
+Fix middleware search params with multiple values

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 (
     <div>
       <div>Rewritten Destination</div>
       <div>a: {searchParams.a}</div>
+      <div>multi: {searchParams.multi?.join(", ")}</div>
     </div>
   );
 }

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,

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,

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:");
+  });
 });