diff --git a/.changeset/wet-dogs-check.md b/.changeset/wet-dogs-check.md new file mode 100644 index 0000000000..4bf4f7e447 --- /dev/null +++ b/.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 diff --git a/contributors.yml b/contributors.yml index 8340c2f402..01294bb010 100644 --- a/contributors.yml +++ b/contributors.yml @@ -9,6 +9,7 @@ - alany411 - alexlbr - AmRo045 +- amsal - andreiduca - arnassavickas - aroyan diff --git a/package.json b/package.json index 607528bd9b..446186dc2d 100644 --- a/package.json +++ b/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" diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 05181d86d2..9d94824d7a 100644 --- a/packages/router/__tests__/router-test.ts +++ b/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 }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 34d10d0c5c..9767a3f328 100644 --- a/packages/router/router.ts +++ b/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; } }