Skip to content

Commit bf46fb6

Browse files
authored
Fix 404 bug with same-origin absolute redirects (#9913)
* Fix 404 bug with same-origin absolute redirects * Remove unused import
1 parent 3f19248 commit bf46fb6

File tree

4 files changed

+132
-21
lines changed

4 files changed

+132
-21
lines changed

.changeset/rare-dancers-shave.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@remix-run/router": patch
3+
---
4+
5+
Fix 404 bug with same-origin absolute redirects

packages/router/__tests__/router-memory-test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,76 @@ describe("a memory router", () => {
105105
);
106106
router.dispose();
107107
});
108+
109+
it("properly handles same-origin absolute URLs", async () => {
110+
let router = createRouter({
111+
routes: [
112+
{
113+
path: "/",
114+
children: [
115+
{
116+
index: true,
117+
},
118+
{
119+
path: "a",
120+
loader: () =>
121+
new Response(null, {
122+
status: 302,
123+
headers: {
124+
Location: "http://localhost/b",
125+
},
126+
}),
127+
},
128+
{
129+
path: "b",
130+
},
131+
],
132+
},
133+
],
134+
history: createMemoryHistory(),
135+
});
136+
137+
await router.navigate("/a");
138+
expect(router.state.location).toMatchObject({
139+
hash: "",
140+
pathname: "/b",
141+
search: "",
142+
});
143+
});
144+
145+
it("properly handles protocol-less same-origin absolute URLs", async () => {
146+
let router = createRouter({
147+
routes: [
148+
{
149+
path: "/",
150+
children: [
151+
{
152+
index: true,
153+
},
154+
{
155+
path: "a",
156+
loader: () =>
157+
new Response(null, {
158+
status: 302,
159+
headers: {
160+
Location: "//localhost/b",
161+
},
162+
}),
163+
},
164+
{
165+
path: "b",
166+
},
167+
],
168+
},
169+
],
170+
history: createMemoryHistory(),
171+
});
172+
173+
await router.navigate("/a");
174+
expect(router.state.location).toMatchObject({
175+
hash: "",
176+
pathname: "/b",
177+
search: "",
178+
});
179+
});
108180
});

packages/router/__tests__/router-test.ts

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6223,6 +6223,31 @@ describe("a router", () => {
62236223
}
62246224
});
62256225

6226+
it("properly handles same-origin absolute URLs", async () => {
6227+
let t = setup({ routes: REDIRECT_ROUTES });
6228+
6229+
let A = await t.navigate("/parent/child", {
6230+
formMethod: "post",
6231+
formData: createFormData({}),
6232+
});
6233+
6234+
let B = await A.actions.child.redirectReturn(
6235+
"http://localhost/parent",
6236+
undefined,
6237+
undefined,
6238+
["parent"]
6239+
);
6240+
await B.loaders.parent.resolve("PARENT");
6241+
expect(t.router.state.location).toMatchObject({
6242+
hash: "",
6243+
pathname: "/parent",
6244+
search: "",
6245+
state: {
6246+
_isRedirect: true,
6247+
},
6248+
});
6249+
});
6250+
62266251
describe("redirect status code handling", () => {
62276252
it("should not treat 300 as a redirect", async () => {
62286253
let t = setup({ routes: REDIRECT_ROUTES });
@@ -11293,6 +11318,20 @@ describe("a router", () => {
1129311318
},
1129411319
];
1129511320

11321+
// Regardless of if the URL is internal or external - all absolute URL
11322+
// responses should return untouched during SSR so the browser can handle
11323+
// them
11324+
let ABSOLUTE_URLS = [
11325+
"http://localhost/",
11326+
"https://localhost/about",
11327+
"http://remix.run/blog",
11328+
"https://remix.run/blog",
11329+
"//remix.run/blog",
11330+
"app://whatever",
11331+
11332+
"web+remix:whatever",
11333+
];
11334+
1129611335
function createRequest(path: string, opts?: RequestInit) {
1129711336
return new Request(`http://localhost${path}`, {
1129811337
signal: new AbortController().signal,
@@ -11616,17 +11655,8 @@ describe("a router", () => {
1161611655
expect((response as Response).headers.get("Location")).toBe("/parent");
1161711656
});
1161811657

11619-
it("should handle external redirect Responses", async () => {
11620-
let urls = [
11621-
"http://remix.run/blog",
11622-
"https://remix.run/blog",
11623-
"//remix.run/blog",
11624-
"app://whatever",
11625-
11626-
"web+remix:whatever",
11627-
];
11628-
11629-
for (let url of urls) {
11658+
it("should handle absolute redirect Responses", async () => {
11659+
for (let url of ABSOLUTE_URLS) {
1163011660
let handler = createStaticHandler([
1163111661
{
1163211662
path: "/",
@@ -12962,15 +12992,8 @@ describe("a router", () => {
1296212992
expect((response as Response).headers.get("Location")).toBe("/parent");
1296312993
});
1296412994

12965-
it("should handle external redirect Responses", async () => {
12966-
let urls = [
12967-
"http://remix.run/blog",
12968-
"https://remix.run/blog",
12969-
"//remix.run/blog",
12970-
"app://whatever",
12971-
];
12972-
12973-
for (let url of urls) {
12995+
it("should handle absolute redirect Responses", async () => {
12996+
for (let url of ABSOLUTE_URLS) {
1297412997
let handler = createStaticHandler([
1297512998
{
1297612999
id: "root",

packages/router/router.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1901,7 +1901,7 @@ export function createRouter(init: RouterInit): Router {
19011901
);
19021902

19031903
// Check if this an external redirect that goes to a new origin
1904-
if (typeof window?.location !== "undefined") {
1904+
if (isBrowser && typeof window?.location !== "undefined") {
19051905
let newOrigin = init.history.createURL(redirect.location).origin;
19061906
if (window.location.origin !== newOrigin) {
19071907
if (replace) {
@@ -3115,6 +3115,17 @@ async function callLoaderOrAction(
31153115
}
31163116

31173117
location = createPath(resolvedLocation);
3118+
} else if (!isStaticRequest) {
3119+
// Strip off the protocol+origin for same-origin absolute redirects.
3120+
// If this is a static reques, we can let it go back to the browser
3121+
// as-is
3122+
let currentUrl = new URL(request.url);
3123+
let url = location.startsWith("//")
3124+
? new URL(currentUrl.protocol + location)
3125+
: new URL(location);
3126+
if (url.origin === currentUrl.origin) {
3127+
location = url.pathname + url.search + url.hash;
3128+
}
31183129
}
31193130

31203131
// Don't process redirects in the router during static requests requests.

0 commit comments

Comments
 (0)