Skip to content

Commit 1c23954

Browse files
authored
fix: Ensure Location header is properly encoded in redirects (#871)
* fix: ensure Location header is encoded in redirects * e2e * changeset * refactor comment * refactor changeset
1 parent ad701c2 commit 1c23954

File tree

6 files changed

+82
-7
lines changed

6 files changed

+82
-7
lines changed

.changeset/proud-dogs-punch.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@opennextjs/aws": patch
3+
---
4+
5+
fix: Ensure Location header is properly encoded in redirects happening from next config
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export default async function Page({
2+
searchParams,
3+
}: {
4+
searchParams: Promise<{ [key: string]: string | string[] | undefined }>;
5+
}) {
6+
const q = (await searchParams).q;
7+
8+
return (
9+
<>
10+
<div data-testid="searchParams">q: {q}</div>
11+
</>
12+
);
13+
}

examples/app-router/app/config-redirect/page.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,18 @@ export default function RedirectDestination() {
33
<div>
44
<h1>I was redirected from next.config.js</h1>
55
<p>/next-config-redirect =&gt; /config-redirect</p>
6+
<a
7+
data-testid="redirect-link"
8+
href="/next-config-redirect-encoding?q=äöå€"
9+
>
10+
/next-config-redirect-encoding?q=äöå€
11+
</a>
12+
<a
13+
data-testid="redirect-link-already-encoded"
14+
href="/next-config-redirect-encoding?q=%C3%A4%C3%B6%C3%A5%E2%82%AC"
15+
>
16+
/next-config-redirect-encoding?q=%C3%A4%C3%B6%C3%A5%E2%82%AC
17+
</a>
618
</div>
719
);
820
}

examples/app-router/next.config.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,6 @@ const nextConfig: NextConfig = {
66
transpilePackages: ["@example/shared"],
77
output: "standalone",
88
// outputFileTracingRoot: "../sst",
9-
eslint: {
10-
ignoreDuringBuilds: true,
11-
},
12-
//TODO: remove this when i'll figure out why it fails locally
13-
typescript: {
14-
ignoreBuildErrors: true,
15-
},
169
images: {
1710
remotePatterns: [
1811
{
@@ -60,6 +53,11 @@ const nextConfig: NextConfig = {
6053
basePath: false,
6154
locale: false,
6255
},
56+
{
57+
source: "/next-config-redirect-encoding",
58+
destination: "/config-redirect/dest",
59+
permanent: false,
60+
},
6361
];
6462
},
6563
async headers() {

packages/open-next/src/core/routingHandler.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ export default async function routingHandler(
9696

9797
const redirect = handleRedirects(internalEvent, RoutesManifest.redirects);
9898
if (redirect) {
99+
// We need to encode the value in the Location header to make sure it is valid according to RFC
100+
// https://stackoverflow.com/a/7654605/16587222
101+
redirect.headers.Location = new URL(
102+
redirect.headers.Location as string,
103+
).href;
99104
debug("redirect", redirect);
100105
return redirect;
101106
}

packages/tests-e2e/tests/appRouter/config.redirect.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,46 @@ test.describe("Next Config Redirect", () => {
7272
});
7373
await expect(el).toBeVisible();
7474
});
75+
test("Should properly encode the Location header for redirects with query params", async ({
76+
page,
77+
baseURL,
78+
}) => {
79+
await page.goto("/config-redirect");
80+
const responsePromise = page.waitForResponse((response) => {
81+
return response.status() === 307;
82+
});
83+
page.getByTestId("redirect-link").click();
84+
const res = await responsePromise;
85+
await page.waitForURL("/config-redirect/dest?q=äöå€");
86+
87+
const locationHeader = res.headers().location;
88+
expect(locationHeader).toBe(
89+
`${baseURL}/config-redirect/dest?q=%C3%A4%C3%B6%C3%A5%E2%82%AC`,
90+
);
91+
expect(res.status()).toBe(307);
92+
93+
const searchParams = page.getByTestId("searchParams");
94+
await expect(searchParams).toHaveText("q: äöå€");
95+
});
96+
test("Should respect already encoded query params", async ({
97+
page,
98+
baseURL,
99+
}) => {
100+
await page.goto("/config-redirect");
101+
const responsePromise = page.waitForResponse((response) => {
102+
return response.status() === 307;
103+
});
104+
page.getByTestId("redirect-link-already-encoded").click();
105+
const res = await responsePromise;
106+
await page.waitForURL("/config-redirect/dest?q=äöå€");
107+
108+
const locationHeader = res.headers().location;
109+
expect(locationHeader).toBe(
110+
`${baseURL}/config-redirect/dest?q=%C3%A4%C3%B6%C3%A5%E2%82%AC`,
111+
);
112+
expect(res.status()).toBe(307);
113+
114+
const searchParams = page.getByTestId("searchParams");
115+
await expect(searchParams).toHaveText("q: äöå€");
116+
});
75117
});

0 commit comments

Comments
 (0)