Skip to content

Commit c6bab5b

Browse files
authored
fix: Use URL parsing instead of substring matching in isExternal (opennextjs#1046)
1 parent da773c9 commit c6bab5b

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed

.changeset/pink-years-teach.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@opennextjs/aws": patch
3+
---
4+
5+
fix: Correct external URL detection in isExternal using proper URL parsing
6+
7+
Replaces substring-based host matching with URL parsing to correctly determine whether a rewritten URL is external.
8+
This fixes an issue where NextResponse.rewrite() would treat certain external URLs as internal when their pathname contained the host as a substring, causing unexpected 404s during middleware rewrites.

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,18 @@ import { generateMessageGroupId } from "./queue.js";
3030
export function isExternal(url?: string, host?: string) {
3131
if (!url) return false;
3232
const pattern = /^https?:\/\//;
33+
if (!pattern.test(url)) return false;
34+
3335
if (host) {
34-
return pattern.test(url) && !url.includes(host);
36+
try {
37+
const parsedUrl = new URL(url);
38+
return parsedUrl.host !== host;
39+
} catch {
40+
// If URL parsing fails, fall back to substring check
41+
return !url.includes(host);
42+
}
3543
}
36-
return pattern.test(url);
44+
return true;
3745
}
3846

3947
export function convertFromQueryString(query: string) {

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,24 @@ describe("isExternal", () => {
8787
it("returns false for absolute https url same host", () => {
8888
expect(isExternal("https://absolute.com/path", "absolute.com")).toBe(false);
8989
});
90+
91+
it("returns true for absolute https url different host but same host in path", () => {
92+
expect(isExternal("https://absolute.com/local.com", "local.com")).toBe(
93+
true,
94+
);
95+
});
96+
97+
it("returns false for same host with port", () => {
98+
expect(isExternal("https://localhost:3000/path", "localhost:3000")).toBe(
99+
false,
100+
);
101+
});
102+
103+
it("returns true for different port on same hostname", () => {
104+
expect(isExternal("https://localhost:3001/path", "localhost:3000")).toBe(
105+
true,
106+
);
107+
});
90108
});
91109

92110
describe("convertFromQueryString", () => {

0 commit comments

Comments
 (0)