Replace filenamify with safeFilenamify to fix wrong filename handling

essential-randomness · essential-randomness · commit b96f0ba2ece0 · 2026-04-04T20:18:46.000-07:00
diff --git a/tests/mocks/handlers/series/index.ts b/tests/mocks/handlers/series/index.ts
@@ -1,8 +1,7 @@
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/series/:series_id",
@@ -11,7 +10,7 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "series",
-        filenamify(params.series_id as string),
+        safeFilenamify(params.series_id as string),
         "index.html"
       )
     );
diff --git a/tests/mocks/handlers/tags/feed.ts b/tests/mocks/handlers/tags/feed.ts
@@ -1,9 +1,7 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/tags/:name/feed.atom",
@@ -12,7 +10,7 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "tags",
-        filenamify(params.name as string),
+        safeFilenamify(params.name as string),
         "feed.atom"
       )
     );
diff --git a/tests/mocks/handlers/tags/name.ts b/tests/mocks/handlers/tags/name.ts
@@ -1,9 +1,7 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/tags/:name",
@@ -12,7 +10,7 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "tags",
-        filenamify(params.name as string),
+        safeFilenamify(params.name as string),
         "index.html"
       )
     );
diff --git a/tests/mocks/handlers/tags/works.ts b/tests/mocks/handlers/tags/works.ts
@@ -1,9 +1,7 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/tags/:name/works",
@@ -12,7 +10,7 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "tags",
-        filenamify(params.name as string),
+        safeFilenamify(params.name as string),
         "works.html"
       )
     );
diff --git a/tests/mocks/handlers/users/profile.ts b/tests/mocks/handlers/users/profile.ts
@@ -1,9 +1,7 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/users/:name/profile",
@@ -12,7 +10,7 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "users",
-        filenamify(params.name as string),
+        safeFilenamify(params.name as string),
         "profile",
         "index.html"
       )
diff --git a/tests/mocks/handlers/works/chapter.ts b/tests/mocks/handlers/works/chapter.ts
@@ -1,9 +1,7 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/works/:work_id/chapters/:chapter_id",
@@ -12,9 +10,9 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "works",
-        filenamify(params.work_id as string),
+        safeFilenamify(params.work_id as string),
         "chapters",
-        `${filenamify((params.chapter_id as string) || "index")}.html`
+        `${safeFilenamify((params.chapter_id as string) || "index")}.html`
       )
     );
 
diff --git a/tests/mocks/handlers/works/index.ts b/tests/mocks/handlers/works/index.ts
@@ -1,17 +1,15 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default [
   http.all("https://archiveofourown.org/works/:work_id", ({ params }) => {
     const html = fs.readFileSync(
       path.resolve(
         getArchiveDataDir(),
         "works",
-        filenamify(params.work_id as string),
+        safeFilenamify(params.work_id as string),
         "index.html"
       )
     );
@@ -24,7 +22,7 @@ export default [
       path.resolve(
         getArchiveDataDir("superlove"),
         "works",
-        filenamify(params.work_id as string),
+        safeFilenamify(params.work_id as string),
         "index.html"
       )
     );
diff --git a/tests/mocks/handlers/works/navigate.ts b/tests/mocks/handlers/works/navigate.ts
@@ -1,9 +1,7 @@
-import { fileURLToPath } from "url";
-import filenamify from "filenamify";
 import fs from "fs";
 import path from "path";
 import { http, HttpHandler, HttpResponse } from "msw";
-import { getArchiveDataDir } from "../../scripts/utils.mjs";
+import { getArchiveDataDir, safeFilenamify } from "../../scripts/utils.mjs";
 
 export default http.all(
   "https://archiveofourown.org/works/:work_id/navigate",
@@ -12,7 +10,7 @@ export default http.all(
       path.resolve(
         getArchiveDataDir(),
         "works",
-        filenamify(params.work_id as string),
+        safeFilenamify(params.work_id as string),
         "navigate.html"
       )
     );
diff --git a/tests/mocks/scripts/encode-mock-files.mts b/tests/mocks/scripts/encode-mock-files.mts
@@ -2,10 +2,10 @@ import dirTree from "directory-tree";
 import path, { dirname } from "path";
 import { fileURLToPath } from "url";
 import { renameSync } from "fs";
-import filenamify from "filenamify";
+import { safeFilenamify } from "./utils.mjs";
 
 const hasInvalidFileCharacters = (filename: string) => {
-  return filenamify(filename) != filename;
+  return safeFilenamify(filename) != filename;
 };
 
 const recursivelyRenameDirectories = (folder: ReturnType<typeof dirTree>) => {
@@ -26,12 +26,12 @@ const recursivelyRenameDirectories = (folder: ReturnType<typeof dirTree>) => {
     console.log(
       `${path.join(containingDir, folder.name)} => ${path.join(
         containingDir,
-        filenamify(folder.name)
+        safeFilenamify(folder.name)
       )}`
     );
     renameSync(
       path.join(containingDir, folder.name),
-      path.join(containingDir, filenamify(folder.name))
+      path.join(containingDir, safeFilenamify(folder.name))
     );
   }
 };
diff --git a/tests/mocks/scripts/redownload-articles.mts b/tests/mocks/scripts/redownload-articles.mts
@@ -7,6 +7,7 @@ const KNOWN_404 = [
 ]
 
 import {
+  decodeFilename,
   delay,
   downloadWithRetry,
   recursivelyGetFiles,
@@ -16,16 +17,6 @@ import {
   Http404Error,
 } from "./utils.mts";
 
-// Function to decode an encoded filename
-// TODO: write a better description
-function decodeFilename(encodedName: string): string {
-  return encodedName
-    .replace(/!d!/g, ".") // Period
-    .replace(/!a!/g, "&") // Ampersand
-    .replace(/!s!/g, "/") // Forward slash
-    .replace(/\*a\*/g, "&"); // Alternative ampersand encoding
-}
-
 function getUrlFromPath(
   relativePath: string,
   archive: "ao3" | "superlove"
diff --git a/tests/mocks/scripts/utils.mts b/tests/mocks/scripts/utils.mts
@@ -3,6 +3,22 @@ import fs from "fs/promises";
 import path from "path";
 import { fileURLToPath } from "url";
 
+// filenamify replaces | with ! which can cause bugs since ! appears naturally in tag names.
+// We encode | as !p! (matching existing !d!, !a!, !s! patterns) before filenamify
+// so the encoding is always reversible.
+export function safeFilenamify(name: string): string {
+  return filenamify(name.replace(/\|/g, "!p!"));
+}
+
+export function decodeFilename(encodedName: string): string {
+  return encodedName
+    .replace(/!p!/g, "|") // Pipe
+    .replace(/!d!/g, ".") // Period
+    .replace(/!a!/g, "&") // Ampersand
+    .replace(/!s!/g, "/") // Forward slash
+    .replace(/\*a\*/g, "&"); // Alternative ampersand encoding
+}
+
 const BASE_DELAY = 1000; // 1 second
 
 const ARCHIVE_URLS = {
@@ -24,7 +40,7 @@ export async function delay(ms: number): Promise<void> {
 export async function downloadWithRetry(
   url: string,
   maxAttempts = 3,
-  currentAttempt = 1
+  currentAttempt = 1,
 ) {
   try {
     const response = await fetch(url, {
@@ -36,7 +52,7 @@ export async function downloadWithRetry(
     if (response.status === 429) {
       const retryAfter = parseInt(
         response.headers.get("retry-after") || "0",
-        10
+        10,
       );
 
       console.log(`Rate limited. Waiting ${retryAfter / 1000} seconds...`);
@@ -66,11 +82,11 @@ export async function downloadWithRetry(
 
     const backoffDelay = Math.min(
       BASE_DELAY * Math.pow(2, currentAttempt - 1) + Math.random() * 1000,
-      30000 // Max 30 seconds
+      30000, // Max 30 seconds
     );
 
     console.log(
-      `Attempt ${currentAttempt} failed, retrying after ${backoffDelay}ms...`
+      `Attempt ${currentAttempt} failed, retrying after ${backoffDelay}ms...`,
     );
     console.log(`Error message was ${errorMessage}`);
     await delay(backoffDelay);
@@ -102,7 +118,9 @@ const getNormalizedTagSearchFolder = (searchParams: URLSearchParams) => {
     const sortedValues = values.length > 1 ? [...values].sort() : values;
     for (const value of sortedValues) {
       const segment = `${key}=${value == "any" ? "" : value}`;
-      entries.push(filenamify(segment.toLowerCase(), { replacement: "_", maxLength: 100 }));
+      entries.push(
+        filenamify(segment.toLowerCase(), { replacement: "_", maxLength: 100 }),
+      );
     }
   }
 
@@ -136,7 +154,7 @@ export function getFilePathFromUrl(url: string | URL) {
   if (segments[0] === "tags" && lastSegment === "search") {
     return path.join(
       getArchiveDataDir(archive),
-      getFilePathForSearchUrl(parsedUrl)
+      getFilePathForSearchUrl(parsedUrl),
     );
   }
 
@@ -146,7 +164,9 @@ export function getFilePathFromUrl(url: string | URL) {
     hasExtension || lastSegment == "works" ? segments.slice(0, -1) : segments;
   const dirPath = path.join(
     getArchiveDataDir(archive),
-    ...dirSegments.map((segment) => filenamify(decodeURIComponent(segment)))
+    ...dirSegments.map((segment) =>
+      safeFilenamify(decodeURIComponent(segment)),
+    ),
   );
 
   // If last segment has an extension, use it as filename, otherwise use index.html
@@ -156,8 +176,8 @@ export function getFilePathFromUrl(url: string | URL) {
     hasExtension
       ? lastSegment
       : lastSegment == "works"
-      ? "works.html"
-      : "index.html"
+        ? "works.html"
+        : "index.html",
   );
 }
 
@@ -194,8 +214,8 @@ export function getArchiveFromUrl(input: string | URL): "ao3" | "superlove" {
   const archive = hostname.includes(ARCHIVE_URLS.ao3)
     ? "ao3"
     : hostname.includes(ARCHIVE_URLS.superlove)
-    ? "superlove"
-    : null;
+      ? "superlove"
+      : null;
   if (!archive) {
     throw new Error(`Cannot determine archive from URL: ${input}`);
   }
