diff --git a/packages/blocks/src/sdk/redirects.test.ts b/packages/blocks/src/sdk/redirects.test.ts new file mode 100644 index 00000000..0f71c80f --- /dev/null +++ b/packages/blocks/src/sdk/redirects.test.ts @@ -0,0 +1,73 @@ +/** + * Regression tests for self-redirects — rules whose source and target resolve + * to the same path, which a browser reports as ERR_TOO_MANY_REDIRECTS. + * + * The cases below are real rows from a bulk-migration CSV (Fresh -> TanStack) + * that took two pages of a production storefront down. + */ +import { describe, expect, it } from "vitest"; +import { addRedirects, loadRedirects, matchRedirect, parseRedirectsCsv } from "./redirects"; + +const fromCsv = (csv: string) => { + const map = loadRedirects({}); + addRedirects(map, parseRedirectsCsv(csv)); + return map; +}; + +describe("self-redirects are dropped at load time", () => { + it("drops a row that literally points at itself", () => { + expect(parseRedirectsCsv("from,to\n/a,/a\n")).toEqual([]); + expect(matchRedirect("/a", fromCsv("from,to\n/a,/a\n"))).toBeNull(); + }); + + it("drops a row whose absolute source collapses onto its target", () => { + // Took /aliancas down: normalizePath drops the query, leaving /x -> /x. + const map = fromCsv( + "from,to,type\nhttps://www.example.com/aliancas?map=category-1,/aliancas,PERMANENT\n", + ); + expect(matchRedirect("/aliancas", map)).toBeNull(); + }); + + it("drops a row whose absolute target collapses onto its source", () => { + // Took the home page down: http://blog.example.com/ -> / + expect(matchRedirect("/", fromCsv("from,to\nhttp://blog.example.com/,/\n"))).toBeNull(); + }); + + it("drops a pair that differs only by trailing slash", () => { + expect(parseRedirectsCsv("from,to\n/a/,/a\n")).toEqual([]); + expect(parseRedirectsCsv("from,to\n/a,/a/\n")).toEqual([]); + }); + + it("drops a pair that differs only by case", () => { + // normalizePath lowercases the source, so /A would match a request to /a. + expect(parseRedirectsCsv("from,to\n/A,/a\n")).toEqual([]); + }); + + it("drops self-redirects declared as CMS blocks, not just CSV rows", () => { + const map = loadRedirects({ + r: { + __resolveType: "website/loaders/redirect.ts", + redirect: { from: "/loop", to: "/loop", type: "permanent" }, + }, + }); + expect(matchRedirect("/loop", map)).toBeNull(); + }); + + it("keeps a same-path redirect that leaves the site", () => { + // Same pathname, different host: a real redirect, not a loop. + const map = fromCsv("from,to\n/x,https://other.example.com/x\n"); + expect(matchRedirect("/x", map)).toMatchObject({ to: "https://other.example.com/x" }); + }); + + it("keeps a same-path redirect that only adds a query", () => { + const map = fromCsv("from,to\n/x,/x?ref=1\n"); + expect(matchRedirect("/x", map)).toBeNull(); + }); + + it("leaves ordinary redirects alone", () => { + const map = fromCsv("from,to,type\n/old,/new,permanent\n/blog/*,/news/*,permanent\n"); + expect(matchRedirect("/old", map)).toMatchObject({ to: "/new", status: 301 }); + expect(matchRedirect("/blog/post-1", map)).toMatchObject({ to: "/news/post-1" }); + expect(matchRedirect("/unknown", map)).toBeNull(); + }); +}); diff --git a/packages/blocks/src/sdk/redirects.ts b/packages/blocks/src/sdk/redirects.ts index 68de0919..aa6d9935 100644 --- a/packages/blocks/src/sdk/redirects.ts +++ b/packages/blocks/src/sdk/redirects.ts @@ -102,6 +102,7 @@ export function loadRedirects(blocks: Record): RedirectMap { for (const entry of list) { if (!entry.from || !entry.to) continue; + if (isSelfRedirect(entry.from, entry.to)) continue; const redirect: Redirect = { from: normalizePath(entry.from), @@ -148,6 +149,7 @@ export function parseRedirectsCsv(csv: string): Redirect[] { // Skip a header row (`from,to[,type]`). Robust for CSVs with or without a // header, and for a header repeated when multiple files are concatenated. if (from.toLowerCase() === "from" && to.toLowerCase() === "to") continue; + if (isSelfRedirect(from, to)) continue; redirects.push({ from: normalizePath(from), @@ -204,6 +206,54 @@ export function matchRedirect(pathname: string, map: RedirectMap): Redirect | nu // Helpers // ------------------------------------------------------------------------- +const isAbsolute = (u: string) => u.startsWith("http://") || u.startsWith("https://"); + +/** origin + normalized pathname, or null when the URL is malformed. */ +function absoluteKey(url: string): string | null { + try { + const parsed = new URL(url); + return parsed.origin + normalizePath(parsed.pathname); + } catch { + return null; + } +} + +/** + * A rule whose source and target resolve to the same page redirects to itself: + * the response sends the browser back to the URL it just asked for, which it + * reports as ERR_TOO_MANY_REDIRECTS. + * + * These are routine in bulk migration exports, and `normalizePath` manufactures + * more of them: it reduces an absolute `from` to `new URL(p).pathname`, so a + * rule scoped to one query (`https://site/x?map=category-1 -> /x`) collapses + * onto the bare path it points at. + * + * Host is compared whenever both sides carry one. A relative target is compared + * on path alone — the map is keyed by pathname, so a rule reached from any host + * lands the visitor back on this site's copy of that path. + * + * The Fresh loader this SDK replaced (deco-cx/apps + * `website/loaders/redirectsFromCsv.ts`) carried the same `from === to` guard. + */ +function isSelfRedirect(from: string, to: string): boolean { + const src = from.trim(); + const dst = to.trim(); + + // Both absolute: same site AND same path is a loop; a different host is not. + if (isAbsolute(src) && isAbsolute(dst)) { + const a = absoluteKey(src); + return a !== null && a === absoluteKey(dst); + } + + // Target on another origin (absolute or protocol-relative) always goes + // somewhere else — the host is what differs, and we cannot know ours. + if (isAbsolute(dst) || dst.startsWith("//")) return false; + + const qIdx = dst.indexOf("?"); + const dstPath = normalizePath(qIdx >= 0 ? dst.slice(0, qIdx) : dst); + return dstPath === normalizePath(src); +} + function normalizePath(path: string): string { let p = path.trim();