Fix 404 bug with same-origin absolute redirects

.changeset/rare-dancers-shave.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Fix 404 bug with same-origin absolute redirects

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

@@ -105,4 +105,76 @@ describe("a memory router", () => {
     );
     router.dispose();
   });
+
+  it("properly handles same-origin absolute URLs", async () => {
+    let router = createRouter({
+      routes: [
+        {
+          path: "/",
+          children: [
+            {
+              index: true,
+            },
+            {
+              path: "a",
+              loader: () =>
+                new Response(null, {
+                  status: 302,
+                  headers: {
+                    Location: "http://localhost/b",
+                  },
+                }),
+            },
+            {
+              path: "b",
+            },
+          ],
+        },
+      ],
+      history: createMemoryHistory(),
+    });
+
+    await router.navigate("/a");
+    expect(router.state.location).toMatchObject({
+      hash: "",
+      pathname: "/b",
+      search: "",
+    });
+  });
+
+  it("properly handles protocol-less same-origin absolute URLs", async () => {
+    let router = createRouter({
+      routes: [
+        {
+          path: "/",
+          children: [
+            {
+              index: true,
+            },
+            {
+              path: "a",
+              loader: () =>
+                new Response(null, {
+                  status: 302,
+                  headers: {
+                    Location: "//localhost/b",
+                  },
+                }),
+            },
+            {
+              path: "b",
+            },
+          ],
+        },
+      ],
+      history: createMemoryHistory(),
+    });
+
+    await router.navigate("/a");
+    expect(router.state.location).toMatchObject({
+      hash: "",
+      pathname: "/b",
+      search: "",
+    });
+  });
 });

packages/router/__tests__/router-test.ts

@@ -6223,6 +6223,31 @@ describe("a router", () => {
       }
     });
 
+    it("properly handles same-origin absolute URLs", async () => {
+      let t = setup({ routes: REDIRECT_ROUTES });
+
+      let A = await t.navigate("/parent/child", {
+        formMethod: "post",
+        formData: createFormData({}),
+      });
+
+      let B = await A.actions.child.redirectReturn(
+        "http://localhost/parent",
+        undefined,
+        undefined,
+        ["parent"]
+      );
+      await B.loaders.parent.resolve("PARENT");
+      expect(t.router.state.location).toMatchObject({
+        hash: "",
+        pathname: "/parent",
+        search: "",
+        state: {
+          _isRedirect: true,
+        },
+      });
+    });
+
     describe("redirect status code handling", () => {
       it("should not treat 300 as a redirect", async () => {
         let t = setup({ routes: REDIRECT_ROUTES });
@@ -11293,6 +11318,20 @@ describe("a router", () => {
       },
     ];
 
+    // Regardless of if the URL is internal or external - all absolute URL
+    // responses should return untouched during SSR so the browser can handle
+    // them
+    let ABSOLUTE_URLS = [
+      "http://localhost/",
+      "https://localhost/about",
+      "http://remix.run/blog",
+      "https://remix.run/blog",
+      "//remix.run/blog",
+      "app://whatever",
+      "mailto:[email protected]",
+      "web+remix:whatever",
+    ];
+
     function createRequest(path: string, opts?: RequestInit) {
       return new Request(`http://localhost${path}`, {
         signal: new AbortController().signal,
@@ -11616,17 +11655,8 @@ describe("a router", () => {
         expect((response as Response).headers.get("Location")).toBe("/parent");
       });
 
-      it("should handle external redirect Responses", async () => {
-        let urls = [
-          "http://remix.run/blog",
-          "https://remix.run/blog",
-          "//remix.run/blog",
-          "app://whatever",
-          "mailto:[email protected]",
-          "web+remix:whatever",
-        ];
-
-        for (let url of urls) {
+      it("should handle absolute redirect Responses", async () => {
+        for (let url of ABSOLUTE_URLS) {
           let handler = createStaticHandler([
             {
               path: "/",
@@ -12954,15 +12984,8 @@ describe("a router", () => {
         expect((response as Response).headers.get("Location")).toBe("/parent");
       });
 
-      it("should handle external redirect Responses", async () => {
-        let urls = [
-          "http://remix.run/blog",
-          "https://remix.run/blog",
-          "//remix.run/blog",
-          "app://whatever",
-        ];
-
-        for (let url of urls) {
+      it("should handle absolute redirect Responses", async () => {
+        for (let url of ABSOLUTE_URLS) {
           let handler = createStaticHandler([
             {
               id: "root",

packages/router/router.ts

@@ -1901,7 +1901,7 @@ export function createRouter(init: RouterInit): Router {
     );
 
     // Check if this an external redirect that goes to a new origin
-    if (typeof window?.location !== "undefined") {
+    if (isBrowser && typeof window?.location !== "undefined") {
       let newOrigin = init.history.createURL(redirect.location).origin;
       if (window.location.origin !== newOrigin) {
         if (replace) {
@@ -3115,6 +3115,17 @@ async function callLoaderOrAction(
         }
 
         location = createPath(resolvedLocation);
+      } else if (!isStaticRequest) {
+        // Strip off the protocol+origin for same-origin absolute redirects.
+        // If this is a static reques, we can let it go back to the browser
+        // as-is
+        let currentUrl = new URL(request.url);
+        let url = location.startsWith("//")
+          ? new URL(currentUrl.protocol + location)
+          : new URL(location);
+        if (url.origin === currentUrl.origin) {
+          location = url.pathname + url.search + url.hash;
+        }
       }
 
       // Don't process redirects in the router during static requests requests.