fix(router): "hard" redirect to different paths on the same origin if redirect location does not contain basename (#10076)

Co-authored-by: Matt Brophy <[email protected]>

Author: amsal

.changeset/wet-dogs-check.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Correctly perform a "hard" redirect for same-origin absolute URLs outside of the router basename

contributors.yml

@@ -10,6 +10,7 @@
 - alany411
 - alexlbr
 - AmRo045
+- amsal
 - andreiduca
 - arnassavickas
 - aroyan

package.json

@@ -105,7 +105,7 @@
   },
   "filesize": {
     "packages/router/dist/router.umd.min.js": {
-      "none": "41.6 kB"
+      "none": "41.7 kB"
     },
     "packages/react-router/dist/react-router.production.min.js": {
       "none": "13 kB"

packages/router/__tests__/router-test.ts

@@ -630,6 +630,10 @@ function setup({
           let popHref = history.createHref(history.location);
           if (currentRouter?.basename) {
             popHref = stripBasename(popHref, currentRouter.basename) as string;
+            invariant(
+              popHref,
+              "href passed to navigate should start with basename"
+            );
           }
           helpers = getNavigationHelpers(popHref, navigationId);
           unsubscribe();
@@ -645,6 +649,10 @@ function setup({
     let navHref = href;
     if (currentRouter.basename) {
       navHref = stripBasename(navHref, currentRouter.basename) as string;
+      invariant(
+        navHref,
+        "href passed to t.navigate() should start with basename"
+      );
     }
     helpers = getNavigationHelpers(navHref, navigationId);
     shims?.forEach((routeId) =>
@@ -6293,6 +6301,56 @@ describe("a router", () => {
       });
     });
 
+    it("properly handles same-origin absolute URLs when using a basename", async () => {
+      let t = setup({ routes: REDIRECT_ROUTES, basename: "/base" });
+
+      let A = await t.navigate("/base/parent/child", {
+        formMethod: "post",
+        formData: createFormData({}),
+      });
+
+      let B = await A.actions.child.redirectReturn(
+        "http://localhost/base/parent",
+        undefined,
+        undefined,
+        ["parent"]
+      );
+      await B.loaders.parent.resolve("PARENT");
+      expect(t.router.state.location).toMatchObject({
+        hash: "",
+        pathname: "/base/parent",
+        search: "",
+        state: {
+          _isRedirect: true,
+        },
+      });
+    });
+
+    it("treats same-origin absolute URLs as external if they don't match the basename", async () => {
+      // This is gross, don't blame me, blame SO :)
+      // https://stackoverflow.com/a/60697570
+      let oldLocation = window.location;
+      const location = new URL(window.location.href) as unknown as Location;
+      location.assign = jest.fn();
+      location.replace = jest.fn();
+      delete (window as any).location;
+      window.location = location as unknown as Location;
+
+      let t = setup({ routes: REDIRECT_ROUTES, basename: "/base" });
+
+      let A = await t.navigate("/base/parent/child", {
+        formMethod: "post",
+        formData: createFormData({}),
+      });
+
+      let url = "http://localhost/not/the/same/basename";
+      await A.actions.child.redirectReturn(url);
+      expect(window.location.assign).toHaveBeenCalledWith(url);
+      expect(window.location.replace).not.toHaveBeenCalled();
+
+      window.location = oldLocation;
+    });
+
     describe("redirect status code handling", () => {
       it("should not treat 300 as a redirect", async () => {
         let t = setup({ routes: REDIRECT_ROUTES });

packages/router/router.ts

@@ -33,6 +33,7 @@ import {
   joinPaths,
   matchRoutes,
   resolveTo,
+  stripBasename,
   warning,
 } from "./utils";
 
@@ -1935,15 +1936,17 @@ export function createRouter(init: RouterInit): Router {
       redirectLocation,
       "Expected a location on the redirect navigation"
     );
-
     // Check if this an absolute external redirect that goes to a new origin
     if (
       ABSOLUTE_URL_REGEX.test(redirect.location) &&
       isBrowser &&
       typeof window?.location !== "undefined"
     ) {
-      let newOrigin = init.history.createURL(redirect.location).origin;
-      if (window.location.origin !== newOrigin) {
+      let url = init.history.createURL(redirect.location);
+      let isDifferentBasename =
+        stripBasename(url.pathname, init.basename || "/") == null;
+
+      if (window.location.origin !== url.origin || isDifferentBasename) {
         if (replace) {
           window.location.replace(redirect.location);
         } else {
@@ -3173,14 +3176,15 @@ 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
+        // Strip off the protocol+origin for same-origin + same-basename absolute
+        // redirects. If this is a static request, 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) {
+        let isSameBasename = stripBasename(url.pathname, basename) != null;
+        if (url.origin === currentUrl.origin && isSameBasename) {
           location = url.pathname + url.search + url.hash;
         }
       }