8000 Fix middleware search params by conico974 · Pull Request #844 · opennextjs/opennextjs-aws · GitHub
[go: up one dir, main page]
Skip to content

Fix middleware search params #844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 30, 2025
Merged

Fix middleware search params #844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
646ba58
fix middleware search params with multiple value
conico974 Apr 30, 2025
4a53331
changeset and lint
conico974 Apr 30, 2025
026730d
review fix
conico974 Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to loa 8000 d files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/khaki-dragons-build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@opennextjs/aws": patch
---

Fix middleware search params with multiple values
3 changes: 2 additions & 1 deletion examples/app-pages-router/app/rewrite-destination/page.tsx
8000
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>
);
}
8 changes: 8 additions & 0 deletions examples/app-pages-router/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 9 additions & 11 deletions packages/open-next/src/core/routing/middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
}

Expand All @@ -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,
Expand Down
55 changes: 40 additions & 15 deletions packages/tests-e2e/tests/appPagesRouter/middleware.rewrite.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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:");
});
});
Loading
0