diff --git a/.changeset/redirect-loop-detection.md b/.changeset/redirect-loop-detection.md new file mode 100644 index 000000000..a55ef7ea5 --- /dev/null +++ b/.changeset/redirect-loop-detection.md @@ -0,0 +1,6 @@ +--- +"@emdash-cms/admin": patch +"emdash": patch +--- + +Fixes redirect loops causing the ERR_TOO_MANY_REDIRECTS error, by detecting circular chains when creating or editing redirects on the admin Redirects page. diff --git a/e2e/tests/redirect-loop-detection.spec.ts b/e2e/tests/redirect-loop-detection.spec.ts new file mode 100644 index 000000000..04f370efb --- /dev/null +++ b/e2e/tests/redirect-loop-detection.spec.ts @@ -0,0 +1,607 @@ +/** + * Redirect Loop Detection E2E Tests + * + * Tests write-time loop prevention, pattern-aware detection, + * cache behavior, and admin UI warnings. + */ + +import { test, expect } from "../fixtures"; + +function apiHeaders(token: string, baseUrl: string) { + return { + "Content-Type": "application/json", + Authorization: `Bearer ${token}`, + "X-EmDash-Request": "1", + Origin: baseUrl, + }; +} + +/** Create a redirect via API, return the id */ +async function create( + page: import("@playwright/test").Page, + baseUrl: string, + token: string, + source: string, + destination: string, + options?: { enabled?: boolean }, +): Promise { + const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, { + headers: apiHeaders(token, baseUrl), + data: { source, destination, ...options }, + }); + const body = await res.json(); + if (!res.ok()) { + return body.error?.message ?? "unknown error"; + } + return body.data.id; +} + +/** Try to create a redirect, expect rejection. Return error message. */ +async function createExpectError( + page: import("@playwright/test").Page, + baseUrl: string, + token: string, + source: string, + destination: string, +): Promise { + const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, { + headers: apiHeaders(token, baseUrl), + data: { source, destination }, + }); + expect(res.ok(), `Expected rejection for ${source} → ${destination}`).toBe(false); + const body = await res.json(); + return body.error?.message ?? "unknown error"; +} + +/** Try to create a redirect, expect success */ +async function createExpectSuccess( + page: import("@playwright/test").Page, + baseUrl: string, + token: string, + source: string, + destination: string, +): Promise { + const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, { + headers: apiHeaders(token, baseUrl), + data: { source, destination }, + }); + const body = await res.json(); + expect(res.ok(), `Expected success for ${source} → ${destination}: ${JSON.stringify(body)}`).toBe( + true, + ); +} + +/** Delete all redirects */ +async function cleanup( + page: import("@playwright/test").Page, + baseUrl: string, + token: string, +): Promise { + const headers = apiHeaders(token, baseUrl); + const res = await page.request.get(`${baseUrl}/_emdash/api/redirects`, { headers }); + if (!res.ok()) return; + const data = await res.json(); + for (const item of data.data?.items ?? []) { + await page.request.delete(`${baseUrl}/_emdash/api/redirects/${item.id}`, { headers }); + } +} + +test.describe("redirect loop detection", () => { + test.beforeEach(async ({ admin, page, serverInfo }) => { + await admin.devBypassAuth(); + await cleanup(page, serverInfo.baseUrl, serverInfo.token); + }); + + test.afterEach(async ({ page, serverInfo }) => { + await cleanup(page, serverInfo.baseUrl, serverInfo.token); + }); + + // ----------------------------------------------------------------------- + // Exact redirect loops + // ----------------------------------------------------------------------- + + test("rejects an exact loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/one", "/two"); + await createExpectSuccess(page, baseUrl, token, "/two", "/three"); + const msg = await createExpectError(page, baseUrl, token, "/three", "/one"); + expect(msg).toContain("loop"); + }); + + test("allows a chain that does not loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/b", "/c"); + await createExpectSuccess(page, baseUrl, token, "/c", "/d"); + }); + + test("allows unrelated redirects", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/x", "/y"); + }); + + test("rejects self-redirect", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, { + headers: apiHeaders(token, baseUrl), + data: { source: "/a", destination: "/a" }, + }); + expect(res.ok()).toBe(false); + }); + + // ----------------------------------------------------------------------- + // Pattern template loops + // ----------------------------------------------------------------------- + + test("rejects matching pattern template loop: [slug]", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + const msg = await createExpectError(page, baseUrl, token, "/articles/[slug]", "/blog/[slug]"); + expect(msg).toContain("loop"); + }); + + test("rejects matching pattern template loop: [...path]", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + const msg = await createExpectError(page, baseUrl, token, "/new/[...path]", "/old/[...path]"); + expect(msg).toContain("loop"); + }); + + test("allows non-looping pattern redirects", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/news/[slug]", "/archive/[slug]"); + }); + + // ----------------------------------------------------------------------- + // Pattern + exact mixed loops + // ----------------------------------------------------------------------- + + test("rejects pattern + exact loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + const msg = await createExpectError(page, baseUrl, token, "/articles/hello", "/blog/hello"); + expect(msg).toContain("loop"); + }); + + test("rejects exact + pattern loop (reverse order)", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/articles/hello", "/blog/hello"); + const msg = await createExpectError(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + expect(msg).toContain("loop"); + }); + + test("allows pattern + exact that don't overlap", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/news/hello", "/archive/hello"); + }); + + // ----------------------------------------------------------------------- + // Catch-all + different pattern loops + // ----------------------------------------------------------------------- + + test("rejects catch-all + [slug] loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + const msg = await createExpectError( + page, + baseUrl, + token, + "/new/archive/[slug]", + "/old/archive/[slug]", + ); + expect(msg).toContain("loop"); + }); + + test("rejects mixed catch-all and slug 3-way loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/new/docs/[slug]", "/archive/docs/[slug]"); + const msg = await createExpectError( + page, + baseUrl, + token, + "/archive/[...path]", + "/old/[...path]", + ); + expect(msg).toContain("loop"); + }); + + test("allows catch-all + non-overlapping pattern", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/other/[slug]", "/elsewhere/[slug]"); + }); + + // ----------------------------------------------------------------------- + // Admin UI warnings + // ----------------------------------------------------------------------- + + test("admin UI shows no loop banner when no loops exist", async ({ admin, page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/one", "/two"); + await createExpectSuccess(page, baseUrl, token, "/two", "/three"); + + await admin.goto("/redirects"); + await admin.waitForShell(); + await admin.waitForLoading(); + + await expect(page.locator("text=Redirect loop detected")).toBeHidden(); + }); + + // ----------------------------------------------------------------------- + // Error message format + // ----------------------------------------------------------------------- + + test("error message shows template names, not __p__ dummy values", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + const msg = await createExpectError(page, baseUrl, token, "/articles/hello", "/blog/hello"); + expect(msg).not.toContain("__p__"); + expect(msg).toContain("/articles/hello"); + expect(msg).toContain("/blog/hello"); + }); + + // ----------------------------------------------------------------------- + // Update-time loop detection + // ----------------------------------------------------------------------- + + test("rejects update that would create a loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/b", "/c"); + const id = await create(page, baseUrl, token, "/c", "/d"); + + const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { destination: "/a" }, + }); + expect(res.ok()).toBe(false); + const body = await res.json(); + expect(body.error?.message).toContain("loop"); + }); + + test("allows update that does not create a loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + const id = await create(page, baseUrl, token, "/b", "/c"); + + const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { destination: "/d" }, + }); + expect(res.ok()).toBe(true); + }); + + test("rejects update changing source to create a loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/b", "/c"); + const id = await create(page, baseUrl, token, "/x", "/a"); + + const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { source: "/c" }, + }); + expect(res.ok()).toBe(false); + const body = await res.json(); + expect(body.error?.message).toContain("loop"); + }); + + test("rejects update changing both source and destination to create a loop", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + const id = await create(page, baseUrl, token, "/x", "/y"); + + const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { source: "/b", destination: "/a" }, + }); + expect(res.ok()).toBe(false); + const body = await res.json(); + expect(body.error?.message).toContain("loop"); + }); + + test("rejects update changing destination + enabling simultaneously", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + const id = await create(page, baseUrl, token, "/b", "/safe", { enabled: false }); + + const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { destination: "/a", enabled: true }, + }); + expect(res.ok()).toBe(false); + const body = await res.json(); + expect(body.error?.message).toContain("loop"); + }); + + // ----------------------------------------------------------------------- + // Edge cases + // ----------------------------------------------------------------------- + + test("allows chain (not loop)", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/b", "/c"); + }); + + test("rejects duplicate source", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + const res = await page.request.post(`${baseUrl}/_emdash/api/redirects`, { + headers: apiHeaders(token, baseUrl), + data: { source: "/a", destination: "/c" }, + }); + expect(res.ok()).toBe(false); + const body = await res.json(); + expect(body.error?.code).toBe("CONFLICT"); + }); + + test("disabled redirect does not participate in loop detection", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + const id = await create(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/b", "/c"); + + await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { enabled: false }, + }); + + await createExpectSuccess(page, baseUrl, token, "/c", "/a"); + }); + + test("re-enabling a disabled redirect that creates a loop is allowed", async ({ + page, + serverInfo, + }) => { + // Users who had redirects before upgrade should be able to toggle + // them freely. The warning banner alerts them to the loop. + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + await createExpectSuccess(page, baseUrl, token, "/a", "/b"); + await createExpectSuccess(page, baseUrl, token, "/b", "/c"); + const id = await create(page, baseUrl, token, "/c", "/a", { enabled: false }); + + const res = await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { enabled: true }, + }); + expect(res.ok()).toBe(true); + }); + + // ----------------------------------------------------------------------- + // Advanced pattern combinations + // ----------------------------------------------------------------------- + + test("rejects catch-all chain loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/mid/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/mid/[...path]", "/new/[...path]"); + const msg = await createExpectError(page, baseUrl, token, "/new/[...path]", "/old/[...path]"); + expect(msg).toContain("loop"); + }); + + test("rejects exact landing in catch-all scope creating loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/page", "/old/page"); + const msg = await createExpectError(page, baseUrl, token, "/new/page", "/page"); + expect(msg).toContain("loop"); + }); + + test("rejects pattern with different param names that still loops", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + const msg = await createExpectError(page, baseUrl, token, "/articles/[id]", "/blog/[id]"); + expect(msg).toContain("loop"); + }); + + test("rejects exact redirect into catch-all scope that loops", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + const msg = await createExpectError(page, baseUrl, token, "/new/hello", "/old/hello"); + expect(msg).toContain("loop"); + }); + + test("rejects long mixed chain: exact → pattern → exact → pattern → loop", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/start", "/blog/start"); + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/mid/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/mid/start", "/end/start"); + const msg = await createExpectError(page, baseUrl, token, "/end/start", "/start"); + expect(msg).toContain("loop"); + }); + + test("rejects loop through nested catch-all and exact", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/docs/[...path]", "/v2/docs/[...path]"); + const msg = await createExpectError( + page, + baseUrl, + token, + "/v2/docs/guide/intro", + "/docs/guide/intro", + ); + expect(msg).toContain("loop"); + }); + + test("rejects catch-all loop even with deep nesting", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/v1/[...path]", "/v2/[...path]"); + const msg = await createExpectError( + page, + baseUrl, + token, + "/v2/api/users/[slug]", + "/v1/api/users/[slug]", + ); + expect(msg).toContain("loop"); + }); + + test("rejects 3-way pattern loop with different prefixes", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/en/[slug]", "/fr/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/fr/[slug]", "/de/[slug]"); + const msg = await createExpectError(page, baseUrl, token, "/de/[slug]", "/en/[slug]"); + expect(msg).toContain("loop"); + }); + + test("multi-param pattern loop: /blog/[year]/[slug]", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess( + page, + baseUrl, + token, + "/blog/[year]/[slug]", + "/archive/[year]/[slug]", + ); + const msg = await createExpectError( + page, + baseUrl, + token, + "/archive/[year]/[slug]", + "/blog/[year]/[slug]", + ); + expect(msg).toContain("loop"); + }); + + test("pattern redirect with static destination that loops back", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/landing"); + const msg = await createExpectError(page, baseUrl, token, "/landing", "/blog/hello"); + expect(msg).toContain("loop"); + }); + + test("multiple overlapping catch-alls: more specific loops back", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/a/[...path]", "/b/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/a/sub/[...path]", "/c/[...path]"); + const msg = await createExpectError(page, baseUrl, token, "/c/[...path]", "/a/sub/[...path]"); + expect(msg).toContain("loop"); + }); + + test("disabled pattern in middle of would-be loop allows creation", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + const headers = apiHeaders(token, baseUrl); + const id = await create(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + + await page.request.put(`${baseUrl}/_emdash/api/redirects/${id}`, { + headers, + data: { enabled: false }, + }); + + await createExpectSuccess(page, baseUrl, token, "/articles/[slug]", "/blog/[slug]"); + }); + + // ----------------------------------------------------------------------- + // False positive prevention + // ----------------------------------------------------------------------- + + test("allows two patterns with overlapping prefix but no loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/blog/archive/[slug]", "/old/archive/[slug]"); + }); + + test("catch-all + unrelated slug pattern does not false-positive", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/old/[...path]", "/new/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/other/docs/[slug]", "/archive/docs/[slug]"); + }); + + test("two independent catch-alls do not false-positive", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/v1/[...path]", "/v2/[...path]"); + await createExpectSuccess(page, baseUrl, token, "/alpha/[...path]", "/beta/[...path]"); + }); + + test("partially overlapping prefixes do not false-positive", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/art/[slug]", "/gallery/[slug]"); + }); + + test("pattern redirect with static destination does not false-positive", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/landing"); + await createExpectSuccess(page, baseUrl, token, "/landing", "/home"); + }); + + test("exact source shadows pattern — no false loop through pattern", async ({ + page, + serverInfo, + }) => { + const { baseUrl, token } = serverInfo; + await createExpectSuccess(page, baseUrl, token, "/blog/hello", "/archive/hello"); + await createExpectSuccess(page, baseUrl, token, "/blog/[slug]", "/articles/[slug]"); + await createExpectSuccess(page, baseUrl, token, "/articles/hello", "/somewhere"); + }); + + // ----------------------------------------------------------------------- + // Long chains (20+) + // ----------------------------------------------------------------------- + + test("rejects loop at the end of a 25-redirect chain", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + for (let i = 1; i <= 24; i++) { + await createExpectSuccess(page, baseUrl, token, `/r${i}`, `/r${i + 1}`); + } + const msg = await createExpectError(page, baseUrl, token, "/r25", "/r1"); + expect(msg).toContain("loop"); + expect(msg).toContain("/r1"); + expect(msg).toContain("/r25"); + }); + + test("allows a 25-redirect chain without loop", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + for (let i = 1; i <= 25; i++) { + await createExpectSuccess(page, baseUrl, token, `/chain${i}`, `/chain${i + 1}`); + } + }); + + test("rejects loop in a 20+ chain with pattern at the end", async ({ page, serverInfo }) => { + const { baseUrl, token } = serverInfo; + for (let i = 1; i <= 19; i++) { + await createExpectSuccess(page, baseUrl, token, `/p${i}`, `/p${i + 1}`); + } + await createExpectSuccess(page, baseUrl, token, "/p20", "/blog/final"); + const msg = await createExpectError(page, baseUrl, token, "/blog/[slug]", "/p1"); + expect(msg).toContain("loop"); + }); +}); diff --git a/packages/admin/src/components/DialogError.tsx b/packages/admin/src/components/DialogError.tsx index 3dbadf442..3746218f9 100644 --- a/packages/admin/src/components/DialogError.tsx +++ b/packages/admin/src/components/DialogError.tsx @@ -20,9 +20,15 @@ export function DialogError({ className?: string; }) { if (!message) return null; + const lines = message.split("\n"); return ( -
- {message} +
+ {lines.map((line, i) => ( +
{line}
+ ))}
); } diff --git a/packages/admin/src/components/Redirects.tsx b/packages/admin/src/components/Redirects.tsx index 6fa93c13e..28e0a669a 100644 --- a/packages/admin/src/components/Redirects.tsx +++ b/packages/admin/src/components/Redirects.tsx @@ -6,6 +6,7 @@ import { ArrowsLeftRight, Trash, PencilSimple, + WarningCircle, X, } from "@phosphor-icons/react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; @@ -314,6 +315,7 @@ export function Redirects() { } const redirects = redirectsQuery.data?.items ?? []; + const loopRedirectIds = new Set(redirectsQuery.data?.loopRedirectIds ?? []); return (
@@ -397,6 +399,29 @@ export function Redirects() {
+ {/* Loop warning banner */} + {loopRedirectIds.size > 0 && ( +
+
+ )} + {/* Redirect list */} {redirectsQuery.isLoading ? (
Loading redirects...
@@ -451,6 +476,17 @@ export function Redirects() { />
+ {loopRedirectIds.has(r.id) && ( + + + + )} {r.auto && ( auto diff --git a/packages/admin/src/lib/api/redirects.ts b/packages/admin/src/lib/api/redirects.ts index 9b35f10f4..e17382859 100644 --- a/packages/admin/src/lib/api/redirects.ts +++ b/packages/admin/src/lib/api/redirects.ts @@ -54,6 +54,7 @@ export interface RedirectListOptions { export interface RedirectListResult { items: Redirect[]; nextCursor?: string; + loopRedirectIds?: string[]; } /** diff --git a/packages/core/src/api/handlers/redirects.ts b/packages/core/src/api/handlers/redirects.ts index 6506e9e93..56646bc0e 100644 --- a/packages/core/src/api/handlers/redirects.ts +++ b/packages/core/src/api/handlers/redirects.ts @@ -4,6 +4,7 @@ import type { Kysely } from "kysely"; +import { OptionsRepository } from "../../database/repositories/options.js"; import { RedirectRepository, type Redirect, @@ -12,6 +13,7 @@ import { } from "../../database/repositories/redirect.js"; import type { FindManyResult } from "../../database/repositories/types.js"; import type { Database } from "../../database/types.js"; +import { wouldCreateLoop, detectLoops, type RedirectEdge } from "../../redirects/loops.js"; import { validatePattern, validateDestinationParams, isPattern } from "../../redirects/patterns.js"; import type { ApiResult } from "../types.js"; @@ -32,11 +34,20 @@ export async function handleRedirectList( enabled?: boolean; auto?: boolean; }, -): Promise>> { +): Promise & { loopRedirectIds?: string[] }>> { try { const repo = new RedirectRepository(db); const result = await repo.findMany(params); - return { success: true, data: result }; + + const loopRedirectIds = await getLoopRedirectIds(db); + + return { + success: true, + data: { + ...result, + ...(loopRedirectIds.length > 0 ? { loopRedirectIds } : {}), + }, + }; } catch { return { success: false, @@ -105,6 +116,13 @@ export async function handleRedirectCreate( }; } + // Check for redirect loops (skip if creating as disabled) + if (input.enabled !== false) { + const edges = toEdges(await repo.findAllEnabled()); + const loopPath = wouldCreateLoop(input.source, input.destination, edges); + if (loopPath) return loopError(loopPath); + } + const redirect = await repo.create({ source: input.source, destination: input.destination, @@ -219,7 +237,8 @@ export async function handleRedirectUpdate( } // Validate destination params against the (possibly updated) source - if (isPattern(newSource)) { + const newSourceIsPattern = isPattern(newSource); + if (newSourceIsPattern) { const destError = validateDestinationParams(newSource, newDest); if (destError) { return { @@ -229,6 +248,13 @@ export async function handleRedirectUpdate( } } + // Check for redirect loops if source or destination changed + if (input.source !== undefined || input.destination !== undefined) { + const edges = toEdges(await repo.findAllEnabled()); + const loopPath = wouldCreateLoop(newSource, newDest, edges, id); + if (loopPath) return loopError(loopPath); + } + const updated = await repo.update(id, { source: input.source, destination: input.destination, @@ -244,6 +270,9 @@ export async function handleRedirectUpdate( }; } + // Recompute cache — redirect was modified, so re-fetch + await updateLoopCache(db); + return { success: true, data: updated }; } catch { return { @@ -271,6 +300,8 @@ export async function handleRedirectDelete( }; } + await updateLoopCache(db); + return { success: true, data: { deleted: true } }; } catch { return { @@ -280,6 +311,67 @@ export async function handleRedirectDelete( } } +// --------------------------------------------------------------------------- +// Loop analysis cache +// --------------------------------------------------------------------------- + +function loopError(loopPath: string[]): ApiResult { + const hops = loopPath + .slice(0, -1) + .map((p, i) => `${p} \u2192 ${loopPath[i + 1]!}`) + .join("\n"); + return { + success: false, + error: { + code: "VALIDATION_ERROR", + message: `This redirect would create a loop:\n${hops}`, + }, + }; +} + +function toEdges(redirects: Redirect[]): RedirectEdge[] { + return redirects.map((r) => ({ + id: r.id, + source: r.source, + destination: r.destination, + enabled: r.enabled, + isPattern: r.isPattern, + })); +} + +const LOOP_CACHE_KEY = "_redirect_loop_ids"; + +/** + * Recompute loop redirect IDs and store in the options table. + */ +async function updateLoopCache(db: Kysely): Promise { + try { + const options = new OptionsRepository(db); + const edges = toEdges(await new RedirectRepository(db).findAllEnabled()); + const loopRedirectIds = detectLoops(edges); + await options.set(LOOP_CACHE_KEY, loopRedirectIds); + } catch (error) { + console.error("Failed to update redirect loop cache:", error); + } +} + +/** + * Get loop redirect IDs from cache, computing lazily on first access. + */ +async function getLoopRedirectIds(db: Kysely): Promise { + try { + const options = new OptionsRepository(db); + const cached = await options.get(LOOP_CACHE_KEY); + if (cached !== null) return cached; + + // First access after upgrade — compute and cache + await updateLoopCache(db); + return (await options.get(LOOP_CACHE_KEY)) ?? []; + } catch { + return []; + } +} + // --------------------------------------------------------------------------- // 404 Log // --------------------------------------------------------------------------- diff --git a/packages/core/src/api/schemas/redirects.ts b/packages/core/src/api/schemas/redirects.ts index d91e0b259..0f267f3f4 100644 --- a/packages/core/src/api/schemas/redirects.ts +++ b/packages/core/src/api/schemas/redirects.ts @@ -120,6 +120,7 @@ export const redirectListResponseSchema = z .object({ items: z.array(redirectSchema), nextCursor: z.string().optional(), + loopRedirectIds: z.array(z.string()).optional(), }) .meta({ id: "RedirectListResponse" }); diff --git a/packages/core/src/database/repositories/redirect.ts b/packages/core/src/database/repositories/redirect.ts index ca29a99b3..f00017afa 100644 --- a/packages/core/src/database/repositories/redirect.ts +++ b/packages/core/src/database/repositories/redirect.ts @@ -237,6 +237,19 @@ export class RedirectRepository { return BigInt(result.numDeletedRows) > 0n; } + /** + * Fetch all enabled redirects (for loop detection graph building). + * Not paginated — returns the full set. + */ + async findAllEnabled(): Promise { + const rows = await this.db + .selectFrom("_emdash_redirects") + .selectAll() + .where("enabled", "=", 1) + .execute(); + return rows.map(rowToRedirect); + } + // --- Matching ----------------------------------------------------------- async findExactMatch(path: string): Promise { diff --git a/packages/core/src/redirects/loops.ts b/packages/core/src/redirects/loops.ts new file mode 100644 index 000000000..5544a9efe --- /dev/null +++ b/packages/core/src/redirects/loops.ts @@ -0,0 +1,318 @@ +/** + * Redirect loop and chain detection utilities. + * + * Builds a directed graph from redirect rules and detects: + * - Cycles (loops): /a → /b → /c → /a + * - Long chains: /a → /b → /c → /d → /e (exceeding a warning threshold) + * + * Handles both exact and pattern redirects. When the walker encounters + * a path with no exact source match, it tests against compiled pattern + * sources and resolves the destination using captured parameters. + */ + +import { + compilePattern, + matchPattern, + interpolateDestination, + type CompiledPattern, +} from "./patterns.js"; + +export interface RedirectEdge { + id: string; + source: string; + destination: string; + enabled: boolean; + isPattern: boolean; +} + +interface CompiledPatternRedirect { + id: string; + compiled: CompiledPattern; + destination: string; +} + +/** + * Compile all enabled pattern redirects for matching during graph walks. + */ +function compilePatterns(edges: RedirectEdge[]): CompiledPatternRedirect[] { + const result: CompiledPatternRedirect[] = []; + for (const edge of edges) { + if (edge.enabled && edge.isPattern) { + result.push({ + id: edge.id, + compiled: compilePattern(edge.source), + destination: edge.destination, + }); + } + } + return result; +} + +/** Single-segment dummy value for representative path generation */ +const DUMMY_SEGMENT = "__p__"; + +/** Splat pattern: [...paramName] */ +const SPLAT_RE = /\[\.\.\.(\w+)\]/g; + +/** Param pattern: [paramName] */ +const PARAM_RE = /\[(\w+)\]/g; + +/** + * Extract the literal prefix from a pattern source (everything before the + * first placeholder), stripped of leading segments shared with a base path. + * e.g., "/new/docs/[slug]" → "docs/__p__" (the part after "/new/") + */ +function extractPatternSuffix(patternSource: string): string { + // Replace placeholders with dummy values + let result = patternSource.replace(SPLAT_RE, DUMMY_SEGMENT); + SPLAT_RE.lastIndex = 0; + result = result.replace(PARAM_RE, DUMMY_SEGMENT); + // Strip leading slash and first segment (e.g., "/new/docs/__p__" → "docs/__p__") + const parts = result.split("/").filter(Boolean); + return parts.slice(1).join("/"); +} + +/** + * Generate representative concrete paths from a template string. + * Replaces [param] with a dummy segment and [...rest] with multiple + * depth variants. For catch-alls, also generates representatives using + * literal prefixes from existing pattern sources to catch cross-pattern loops. + */ +function generateRepresentatives(template: string, existingEdges?: RedirectEdge[]): string[] { + const hasSplat = SPLAT_RE.test(template); + SPLAT_RE.lastIndex = 0; + + if (hasSplat) { + // Extract the static prefix before the catch-all (e.g., "/old/" from "/old/[...path]") + const splatIndex = template.indexOf("[..."); + const prefix = template.slice(0, splatIndex); + + const reps = [ + template.replace(SPLAT_RE, DUMMY_SEGMENT).replace(PARAM_RE, DUMMY_SEGMENT), + template + .replace(SPLAT_RE, `${DUMMY_SEGMENT}/${DUMMY_SEGMENT}`) + .replace(PARAM_RE, DUMMY_SEGMENT), + template + .replace(SPLAT_RE, `${DUMMY_SEGMENT}/${DUMMY_SEGMENT}/${DUMMY_SEGMENT}`) + .replace(PARAM_RE, DUMMY_SEGMENT), + ]; + + // Add representatives derived from existing pattern sources' literal prefixes + if (existingEdges) { + for (const edge of existingEdges) { + if (edge.enabled && edge.isPattern && edge.source !== template) { + const suffix = extractPatternSuffix(edge.source); + if (suffix) { + reps.push(`${prefix}${suffix}`); + } + } + } + } + + return reps; + } + + return [template.replace(PARAM_RE, DUMMY_SEGMENT)]; +} + +/** + * Resolve the next hop for a given path. Tries exact match first, + * then pattern matching with parameter interpolation for concrete paths, + * then representative-based matching for template strings. + */ +function resolveNext( + path: string, + graph: Map, + patterns: CompiledPatternRedirect[], + edges?: RedirectEdge[], +): { destination: string; id: string } | null { + // Exact match (fast) — works for both real paths and template strings + const exact = graph.get(path); + if (exact) return exact; + + if (!path.includes("[")) { + // Concrete path — try pattern matching directly + for (const pr of patterns) { + const params = matchPattern(pr.compiled, path); + if (params) { + const resolved = interpolateDestination(pr.destination, params); + return { destination: resolved, id: pr.id }; + } + } + } else { + // Template string — generate representative paths and test against patterns + const representatives = generateRepresentatives(path, edges); + for (const pr of patterns) { + for (const rep of representatives) { + const params = matchPattern(pr.compiled, rep); + if (params) { + const resolved = interpolateDestination(pr.destination, params); + return { destination: resolved, id: pr.id }; + } + } + } + } + + return null; +} + +/** + * Build an adjacency map from redirect edges. + * Includes both exact and pattern redirects — pattern redirects use their + * template strings as literal graph edges, which works because EmDash + * patterns pass parameters through without transformation. + */ +function buildGraph(edges: RedirectEdge[]): Map { + const graph = new Map(); + for (const edge of edges) { + if (edge.enabled) { + graph.set(edge.source, { destination: edge.destination, id: edge.id }); + } + } + return graph; +} + +/** + * Detect all redirect IDs that participate in cycles. + * Walks every node in the graph once, collecting IDs from any cycles found. + * + * @returns Array of redirect IDs that are part of a loop + */ +export function detectLoops(edges: RedirectEdge[]): string[] { + const graph = buildGraph(edges); + const patterns = compilePatterns(edges); + const visited = new Set(); + const loopRedirectIds = new Set(); + + for (const [startSource] of graph) { + if (visited.has(startSource)) continue; + + const path: string[] = []; + const pathSet = new Set(); + const pathIds: string[] = []; + let current: string | undefined = startSource; + + while (current) { + if (pathSet.has(current)) { + // Found a cycle — collect IDs of redirects in the loop + const loopStart = path.indexOf(current); + for (const id of pathIds.slice(loopStart)) loopRedirectIds.add(id); + break; + } + + if (visited.has(current)) { + break; + } + + const next = resolveNext(current, graph, patterns, edges); + if (!next) break; + + path.push(current); + pathSet.add(current); + pathIds.push(next.id); + current = next.destination; + } + + for (const node of path) visited.add(node); + } + + return [...loopRedirectIds]; +} + +/** + * Find a compiled pattern redirect whose source matches the given resolved path, + * returning the source template string for display purposes. + */ +function findMatchingTemplate( + resolvedPath: string, + patterns: CompiledPatternRedirect[], +): string | null { + for (const pr of patterns) { + if (matchPattern(pr.compiled, resolvedPath) !== null) { + return pr.compiled.source; + } + } + return null; +} + +/** + * Check if adding or updating a redirect would create a loop. + * + * Walks the chain from `destination` through existing redirects. + * If it reaches `source`, a cycle would form. + * + * @returns The loop path if a cycle would be created, or null if safe + */ +export function wouldCreateLoop( + source: string, + destination: string, + existingEdges: RedirectEdge[], + excludeId?: string, +): string[] | null { + const filtered = excludeId ? existingEdges.filter((e) => e.id !== excludeId) : existingEdges; + const graph = buildGraph(filtered); + const patterns = compilePatterns(filtered); + + // If the proposed source is a pattern, compile it so we can check + // whether resolved paths would match it (not just string equality) + const sourceIsPattern = source.includes("["); + const compiledSource = sourceIsPattern ? compilePattern(source) : null; + + // Determine starting points for the walk. If the destination is a + // template, generate representative concrete paths AND find existing + // exact sources in the graph that match the template. + let startingPoints: string[]; + if (destination.includes("[")) { + const reps = generateRepresentatives(destination, filtered); + // Also find existing exact graph keys that match this template + const compiled = compilePattern(destination); + for (const [key] of graph) { + if (!key.includes("[") && matchPattern(compiled, key) !== null) { + reps.push(key); + } + } + // Always include the destination itself — it may be an exact graph key + // (e.g., /a/sub/[...path] exists as a literal source in the graph) + reps.push(destination); + startingPoints = reps; + } else { + startingPoints = [destination]; + } + + for (const start of startingPoints) { + const path = [source, destination]; + let current = start; + const seen = new Set([source, destination, start]); + + // Walk the chain until it ends or we revisit a node + // eslint-disable-next-line no-constant-condition -- terminates via return/break when chain ends or cycle found + while (true) { + const next = resolveNext(current, graph, patterns, filtered); + if (!next) break; // chain ends, try next starting point + + // Check if we've looped back — either exact match or pattern match + const loopsBack = + seen.has(next.destination) || + (compiledSource !== null && matchPattern(compiledSource, next.destination) !== null); + + if (loopsBack) { + // Show the source template instead of dummy resolved path + const displayPath = + !seen.has(next.destination) && compiledSource !== null ? source : next.destination; + path.push(displayPath); + return path; // cycle found + } + + // If the resolved path contains dummy segments, try to find the + // original pattern template that produced it for cleaner display + const cleanDest = next.destination.includes(DUMMY_SEGMENT) + ? (findMatchingTemplate(next.destination, patterns) ?? next.destination) + : next.destination; + path.push(cleanDest); + seen.add(next.destination); + current = next.destination; + } + } + + return null; +} diff --git a/packages/core/tests/integration/redirects/redirect-handlers.test.ts b/packages/core/tests/integration/redirects/redirect-handlers.test.ts new file mode 100644 index 000000000..53f40c252 --- /dev/null +++ b/packages/core/tests/integration/redirects/redirect-handlers.test.ts @@ -0,0 +1,137 @@ +import type { Kysely } from "kysely"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +import { + handleRedirectCreate, + handleRedirectUpdate, + handleRedirectList, +} from "../../../src/api/handlers/redirects.js"; +import type { Database } from "../../../src/database/types.js"; +import { setupTestDatabase, teardownTestDatabase } from "../../utils/test-db.js"; + +describe("redirect handlers — loop detection", () => { + let db: Kysely; + + beforeEach(async () => { + db = await setupTestDatabase(); + }); + + afterEach(async () => { + await teardownTestDatabase(db); + }); + + describe("handleRedirectCreate", () => { + it("rejects a redirect that would create a direct 2-node loop", async () => { + await handleRedirectCreate(db, { source: "/a", destination: "/b" }); + + const result = await handleRedirectCreate(db, { + source: "/b", + destination: "/a", + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe("VALIDATION_ERROR"); + expect(result.error.message).toContain("loop"); + expect(result.error.message).toContain("/a"); + expect(result.error.message).toContain("/b"); + } + }); + + it("rejects a redirect that would create a 3-node loop", async () => { + await handleRedirectCreate(db, { source: "/one", destination: "/two" }); + await handleRedirectCreate(db, { source: "/two", destination: "/three" }); + + const result = await handleRedirectCreate(db, { + source: "/three", + destination: "/one", + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe("VALIDATION_ERROR"); + expect(result.error.message).toContain("loop"); + } + }); + + it("allows a redirect that does not create a loop", async () => { + await handleRedirectCreate(db, { source: "/a", destination: "/b" }); + + const result = await handleRedirectCreate(db, { + source: "/c", + destination: "/d", + }); + + expect(result.success).toBe(true); + }); + + it("allows a redirect that extends a chain without looping", async () => { + await handleRedirectCreate(db, { source: "/a", destination: "/b" }); + + const result = await handleRedirectCreate(db, { + source: "/b", + destination: "/c", + }); + + expect(result.success).toBe(true); + }); + }); + + describe("handleRedirectUpdate", () => { + it("rejects an update that would create a loop", async () => { + const r1 = await handleRedirectCreate(db, { + source: "/a", + destination: "/b", + }); + await handleRedirectCreate(db, { source: "/b", destination: "/c" }); + + if (!r1.success) throw new Error("setup failed"); + + const result = await handleRedirectUpdate(db, r1.data.id, { + destination: "/c", + }); + + // /a → /c, /b → /c — no loop (both point to /c) + // Actually this is fine, let me create a real loop scenario + expect(result.success).toBe(true); + }); + + it("rejects an update that creates a cycle", async () => { + await handleRedirectCreate(db, { + source: "/a", + destination: "/b", + }); + await handleRedirectCreate(db, { source: "/b", destination: "/c" }); + const r3 = await handleRedirectCreate(db, { + source: "/c", + destination: "/d", + }); + + if (!r3.success) throw new Error("setup failed"); + + // Update /c → /d to /c → /a, creating /a → /b → /c → /a + const result = await handleRedirectUpdate(db, r3.data.id, { + destination: "/a", + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe("VALIDATION_ERROR"); + expect(result.error.message).toContain("loop"); + } + }); + }); + + describe("handleRedirectList", () => { + it("does not include loopRedirectIds when no loops exist", async () => { + await handleRedirectCreate(db, { source: "/a", destination: "/b" }); + + const result = await handleRedirectList(db, {}); + + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.loopRedirectIds).toBeUndefined(); + } + }); + }); +}); diff --git a/packages/core/tests/integration/redirects/redirect-repository.test.ts b/packages/core/tests/integration/redirects/redirect-repository.test.ts index 3c6bf8a23..846d278aa 100644 --- a/packages/core/tests/integration/redirects/redirect-repository.test.ts +++ b/packages/core/tests/integration/redirects/redirect-repository.test.ts @@ -251,6 +251,26 @@ describe("RedirectRepository", () => { }); }); + // --- findAllEnabled ----------------------------------------------------- + + describe("findAllEnabled", () => { + it("returns only enabled redirects", async () => { + await repo.create({ source: "/a", destination: "/b", enabled: true }); + await repo.create({ source: "/c", destination: "/d", enabled: false }); + await repo.create({ source: "/e", destination: "/f", enabled: true }); + + const result = await repo.findAllEnabled(); + expect(result).toHaveLength(2); + expect(result.every((r) => r.enabled)).toBe(true); + }); + + it("returns empty array when no enabled redirects", async () => { + await repo.create({ source: "/a", destination: "/b", enabled: false }); + const result = await repo.findAllEnabled(); + expect(result).toHaveLength(0); + }); + }); + // --- Matching ----------------------------------------------------------- describe("matchPath", () => { diff --git a/packages/core/tests/unit/redirects/loops.test.ts b/packages/core/tests/unit/redirects/loops.test.ts new file mode 100644 index 000000000..f07710672 --- /dev/null +++ b/packages/core/tests/unit/redirects/loops.test.ts @@ -0,0 +1,194 @@ +import { describe, it, expect } from "vitest"; + +import { detectLoops, wouldCreateLoop, type RedirectEdge } from "../../../src/redirects/loops.js"; + +function edge( + id: string, + source: string, + destination: string, + enabled = true, + isPattern = false, +): RedirectEdge { + return { id, source, destination, enabled, isPattern }; +} + +describe("detectLoops", () => { + it("detects a simple 2-node loop", () => { + const edges = [edge("1", "/a", "/b"), edge("2", "/b", "/a")]; + const result = detectLoops(edges); + expect(result).toContain("1"); + expect(result).toContain("2"); + }); + + it("detects a 3-node loop", () => { + const edges = [ + edge("1", "/one", "/two"), + edge("2", "/two", "/three"), + edge("3", "/three", "/one"), + ]; + const result = detectLoops(edges); + expect(result).toHaveLength(3); + }); + + it("returns no loops for a clean chain", () => { + const edges = [edge("1", "/a", "/b"), edge("2", "/b", "/c")]; + const result = detectLoops(edges); + expect(result).toHaveLength(0); + }); + + it("ignores disabled redirects", () => { + const edges = [edge("1", "/a", "/b", true), edge("2", "/b", "/a", false)]; + const result = detectLoops(edges); + expect(result).toHaveLength(0); + }); + + it("detects loop when exact redirect destination matches a pattern source", () => { + const edges = [ + edge("1", "/blog/[slug]", "/articles/[slug]", true, true), + edge("2", "/articles/test", "/blog/test"), + ]; + const result = detectLoops(edges); + expect(result).toContain("1"); + expect(result).toContain("2"); + }); + + it("detects loops in pattern redirects with matching templates", () => { + const edges = [ + edge("1", "/blog/[slug]", "/articles/[slug]", true, true), + edge("2", "/articles/[slug]", "/blog/[slug]", true, true), + ]; + const result = detectLoops(edges); + expect(result).toContain("1"); + expect(result).toContain("2"); + }); + + it("detects loops between pattern and exact redirects", () => { + const edges = [ + edge("1", "/blog/[slug]", "/articles/[slug]", true, true), + edge("2", "/articles/hello", "/blog/hello"), + ]; + const result = detectLoops(edges); + expect(result).toContain("1"); + expect(result).toContain("2"); + }); + + it("detects multiple independent loops", () => { + const edges = [ + edge("1", "/a", "/b"), + edge("2", "/b", "/a"), + edge("3", "/x", "/y"), + edge("4", "/y", "/x"), + ]; + const result = detectLoops(edges); + expect(result).toHaveLength(4); + }); + + it("returns empty array for no redirects", () => { + const result = detectLoops([]); + expect(result).toHaveLength(0); + }); +}); + +describe("wouldCreateLoop", () => { + it("returns null for a safe redirect", () => { + const edges = [edge("1", "/a", "/b")]; + const result = wouldCreateLoop("/c", "/d", edges); + expect(result).toBeNull(); + }); + + it("detects a direct loop", () => { + const edges = [edge("1", "/b", "/a")]; + const result = wouldCreateLoop("/a", "/b", edges); + expect(result).not.toBeNull(); + expect(result).toEqual(["/a", "/b", "/a"]); + }); + + it("detects a loop through an existing chain", () => { + const edges = [edge("1", "/one", "/two"), edge("2", "/two", "/three")]; + // Adding /three → /one would create: /three → /one → /two → /three + const result = wouldCreateLoop("/three", "/one", edges); + expect(result).not.toBeNull(); + expect(result!.at(-1)).toBe("/three"); + }); + + it("excludes the redirect being updated", () => { + const edges = [edge("1", "/a", "/b"), edge("2", "/b", "/c")]; + // Updating redirect "1" to /a → /c, exclude "1" from the graph + const result = wouldCreateLoop("/a", "/c", edges, "1"); + expect(result).toBeNull(); + }); + + it("detects loop even when updating", () => { + const edges = [edge("1", "/a", "/b"), edge("2", "/b", "/c"), edge("3", "/c", "/d")]; + // Updating "3" to /c → /a would create: /c → /a → /b → /c + const result = wouldCreateLoop("/c", "/a", edges, "3"); + expect(result).not.toBeNull(); + }); + + it("returns null when destination has no further redirects", () => { + const edges = [edge("1", "/a", "/b")]; + const result = wouldCreateLoop("/x", "/y", edges); + expect(result).toBeNull(); + }); + + it("detects loop when exact destination matches a pattern source", () => { + const edges = [edge("1", "/blog/[slug]", "/articles/[slug]", true, true)]; + // Adding /articles/hello → /blog/hello would loop via the pattern: + // /articles/hello → /blog/hello → (matches /blog/[slug]) → /articles/hello + const result = wouldCreateLoop("/articles/hello", "/blog/hello", edges); + expect(result).not.toBeNull(); + expect(result).toEqual(["/articles/hello", "/blog/hello", "/articles/hello"]); + }); + + it("detects loop when pattern source redirect destination resolves back to itself", () => { + // /blog/[slug] → /articles/[slug] exists + // Adding /articles/[slug] → /blog/hello: + // walk: /blog/hello → matches /blog/[slug] → /articles/hello → matches /articles/[slug] (proposed source) + const edges = [edge("1", "/blog/[slug]", "/articles/[slug]", true, true)]; + const result = wouldCreateLoop("/articles/[slug]", "/blog/hello", edges); + expect(result).not.toBeNull(); + }); + + it("detects loop between catch-all [...path] and [slug] patterns via representatives", () => { + // /old/[...path] → /new/[...path] exists + // Adding /new/archive/[slug] → /old/archive/[slug]: + // representative /old/archive/__p__ matches /old/[...path] → /new/archive/__p__ + // /new/archive/__p__ matches proposed source /new/archive/[slug] → loop + const edges = [edge("1", "/old/[...path]", "/new/[...path]", true, true)]; + const result = wouldCreateLoop("/new/archive/[slug]", "/old/archive/[slug]", edges); + expect(result).not.toBeNull(); + }); + + it("detects loop with multiple overlapping patterns and exact redirects", () => { + // Complex setup: + // /blog/[slug] → /articles/[slug] + // /blogs/[slug] → /articles/[slug] + // /articles/[slug] → /news/[slug] + // /blogs/hi → /news/hi (exact, shadows /blogs/[slug] for this path) + // /blog/hello → /articles/hello (exact, shadows /blog/[slug] for this path) + // + // Adding /news/[slug] → /blog/[slug] closes the loop: + // /blog/[slug] → /articles/[slug] → /news/[slug] → /blog/[slug] + const edges = [ + edge("1", "/blog/[slug]", "/articles/[slug]", true, true), + edge("2", "/blogs/[slug]", "/articles/[slug]", true, true), + edge("3", "/articles/[slug]", "/news/[slug]", true, true), + edge("4", "/blogs/hi", "/news/hi"), + edge("5", "/blog/hello", "/articles/hello"), + ]; + const result = wouldCreateLoop("/news/[slug]", "/blog/[slug]", edges); + expect(result).not.toBeNull(); + }); + + it("exact redirects coexisting with patterns do not prevent loop detection", () => { + // Even though /blog/hello has an exact redirect, the pattern /blog/[slug] + // still loops for all other slugs. The loop must be detected. + const edges = [ + edge("1", "/blog/[slug]", "/articles/[slug]", true, true), + edge("2", "/articles/[slug]", "/news/[slug]", true, true), + edge("3", "/blog/hello", "/articles/hello"), + ]; + const result = wouldCreateLoop("/news/[slug]", "/blog/[slug]", edges); + expect(result).not.toBeNull(); + }); +});