Skip to content

Commit ada2e4b

Browse files
sommeeeersommeeeer
committed
add normalizeLocationHeader
add comment in middleware check status code fix e2e review add unit test for normalizeLocationHeader refactor to match Next behavior update unit test refactor comment refactor changeset
1 parent f0ffff5 commit ada2e4b

File tree

6 files changed

+133
-12
lines changed

6 files changed

+133
-12
lines changed

‎.changeset/fluffy-dingos-tap.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@opennextjs/aws": patch
3+
---
4+
5+
fix: Normalize the Location header in redirects
6+
7+
Normalizes the Location header to either be a relative path or a full URL.
8+
If the Location header is relative to the host, it will return a relative path.
9+
If it is an absolute URL, it will return the full URL.
10+
We want to match the behavior of `next start`.
11+
Query parameters in redirects from Next Config are encoded, but from the middleware they are not touched.

‎packages/open-next/src/core/routing/middleware.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
convertBodyToReadableStream,
1616
getMiddlewareMatch,
1717
isExternal,
18+
normalizeLocationHeader,
1819
} from "./util.js";
1920

2021
const middlewareManifest = MiddlewareManifest;
@@ -25,6 +26,8 @@ const middleMatch = getMiddlewareMatch(
2526
functionsConfigManifest,
2627
);
2728

29+
const REDIRECTS = new Set([301, 302, 303, 307, 308]);
30+
2831
type MiddlewareEvent = InternalEvent & {
2932
responseHeaders?: Record<string, string | string[]>;
3033
isExternalRewrite?: boolean;
@@ -132,6 +135,11 @@ export async function handleMiddleware(
132135
resHeaders[key] = resHeaders[key]
133136
? [...resHeaders[key], value]
134137
: [value];
138+
} else if (
139+
REDIRECTS.has(statusCode) &&
140+
key.toLowerCase() === "location"
141+
) {
142+
resHeaders[key] = normalizeLocationHeader(value, internalEvent.url);
135143
} else {
136144
resHeaders[key] = value;
137145
}

‎packages/open-next/src/core/routing/util.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import crypto from "node:crypto";
22
import type { OutgoingHttpHeaders } from "node:http";
3+
import { parse as parseQs, stringify as stringifyQs } from "node:querystring";
34
import { Readable } from "node:stream";
45

56
import { BuildId, HtmlPages, NextConfig } from "config/index.js";
@@ -437,3 +438,44 @@ export async function invalidateCDNOnRequest(
437438
]);
438439
}
439440
}
441+
442+
/**
443+
* Normalizes the Location header to either be a relative path or a full URL.
444+
* If the Location header is relative to the origin, it will return a relative path.
445+
* If it is an absolute URL, it will return the full URL.
446+
* Redirects from Next config query parameters are encoded using `stringifyQs`
447+
* Redirects from the middleware the query parameters are not encoded.
448+
*
449+
* @param location The Location header value
450+
* @param baseUrl The base URL to use for relative paths (i.e the original request URL)
451+
* @param encodeQuery Optional flag to indicate if query parameters should be encoded in the Location header
452+
* @returns An absolute or relative Location header value
453+
*/
454+
export function normalizeLocationHeader(
455+
location: string,
456+
baseUrl: string,
457+
encodeQuery = false,
458+
): string {
459+
if (!URL.canParse(location)) {
460+
// If the location is not a valid URL, return it as-is
461+
return location;
462+
}
463+
464+
const locationURL = new URL(location);
465+
const origin = new URL(baseUrl).origin;
466+
467+
// Redirects from the middleware do not encode the query parameters
468+
let search = locationURL.search;
469+
// If encodeQuery is true, we need to encode the query parameters
470+
// This is used for redirects from Next config
471+
// We could have used URLSearchParams, but that doesn't match what Next does.
472+
if (encodeQuery) {
473+
search = search ? `?${stringifyQs(parseQs(search.slice(1)))}` : "";
474+
}
475+
const href = `${locationURL.origin}${locationURL.pathname}${search}${locationURL.hash}`;
476+
// The URL is relative if the origin is the same as the base URL's origin
477+
if (locationURL.origin === origin) {
478+
return href.slice(origin.length);
479+
}
480+
return href;
481+
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import {
2828
dynamicRouteMatcher,
2929
staticRouteMatcher,
3030
} from "./routing/routeMatcher";
31-
import { constructNextUrl } from "./routing/util";
31+
import { constructNextUrl, normalizeLocationHeader } from "./routing/util";
3232

3333
export const MIDDLEWARE_HEADER_PREFIX = "x-middleware-response-";
3434
export const MIDDLEWARE_HEADER_PREFIX_LEN = MIDDLEWARE_HEADER_PREFIX.length;
@@ -109,14 +109,14 @@ export default async function routingHandler(
109109
const redirect = handleRedirects(eventOrResult, RoutesManifest.redirects);
110110
if (redirect) {
111111
// We need to encode the value in the Location header to make sure it is valid according to RFC
112-
// https://stackoverflow.com/a/7654605/16587222
113-
redirect.headers.Location = new URL(
112+
redirect.headers.Location = normalizeLocationHeader(
114113
redirect.headers.Location as string,
115-
).href;
114+
event.url,
115+
true,
116+
);
116117
debug("redirect", redirect);
117118
return redirect;
118119
}
119-
120120
const middlewareEventOrResult = await handleMiddleware(
121121
eventOrResult,
122122
// We need to pass the initial search without any decoding

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ test.describe("Next Config Redirect", () => {
7474
});
7575
test("Should properly encode the Location header for redirects with query params", async ({
7676
page,
77-
baseURL,
7877
}) => {
7978
await page.goto("/config-redirect");
8079
const responsePromise = page.waitForResponse((response) => {
@@ -86,17 +85,14 @@ test.describe("Next Config Redirect", () => {
8685

8786
const locationHeader = res.headers().location;
8887
expect(locationHeader).toBe(
89-
`${baseURL}/config-redirect/dest?q=%C3%A4%C3%B6%C3%A5%E2%82%AC`,
88+
"/config-redirect/dest?q=%C3%A4%C3%B6%C3%A5%E2%82%AC",
9089
);
9190
expect(res.status()).toBe(307);
9291

9392
const searchParams = page.getByTestId("searchParams");
9493
await expect(searchParams).toHaveText("q: äöå€");
9594
});
96-
test("Should respect already encoded query params", async ({
97-
page,
98-
baseURL,
99-
}) => {
95+
test("Should respect already encoded query params", async ({ page }) => {
10096
await page.goto("/config-redirect");
10197
const responsePromise = page.waitForResponse((response) => {
10298
return response.status() === 307;
@@ -107,7 +103,7 @@ test.describe("Next Config Redirect", () => {
107103

108104
const locationHeader = res.headers().location;
109105
expect(locationHeader).toBe(
110-
`${baseURL}/config-redirect/dest?q=%C3%A4%C3%B6%C3%A5%E2%82%AC`,
106+
"/config-redirect/dest?q=%C3%A4%C3%B6%C3%A5%E2%82%AC",
111107
);
112108
expect(res.status()).toBe(307);
113109

‎packages/tests-unit/tests/core/routing/util.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
getUrlParts,
1818
invalidateCDNOnRequest,
1919
isExternal,
20+
normalizeLocationHeader,
2021
revalidateIfRequired,
2122
unescapeRegex,
2223
} from "@opennextjs/aws/core/routing/util.js";
@@ -876,3 +877,66 @@ describe("constructNextUrl", () => {
876877
expect(result).toBe("http://localhost/base/path");
877878
});
878879
});
880+
881+
describe("normalizeLocationHeader", () => {
882+
it("should normalize relative location header", () => {
883+
const result = normalizeLocationHeader(
884+
"http://localhost:3000/path",
885+
"http://localhost:3000",
886+
);
887+
expect(result).toBe("/path");
888+
});
889+
890+
it("should not change absolute location header", () => {
891+
const result = normalizeLocationHeader(
892+
"https://opennext.js.org/aws",
893+
"http://localhost:3000",
894+
);
895+
expect(result).toBe("https://opennext.js.org/aws");
896+
});
897+
898+
it("should normalize relative location with query parameters", () => {
899+
const result = normalizeLocationHeader(
900+
"http://localhost:3000/path?query=1",
901+
"http://localhost:3000",
902+
);
903+
expect(result).toBe("/path?query=1");
904+
});
905+
906+
it("should normalize and encode special characters in location header", () => {
907+
const result = normalizeLocationHeader(
908+
"http://localhost:3000/about?query=æøå!&",
909+
"http://localhost:3000",
910+
true,
911+
);
912+
expect(result).toBe("/about?query=%C3%A6%C3%B8%C3%A5!");
913+
});
914+
915+
it("should normalize and respect already encoded characters in location header", () => {
916+
const result = normalizeLocationHeader(
917+
"http://localhost:3000/path?query=test%2F%2F",
918+
"http://localhost:3000",
919+
true,
920+
);
921+
expect(result).toBe("/path?query=test%2F%2F");
922+
});
923+
924+
it("should preserve multiple query parameters in location header", () => {
925+
const result = normalizeLocationHeader(
926+
"http://localhost:3000/path?query1=value1&query2=value2&query1=value3&query2=value4&random=randomvalue",
927+
"http://localhost:3000",
928+
true,
929+
);
930+
expect(result).toBe(
931+
"/path?query1=value1&query1=value3&query2=value2&query2=value4&random=randomvalue",
932+
);
933+
});
934+
935+
it("should return absolute URL correctly when the base URL is different", () => {
936+
const result = normalizeLocationHeader(
937+
"https://example.com/path?query=1",
938+
"http://localhost:3000",
939+
);
940+
expect(result).toBe("https://example.com/path?query=1");
941+
});
942+
});

0 commit comments

Comments
 (0)