From 59f5477b8c12ce5ca089ed2d8a54cc64a5ac056d Mon Sep 17 00:00:00 2001 From: igoramf Date: Tue, 1 Sep 2026 07:59:26 -0300 Subject: [PATCH] fix(redirects): respect quoted CSV fields and read the type case-insensitively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent parsing bugs found while investigating a redirect loop; neither causes the loop, both corrupt real data. Quoted fields: `parseRedirectsCsv` split on every comma, but bulk exports quote rows whose query contains one: "https://site/relogios?map=category-1,category-2",/relogios,PERMANENT That row parsed as from=`/"https://site/relogios?map=category-1`, to=`category-2"`. In one production CSV, 643 of 3017 rows came out with a fragment of a query as their target (`to: "c"`, `to: "priceFrom"`). They are inert today only because the mangled source never matches a request. Type spelling: only lowercase `permanent` mapped to 301. Exports commonly write `PERMANENT`, so every one of that site's ~3000 redirects served 302 — a temporary redirect passes no ranking signal to the new URL, silently discarding the SEO value of the migration. Co-Authored-By: Claude Opus 5 (1M context) --- packages/blocks/src/sdk/redirects.ts | 52 +++++++++++++- .../src/sdk/redirectsCsvParsing.test.ts | 71 +++++++++++++++++++ 2 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 packages/blocks/src/sdk/redirectsCsvParsing.test.ts diff --git a/packages/blocks/src/sdk/redirects.ts b/packages/blocks/src/sdk/redirects.ts index 68de0919..96e829bc 100644 --- a/packages/blocks/src/sdk/redirects.ts +++ b/packages/blocks/src/sdk/redirects.ts @@ -106,7 +106,7 @@ export function loadRedirects(blocks: Record): RedirectMap { const redirect: Redirect = { from: normalizePath(entry.from), to: entry.to, - status: entry.type === "permanent" ? 301 : 302, + status: normalizeStatus(entry.type), }; if (redirect.from.includes("*")) { @@ -140,7 +140,7 @@ export function parseRedirectsCsv(csv: string): Redirect[] { const line = raw.trim(); if (!line || line.startsWith("#")) continue; - const parts = line.split(",").map((p) => p.trim()); + const parts = splitCsvLine(line); if (parts.length < 2) continue; const [from, to, type] = parts; @@ -152,7 +152,7 @@ export function parseRedirectsCsv(csv: string): Redirect[] { redirects.push({ from: normalizePath(from), to, - status: type === "permanent" || type === "301" ? 301 : 302, + status: normalizeStatus(type), }); } @@ -204,6 +204,52 @@ export function matchRedirect(pathname: string, map: RedirectMap): Redirect | nu // Helpers // ------------------------------------------------------------------------- +/** + * Split one CSV line on commas that are not inside a quoted field. + * + * Bulk redirect exports routinely hold VTEX URLs whose query contains commas + * (`?map=category-1,category-2`). Those rows are quoted, and a plain + * `split(",")` shreds them into rules with a truncated source and a fragment of + * the query as the target. + */ +function splitCsvLine(line: string): string[] { + const parts: string[] = []; + let current = ""; + let quoted = false; + + for (let i = 0; i < line.length; i++) { + const char = line[i]; + if (char === '"') { + // "" inside a quoted field is an escaped quote (RFC 4180). + if (quoted && line[i + 1] === '"') { + current += '"'; + i++; + } else { + quoted = !quoted; + } + } else if (char === "," && !quoted) { + parts.push(current.trim()); + current = ""; + } else { + current += char; + } + } + parts.push(current.trim()); + return parts; +} + +/** + * Redirect type -> HTTP status, case-insensitively. + * + * Exports commonly write `PERMANENT`. Matching only the lowercase spelling + * downgraded those rows to 302, and a temporary redirect passes no ranking + * signal to the new URL. + */ +function normalizeStatus(type?: string): 301 | 302 { + const t = type?.trim().toLowerCase(); + return t === "permanent" || t === "301" ? 301 : 302; +} + function normalizePath(path: string): string { let p = path.trim(); diff --git a/packages/blocks/src/sdk/redirectsCsvParsing.test.ts b/packages/blocks/src/sdk/redirectsCsvParsing.test.ts new file mode 100644 index 00000000..f659b9c8 --- /dev/null +++ b/packages/blocks/src/sdk/redirectsCsvParsing.test.ts @@ -0,0 +1,71 @@ +/** + * CSV parsing details that bulk redirect exports depend on: quoted fields and + * the spelling of the redirect type. + */ +import { describe, expect, it } from "vitest"; +import { loadRedirects, matchRedirect, parseRedirectsCsv } from "./redirects"; + +describe("quoted fields", () => { + it("does not split on a comma inside a quoted source", () => { + // Real VTEX export row: the query itself contains a comma. + const [redirect] = parseRedirectsCsv( + 'from,to,type\n"https://www.example.com/relogios?map=category-1,category-2",/relogios,PERMANENT\n', + ); + + expect(redirect).toMatchObject({ to: "/relogios", status: 301 }); + expect(redirect.from).toBe("/relogios"); + }); + + it("does not split on a comma inside a quoted target", () => { + const [redirect] = parseRedirectsCsv('from,to\n/old,"/new?map=a,b"\n'); + expect(redirect).toMatchObject({ from: "/old", to: "/new?map=a,b" }); + }); + + it("unescapes a doubled quote inside a quoted field", () => { + const [redirect] = parseRedirectsCsv('from,to\n/old,"/new?q=""x"""\n'); + expect(redirect.to).toBe('/new?q="x"'); + }); + + it("still handles rows with no quoting at all", () => { + const map = loadRedirects({}); + expect(parseRedirectsCsv("from,to,type\n/a,/b,permanent\n")).toEqual([ + { from: "/a", to: "/b", status: 301 }, + ]); + expect(map.exact.size).toBe(0); + }); + + it("cannot rescue an unquoted row whose query holds a comma", () => { + // Documents the boundary: quoting is what makes a comma literal. An export + // that omits the quotes is still shredded, and that is the export's bug. + const [redirect] = parseRedirectsCsv("from,to\n/x?map=a,b\n"); + expect(redirect).toMatchObject({ from: "/x?map=a", to: "b" }); + }); +}); + +describe("redirect type spelling", () => { + const statusOf = (type: string) => parseRedirectsCsv(`from,to,type\n/a,/b,${type}\n`)[0].status; + + it("reads the CSV type case-insensitively", () => { + expect(statusOf("PERMANENT")).toBe(301); + expect(statusOf("Permanent")).toBe(301); + expect(statusOf("permanent")).toBe(301); + expect(statusOf("301")).toBe(301); + }); + + it("still defaults to temporary", () => { + expect(statusOf("TEMPORARY")).toBe(302); + expect(statusOf("temporary")).toBe(302); + expect(statusOf("")).toBe(302); + expect(parseRedirectsCsv("from,to\n/a,/b\n")[0].status).toBe(302); + }); + + it("reads the CMS block type case-insensitively too", () => { + const map = loadRedirects({ + r: { + __resolveType: "website/loaders/redirect.ts", + redirect: { from: "/a", to: "/b", type: "PERMANENT" }, + }, + }); + expect(matchRedirect("/a", map)?.status).toBe(301); + }); +});