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..9faa2dba 100644 --- a/packages/blocks/src/sdk/redirects.ts +++ b/packages/blocks/src/sdk/redirects.ts @@ -40,6 +40,16 @@ export interface Redirect { from: string; to: string; status: 301 | 302; + /** + * Query the source URL was scoped to, without the leading "?" (e.g. + * `map=category-1`). Only set when the CSV/CMS `from` carried one. + * + * The map is keyed by pathname, so without this a query-scoped rule would + * have to widen into a whole-page rule — which is how these rows used to + * produce redirect loops. `matchRedirect` fires them only when the incoming + * search matches. + */ + search?: string; } export interface RedirectMap { @@ -47,6 +57,12 @@ export interface RedirectMap { exact: Map; /** Glob/prefix redirects checked sequentially (few in practice). */ patterns: Array<{ prefix: string; redirect: Redirect }>; + /** + * Rules whose `from` carried a query, bucketed by pathname. Only reachable + * when `matchRedirect` is given the request's search string, so they can + * never hijack a bare path. + */ + scoped: Map; } // ------------------------------------------------------------------------- @@ -83,6 +99,7 @@ export function registerRedirectResolveType(resolveType: string): void { export function loadRedirects(blocks: Record): RedirectMap { const exact = new Map(); const patterns: Array<{ prefix: string; redirect: Redirect }> = []; + const scoped = new Map(); for (const [_key, block] of Object.entries(blocks)) { if (!block || typeof block !== "object") continue; @@ -103,22 +120,21 @@ export function loadRedirects(blocks: Record): RedirectMap { for (const entry of list) { if (!entry.from || !entry.to) continue; + const { path, search } = splitSource(entry.from); + if (!search && isSelfRedirect(entry.from, entry.to)) continue; + const redirect: Redirect = { - from: normalizePath(entry.from), + from: path, to: entry.to, status: entry.type === "permanent" ? 301 : 302, + ...(search ? { search } : {}), }; - if (redirect.from.includes("*")) { - const prefix = redirect.from.replace(/\*+$/, ""); - patterns.push({ prefix, redirect }); - } else { - exact.set(redirect.from, redirect); - } + addToMap({ exact, patterns, scoped }, redirect); } } - return { exact, patterns }; + return { exact, patterns, scoped }; } // ------------------------------------------------------------------------- @@ -149,10 +165,14 @@ export function parseRedirectsCsv(csv: string): Redirect[] { // header, and for a header repeated when multiple files are concatenated. if (from.toLowerCase() === "from" && to.toLowerCase() === "to") continue; + const { path, search } = splitSource(from); + if (!search && isSelfRedirect(from, to)) continue; + redirects.push({ - from: normalizePath(from), + from: path, to, status: type === "permanent" || type === "301" ? 301 : 302, + ...(search ? { search } : {}), }); } @@ -164,12 +184,23 @@ export function parseRedirectsCsv(csv: string): Redirect[] { */ export function addRedirects(map: RedirectMap, redirects: Redirect[]): void { for (const redirect of redirects) { - if (redirect.from.includes("*")) { - const prefix = redirect.from.replace(/\*+$/, ""); - map.patterns.push({ prefix, redirect }); - } else { - map.exact.set(redirect.from, redirect); - } + addToMap(map, redirect); + } +} + +/** Route a redirect into the bucket that matches its shape. */ +function addToMap(map: RedirectMap, redirect: Redirect): void { + if (redirect.search) { + const bucket = map.scoped.get(redirect.from); + if (bucket) bucket.push(redirect); + else map.scoped.set(redirect.from, [redirect]); + return; + } + if (redirect.from.includes("*")) { + const prefix = redirect.from.replace(/\*+$/, ""); + map.patterns.push({ prefix, redirect }); + } else { + map.exact.set(redirect.from, redirect); } } @@ -183,9 +214,19 @@ export function addRedirects(map: RedirectMap, redirects: Redirect[]): void { * Checks exact matches first (O(1)), then glob patterns (O(n), but * typically few patterns exist). */ -export function matchRedirect(pathname: string, map: RedirectMap): Redirect | null { +export function matchRedirect( + pathname: string, + map: RedirectMap, + search?: string, +): Redirect | null { const normalized = normalizePath(pathname); + // Query-scoped rules are the more specific match, so they win over a + // bare-path rule for the same pathname. Callers that pass no `search` + // never reach them. + const scopedMatch = matchScoped(normalized, map, search); + if (scopedMatch) return scopedMatch; + const exactMatch = map.exact.get(normalized); if (exactMatch) return exactMatch; @@ -200,10 +241,111 @@ export function matchRedirect(pathname: string, map: RedirectMap): Redirect | nu return null; } +/** + * A query-scoped rule matches when every `key=value` it was defined with is + * present in the request. Subset rather than equality, so appended tracking + * params (utm_*, gclid, fbclid) do not defeat the rule. + */ +function matchScoped(pathname: string, map: RedirectMap, search?: string): Redirect | null { + if (!search) return null; + const bucket = map.scoped.get(pathname); + if (!bucket) return null; + + const actual = new URLSearchParams(search.startsWith("?") ? search.slice(1) : search); + for (const redirect of bucket) { + let matches = true; + for (const [key, value] of new URLSearchParams(redirect.search)) { + if (actual.get(key) !== value) { + matches = false; + break; + } + } + if (matches) return redirect; + } + return null; +} + // ------------------------------------------------------------------------- // Helpers // ------------------------------------------------------------------------- +/** + * Split a redirect source into pathname and query. + * + * The map is keyed by pathname, so the query has to travel beside it rather + * than being dropped — dropping it widens a query-scoped rule into a rule for + * the whole page (see `Redirect.search`). + */ +function splitSource(from: string): { path: string; search?: string } { + const raw = from.trim(); + + if (isAbsolute(raw)) { + try { + const url = new URL(raw); + return { + path: normalizePath(url.pathname), + search: url.search ? url.search.slice(1) : undefined, + }; + } catch { + // malformed URL: fall through to the string-level split + } + } + + const qIdx = raw.indexOf("?"); + if (qIdx >= 0) { + return { path: normalizePath(raw.slice(0, qIdx)), search: raw.slice(qIdx + 1) || undefined }; + } + return { path: normalizePath(raw) }; +} + +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(); diff --git a/packages/blocks/src/sdk/redirectsQueryScoped.test.ts b/packages/blocks/src/sdk/redirectsQueryScoped.test.ts new file mode 100644 index 00000000..d044bb88 --- /dev/null +++ b/packages/blocks/src/sdk/redirectsQueryScoped.test.ts @@ -0,0 +1,98 @@ +/** + * Rules whose source carries a query string. + * + * The Fresh loader this SDK replaced matched the whole href, so a rule written + * against `/x?map=category-1` fired for that query only. Here the map is keyed + * by pathname, so the query travels beside it and is checked at match time. + */ +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("query-scoped rules", () => { + it("fires for the query it was written against", () => { + const map = fromCsv("from,to,type\n/relogios?map=category-1,/relogios/todos,permanent\n"); + + expect(matchRedirect("/relogios", map, "map=category-1")).toMatchObject({ + to: "/relogios/todos", + status: 301, + }); + expect(matchRedirect("/relogios", map, "?map=category-1")).toMatchObject({ + to: "/relogios/todos", + }); + }); + + it("stays out of the way of the bare path", () => { + const map = fromCsv("from,to\n/relogios?map=category-1,/relogios/todos\n"); + + expect(matchRedirect("/relogios", map)).toBeNull(); + expect(matchRedirect("/relogios", map, "")).toBeNull(); + expect(matchRedirect("/relogios", map, "map=something-else")).toBeNull(); + }); + + it("survives a source that points at its own bare path", () => { + // The row that caused the loop: legitimate once the query is honored. + const map = fromCsv( + "from,to,type\nhttps://www.example.com/aliancas?map=category-1,/aliancas,PERMANENT\n", + ); + + expect(matchRedirect("/aliancas", map)).toBeNull(); + expect(matchRedirect("/aliancas", map, "map=category-1")).toMatchObject({ to: "/aliancas" }); + }); + + it("tolerates tracking params appended to the request", () => { + const map = fromCsv("from,to\n/p?map=c,/new\n"); + + expect(matchRedirect("/p", map, "map=c&utm_source=news&gclid=x")).toMatchObject({ to: "/new" }); + expect(matchRedirect("/p", map, "utm_source=news")).toBeNull(); + }); + + it("requires every param of a multi-param rule", () => { + const map = fromCsv("from,to\n/p?a=1&b=2,/new\n"); + + expect(matchRedirect("/p", map, "a=1&b=2")).toMatchObject({ to: "/new" }); + expect(matchRedirect("/p", map, "a=1")).toBeNull(); + expect(matchRedirect("/p", map, "a=1&b=3")).toBeNull(); + }); + + it("wins over the bare-path rule for the same pathname", () => { + const map = fromCsv("from,to\n/x,/bare\n/x?v=1,/scoped\n"); + + expect(matchRedirect("/x", map)).toMatchObject({ to: "/bare" }); + expect(matchRedirect("/x", map, "v=1")).toMatchObject({ to: "/scoped" }); + expect(matchRedirect("/x", map, "v=2")).toMatchObject({ to: "/bare" }); + }); + + it("keeps several rules on one pathname apart", () => { + const map = fromCsv("from,to\n/x?v=1,/one\n/x?v=2,/two\n"); + + expect(matchRedirect("/x", map, "v=1")).toMatchObject({ to: "/one" }); + expect(matchRedirect("/x", map, "v=2")).toMatchObject({ to: "/two" }); + expect(matchRedirect("/x", map, "v=3")).toBeNull(); + }); + + it("works for CMS blocks, not just CSV rows", () => { + const map = loadRedirects({ + r: { + __resolveType: "website/loaders/redirect.ts", + redirect: { from: "/x?legacy=1", to: "/y", type: "permanent" }, + }, + }); + + expect(matchRedirect("/x", map)).toBeNull(); + expect(matchRedirect("/x", map, "legacy=1")).toMatchObject({ to: "/y", status: 301 }); + }); + + it("leaves rules without a query untouched", () => { + 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("/old", map, "utm_source=x")).toMatchObject({ to: "/new" }); + expect(matchRedirect("/blog/post-1", map, "a=1")).toMatchObject({ to: "/news/post-1" }); + }); +}); diff --git a/packages/tanstack/src/sdk/workerEntry.ts b/packages/tanstack/src/sdk/workerEntry.ts index 633c4c6c..8073df3a 100644 --- a/packages/tanstack/src/sdk/workerEntry.ts +++ b/packages/tanstack/src/sdk/workerEntry.ts @@ -1105,13 +1105,7 @@ export function createDecoWorkerEntry( return detectCacheProfile(target); } - const KNOWN_SEGMENT_FIELDS = new Set([ - "device", - "loggedIn", - "salesChannel", - "regionId", - "flags", - ]); + const KNOWN_SEGMENT_FIELDS = new Set(["device", "loggedIn", "salesChannel", "regionId", "flags"]); function hashSegment(seg: SegmentKey): string { const parts: string[] = [seg.device]; @@ -1819,7 +1813,9 @@ export function createDecoWorkerEntry( _redirectMap = loadRedirects(loadBlocks()); _redirectMapRevision = currentRevision; } - const cmsRedirect = matchRedirect(url.pathname, _redirectMap!); + // `url.search` is what lets query-scoped rules (`/x?map=category-1 -> /y`) + // match; without it they stay inert rather than hijacking the bare path. + const cmsRedirect = matchRedirect(url.pathname, _redirectMap!, url.search); if (cmsRedirect) { return new Response(null, { status: cmsRedirect.status, @@ -1966,11 +1962,7 @@ export function createDecoWorkerEntry( } // ?asJson — return resolved page data as JSON (legacy deco compat) - if ( - options.asJson !== false && - url.searchParams.has("asJson") && - request.method === "GET" - ) { + if (options.asJson !== false && url.searchParams.has("asJson") && request.method === "GET") { const basePath = url.pathname; const cookies: Record = {}; for (const pair of (request.headers.get("cookie") ?? "").split(";")) {