fix(pdf): pdf-to-image presets no longer 404 on 2+ files (#643)

Upload two PDFs to pdf-to-jpg and it answered `Tool "pdf-to-jpg" not
found`. pdf-to-jpg, pdf-to-png and pdf-to-tiff share
registerPdfToImageRoute, which registered a single-file endpoint and
nothing else, so the shared preset settings component's 2+-file
submission fell through to the generic `:section/:toolId/batch` route,
whose registry lookup misses every tool outside
createToolRoute/registerToolProcessFn.

Mirror of #627, different fix. image-to-pdf is many-to-one, so #633 sent
every file in one request. This direction is one-to-many: separate PDFs
want separate conversions, which is what /batch is for. The route now
serves its own /batch, the shape svg-to-raster already uses, and the
literal path beats the generic parametric one.

One PDF fans out to many page images, so a per-file result is a ZIP, same
as the single-file route. A batch returns a ZIP of per-document ZIPs in
upload order, keyed by X-File-Results so each result pairs with the file
it came from. A document that is unreadable, locked, empty, short of the
requested page range, or carrying no pages at all fails alone; 422 with a
reason per file when none survive.

That literal path also shadows the generic route's requireToolAccess
call, which would have turned a 403 into a converted ZIP for roles
without tools:use. All four endpoints in this file now gate.

Four ways the batch path could have reported something untrue are closed
with it: a storage fault blamed on the document (statusCode-carrying
errors now reach the error handler, the rest are logged before being
reduced to a generic message), per-file reasons stranded in a field
parseApiError never reads, a zero-byte upload dropped so that later
results landed on the wrong file, and a mid-stream failure ended cleanly
enough to pass for success (the socket is destroyed instead).

Page rendering and ZIP assembly are shared helpers now, createUniqueNamer
moves to lib/filename.ts next to its two existing copies, and
tool-route-drift fails if any batch-dispatched preset loses its /batch
route. Follow-up for the same defects in the sibling custom routes: #645.

Fixes #632

apps/api/src/lib/filename.ts

@@ -122,3 +122,28 @@ export function sanitizeFilename(raw: string): string {
 
   return name;
 }
+
+/**
+ * Give each output a name unique within one batch response.
+ *
+ * Two uploads may share a filename, and clients key results by name
+ * (the X-File-Results map), so a collision would drop a result rather than
+ * show it. Collisions get a `_1`, `_2`, ... suffix before the extension.
+ */
+export function createUniqueNamer(): (name: string) => string {
+  const used = new Set<string>();
+  return (name) => {
+    if (!used.has(name)) {
+      used.add(name);
+      return name;
+    }
+    const dotIdx = name.lastIndexOf(".");
+    const base = dotIdx > 0 ? name.slice(0, dotIdx) : name;
+    const ext = dotIdx > 0 ? name.slice(dotIdx) : "";
+    let counter = 1;
+    while (used.has(`${base}_${counter}${ext}`)) counter++;
+    const candidate = `${base}_${counter}${ext}`;
+    used.add(candidate);
+    return candidate;
+  };
+}

apps/api/src/routes/batch.ts

@@ -26,7 +26,7 @@ import { getSecurityHeaders } from "../lib/csp.js";
 import { formatZodErrors } from "../lib/errors.js";
 import { getFirstMissingBundleForTool } from "../lib/feature-status.js";
 import { validateImageBuffer } from "../lib/file-validation.js";
-import { sanitizeFilename } from "../lib/filename.js";
+import { createUniqueNamer, sanitizeFilename } from "../lib/filename.js";
 import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
 import { decodeHeic } from "../lib/heic-converter.js";
 import { deleteObject, getObjectStream, putObject } from "../lib/object-storage.js";
@@ -551,24 +551,7 @@ export async function registerBatchRoutes(app: FastifyInstance): Promise<void> {
           }
 
           // Deduplicate output filenames and build X-File-Results header
-          const usedNames = new Set<string>();
-          function getUniqueName(name: string): string {
-            if (!usedNames.has(name)) {
-              usedNames.add(name);
-              return name;
-            }
-            const dotIdx = name.lastIndexOf(".");
-            const base = dotIdx > 0 ? name.slice(0, dotIdx) : name;
-            const ext = dotIdx > 0 ? name.slice(dotIdx) : "";
-            let counter = 1;
-            let candidate = `${base}_${counter}${ext}`;
-            while (usedNames.has(candidate)) {
-              counter++;
-              candidate = `${base}_${counter}${ext}`;
-            }
-            usedNames.add(candidate);
-            return candidate;
-          }
+          const getUniqueName = createUniqueNamer();
 
           const fileResultsMap: Record<string, string> = {};
           for (const entry of successEntries) {

apps/api/src/routes/tools/pdf-to-image.ts

@@ -6,10 +6,19 @@ import * as mupdf from "mupdf";
 import sharp from "sharp";
 import { z } from "zod";
 import { env } from "../../config.js";
+import { getSecurityHeaders } from "../../lib/csp.js";
 import { formatZodErrors } from "../../lib/errors.js";
+import { createUniqueNamer, sanitizeFilename } from "../../lib/filename.js";
 import { encodeJxl } from "../../lib/format-encoders.js";
 import { encodeHeic } from "../../lib/heic-converter.js";
-import { getObjectBuffer, putObject } from "../../lib/object-storage.js";
+import {
+  deletePrefix,
+  getObjectBuffer,
+  getObjectStream,
+  putObject,
+} from "../../lib/object-storage.js";
+import { requireToolAccess } from "../../permissions.js";
+import { updateJobProgress } from "../progress.js";
 
 // ── Settings schema ──────────────────────────────────────────────
 const settingsSchema = z.object({
@@ -179,6 +188,102 @@ function isPdfBuffer(buf: Buffer): boolean {
   return buf.subarray(0, 1024).includes("%PDF-");
 }
 
+/**
+ * A caller-fixable problem with one document: it is locked, or the requested
+ * page range does not fit it. Separated from render failures so the
+ * single-file route answers 400 rather than 422, and so the batch route can
+ * report the reason against the file that caused it.
+ */
+class PdfInputError extends Error {}
+
+interface RenderedPages {
+  pages: Array<{ page: number; downloadUrl: string; size: number }>;
+  filenames: string[];
+  totalPages: number;
+  selectedPages: number[];
+}
+
+/**
+ * Render the settings-selected pages of one PDF into `outputs/<jobId>/`.
+ * Each page is written to storage as soon as it is encoded, so peak memory
+ * stays at a single page no matter how long the document is.
+ */
+async function renderPdfPages(
+  fileBuffer: Buffer,
+  settings: z.infer<typeof settingsSchema>,
+  jobId: string,
+): Promise<RenderedPages> {
+  let doc: mupdf.Document | null = null;
+  try {
+    doc = mupdf.Document.openDocument(fileBuffer, "application/pdf");
+    if (doc.needsPassword()) {
+      throw new PdfInputError("Password-protected PDFs are not supported");
+    }
+
+    const totalPages = doc.countPages();
+    let selectedPages: number[];
+    try {
+      selectedPages = parsePageRange(settings.pages, totalPages);
+    } catch (err) {
+      throw new PdfInputError(err instanceof Error ? err.message : "Invalid page range");
+    }
+
+    // mupdf repairs and opens a document whose page tree is empty, which would
+    // otherwise render nothing and hand back a valid but empty ZIP labelled a
+    // success.
+    if (selectedPages.length === 0) {
+      throw new PdfInputError("This PDF has no pages to convert");
+    }
+
+    const ext = FORMAT_EXT[settings.format] ?? ".png";
+    const pages: RenderedPages["pages"] = [];
+    const filenames: string[] = [];
+
+    for (const pageNum of selectedPages) {
+      const pngBytes = renderPage(doc, pageNum - 1, settings.dpi);
+      const imageBuffer = await convertWithSharp(
+        pngBytes,
+        settings.format,
+        settings.quality,
+        settings.colorMode,
+      );
+      const filename = `page-${pageNum}${ext}`;
+      await putObject(`outputs/${jobId}/${filename}`, imageBuffer);
+      filenames.push(filename);
+      pages.push({
+        page: pageNum,
+        downloadUrl: `/api/v1/download/${jobId}/${encodeURIComponent(filename)}`,
+        size: imageBuffer.length,
+      });
+    }
+
+    return { pages, filenames, totalPages, selectedPages };
+  } finally {
+    doc?.destroy();
+  }
+}
+
+/**
+ * Zip one job's already-stored page images, reading a single entry at a time.
+ * The finished archive is materialized in memory, so peak cost is one whole
+ * document's worth of encoded pages.
+ */
+async function buildPagesZip(jobId: string, filenames: string[]): Promise<Buffer> {
+  const archive = archiver("zip", { zlib: { level: 5 } });
+  const chunks: Buffer[] = [];
+  archive.on("data", (chunk: Buffer) => chunks.push(chunk));
+  const done = new Promise<void>((resolve, reject) => {
+    archive.on("end", resolve);
+    archive.on("error", reject);
+  });
+  for (const filename of filenames) {
+    archive.append(await getObjectBuffer(`outputs/${jobId}/${filename}`), { name: filename });
+  }
+  await archive.finalize();
+  await done;
+  return Buffer.concat(chunks);
+}
+
 // ── Route registration ───────────────────────────────────────────
 export function registerPdfToImageRoute(
   app: FastifyInstance,
@@ -188,6 +293,8 @@ export function registerPdfToImageRoute(
 
   // ── Info endpoint ────────────────────────────────────────────
   app.post(`${basePath}/info`, async (request, reply) => {
+    if (!(await requireToolAccess(request, reply, opts.toolId))) return;
+
     let fileBuffer: Buffer | null = null;
     try {
       const result = await readPdfFromParts(request);
@@ -230,6 +337,8 @@ export function registerPdfToImageRoute(
 
   // ── Preview endpoint (thumbnails) ─────────────────────────────
   app.post(`${basePath}/preview`, async (request, reply) => {
+    if (!(await requireToolAccess(request, reply, opts.toolId))) return;
+
     let fileBuffer: Buffer | null = null;
     try {
       const result = await readPdfFromParts(request);
@@ -293,8 +402,231 @@ export function registerPdfToImageRoute(
     }
   });
 
+  // ── Batch endpoint ───────────────────────────────────────────
+  //
+  // This literal path takes priority over the generic
+  // `/api/v1/tools/:section/:toolId/batch` route, which only knows tools in
+  // the createToolRoute/registerToolProcessFn registry. pdf-to-image and its
+  // presets never register there, so without this they 404 the moment the web
+  // client sends a second file (issue #632).
+  //
+  // One PDF fans out to many page images, so the per-file result is a ZIP,
+  // matching what the single-file route already returns. The response is
+  // therefore a ZIP of per-document ZIPs, one per input, in upload order.
+  app.post(
+    `${basePath}/batch`,
+    { config: { rateLimit: { max: 20, timeWindow: "1 minute" } } },
+    async (request, reply) => {
+      if (!(await requireToolAccess(request, reply, opts.toolId))) return;
+
+      // Rasterizing several documents can outrun the default socket timeout.
+      request.raw.socket?.setTimeout?.(0);
+
+      const files: Array<{ buffer: Buffer; filename: string }> = [];
+      let settingsRaw: string | null = null;
+      let clientJobId: string | null = null;
+
+      try {
+        for await (const part of request.parts()) {
+          if (part.type === "file") {
+            const chunks: Buffer[] = [];
+            for await (const chunk of part.file) {
+              chunks.push(chunk);
+            }
+            // Empty parts keep their slot: the client maps results back onto
+            // its own file list by index, so dropping one here would label a
+            // converted document with a different file's name.
+            files.push({
+              buffer: Buffer.concat(chunks),
+              filename: sanitizeFilename(part.filename ?? "document.pdf"),
+            });
+          } else if (part.fieldname === "settings") {
+            settingsRaw = part.value as string;
+          } else if (part.fieldname === "clientJobId") {
+            // Doubles as an object-key segment and a response header value, so
+            // anything outside the key charset is ignored rather than allowed
+            // to fail every file or to break writeHead after hijack.
+            const raw = part.value as string;
+            if (typeof raw === "string" && /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/.test(raw)) {
+              clientJobId = raw;
+            }
+          }
+        }
+      } catch (err) {
+        return reply.status(400).send({
+          error: "Failed to parse multipart request",
+          details: err instanceof Error ? err.message : String(err),
+        });
+      }
+
+      if (files.length === 0) {
+        return reply.status(400).send({ error: "No PDF files provided" });
+      }
+
+      // Backstop only: the multipart iterator reads MAX_BATCH_SIZE per call and
+      // busboy stops at the limit first, so this rarely fires.
+      if (env.MAX_BATCH_SIZE > 0 && files.length > env.MAX_BATCH_SIZE) {
+        return reply.status(400).send({
+          error: `Too many files. Maximum batch size is ${env.MAX_BATCH_SIZE}`,
+        });
+      }
+
+      let settings: z.infer<typeof settingsSchema>;
+      try {
+        const parsed = settingsRaw ? JSON.parse(settingsRaw) : {};
+        const result = settingsSchema.safeParse(parsed);
+        if (!result.success) {
+          return reply
+            .status(400)
+            .send({ error: "Invalid settings", details: formatZodErrors(result.error.issues) });
+        }
+        settings = result.data;
+      } catch {
+        return reply.status(400).send({ error: "Settings must be valid JSON" });
+      }
+
+      if (opts.lockedFormat) {
+        settings.format = opts.lockedFormat as typeof settings.format;
+      }
+
+      const jobId = clientJobId || randomUUID();
+      const results: Array<{ key: string; prefix: string; filename: string } | null> = new Array(
+        files.length,
+      ).fill(null);
+      const errors: Array<{ filename: string; error: string }> = [];
+
+      const publishProgress = (completedFiles: number, currentFile?: string) =>
+        updateJobProgress({
+          jobId,
+          status: "processing",
+          totalFiles: files.length,
+          completedFiles,
+          failedFiles: errors.length,
+          errors,
+          ...(currentFile ? { currentFile } : {}),
+        });
+
+      publishProgress(0);
+
+      // Sequential on purpose: mupdf rasterization is synchronous and CPU-bound,
+      // so interleaving documents mostly multiplies peak memory.
+      for (let i = 0; i < files.length; i++) {
+        const file = files[i];
+        try {
+          publishProgress(i, file.filename);
+          if (file.buffer.length === 0) {
+            throw new PdfInputError("File is empty");
+          }
+          if (!isPdfBuffer(file.buffer)) {
+            throw new PdfInputError("Invalid or corrupt PDF file");
+          }
+          const childJobId = `${jobId}-f${i}`;
+          const prefix = `outputs/${childJobId}`;
+          const { filenames } = await renderPdfPages(file.buffer, settings, childJobId);
+          const zipName = `${file.filename.replace(/\.pdf$/i, "")}-pages.zip`;
+          const key = `${prefix}/${zipName}`;
+          await putObject(key, await buildPagesZip(childJobId, filenames));
+          results[i] = { key, prefix, filename: zipName };
+        } catch (err) {
+          // A storage or infrastructure fault is not a bad document. Let it
+          // reach the error handler (which logs it, reports it, and honors its
+          // status) instead of telling the user their PDF is broken.
+          if (typeof (err as { statusCode?: number })?.statusCode === "number") throw err;
+          if (err instanceof PdfInputError) {
+            errors.push({ filename: file.filename, error: err.message });
+          } else {
+            request.log.error(
+              { err, filename: file.filename, toolId: opts.toolId },
+              "PDF batch file conversion failed",
+            );
+            // Generic on purpose: only messages this route authors are safe to
+            // echo, and internal errors can carry absolute paths.
+            errors.push({ filename: file.filename, error: "PDF conversion failed" });
+          }
+        }
+      }
+
+      updateJobProgress({
+        jobId,
+        status: errors.length === files.length ? "failed" : "completed",
+        totalFiles: files.length,
+        completedFiles: files.length,
+        failedFiles: errors.length,
+        errors,
+      });
+
+      if (errors.length === files.length) {
+        // parseApiError on the client reads `error` and `details`, so the
+        // per-file reasons have to ride in `details` to be seen at all.
+        return reply.status(422).send({
+          error: "All files failed processing",
+          details: errors.map((e) => `${e.filename}: ${e.error}`),
+          errors,
+        });
+      }
+
+      const uniqueName = createUniqueNamer();
+      const fileResultsMap: Record<string, string> = {};
+      for (let i = 0; i < results.length; i++) {
+        const entry = results[i];
+        if (!entry) continue;
+        entry.filename = uniqueName(entry.filename);
+        fileResultsMap[String(i)] = entry.filename;
+      }
+
+      // Hijack and stream the ZIP response
+      reply.hijack();
+      reply.raw.writeHead(200, {
+        "Content-Type": "application/zip",
+        "Content-Disposition": `attachment; filename="batch-${opts.toolId}-${jobId.slice(0, 8)}.zip"`,
+        "Transfer-Encoding": "chunked",
+        "X-Job-Id": jobId,
+        "X-File-Results": encodeURIComponent(JSON.stringify(fileResultsMap)),
+        ...getSecurityHeaders(),
+      });
+
+      const archive = archiver("zip", { zlib: { level: 5 } });
+
+      // Headers are already out, so a failure here cannot change the status.
+      // Destroy the socket rather than end() it: a clean end on a chunked
+      // response is indistinguishable from success, and the client would keep
+      // a truncated ZIP believing it complete.
+      const failStream = (err: Error, msg: string) => {
+        request.log.error({ err, jobId }, msg);
+        archive.abort();
+        reply.raw.destroy(err);
+      };
+
+      archive.on("error", (err) => failStream(err, "Archiver error during PDF batch processing"));
+      archive.pipe(reply.raw);
+
+      try {
+        for (const entry of results) {
+          if (!entry) continue;
+          archive.append(await getObjectStream(entry.key), { name: entry.filename });
+        }
+        await archive.finalize();
+      } catch (err) {
+        failStream(
+          err instanceof Error ? err : new Error(String(err)),
+          "Failed to stream ZIP entries during PDF batch processing",
+        );
+      }
+
+      // The per-page images and per-document ZIPs exist only to build the
+      // response, which is now sent. Leaving them would strand every rendered
+      // page on the volume until the storage sweep, and they carry no jobs row
+      // for retention or GDPR deletion to find.
+      await Promise.all(
+        results.map((entry) => (entry ? deletePrefix(entry.prefix).catch(() => {}) : null)),
+      );
+    },
+  );
+
   // ── Main processing endpoint ─────────────────────────────────
   app.post(basePath, async (request, reply) => {
+    if (!(await requireToolAccess(request, reply, opts.toolId))) return;
+
     let fileBuffer: Buffer | null = null;
     let settingsRaw: string | null = null;
 
@@ -335,66 +667,16 @@ export function registerPdfToImageRoute(
       settings.format = opts.lockedFormat as typeof settings.format;
     }
 
-    let doc: mupdf.Document | null = null;
+    const jobId = randomUUID();
     try {
-      doc = mupdf.Document.openDocument(fileBuffer, "application/pdf");
-      if (doc.needsPassword()) {
-        return reply.status(400).send({ error: "Password-protected PDFs are not supported" });
-      }
+      const { pages, filenames, totalPages, selectedPages } = await renderPdfPages(
+        fileBuffer,
+        settings,
+        jobId,
+      );
 
-      const totalPages = doc.countPages();
-
-      let selectedPages: number[];
-      try {
-        selectedPages = parsePageRange(settings.pages, totalPages);
-      } catch (err) {
-        return reply
-          .status(400)
-          .send({ error: err instanceof Error ? err.message : "Invalid page range" });
-      }
-
-      const ext = FORMAT_EXT[settings.format] ?? ".png";
-      const jobId = randomUUID();
-      const pages: Array<{ page: number; downloadUrl: string; size: number }> = [];
-      const pageFilenames: string[] = [];
-
-      for (const pageNum of selectedPages) {
-        const pngBytes = renderPage(doc, pageNum - 1, settings.dpi);
-        const imageBuffer = await convertWithSharp(
-          pngBytes,
-          settings.format,
-          settings.quality,
-          settings.colorMode,
-        );
-        const filename = `page-${pageNum}${ext}`;
-        await putObject(`outputs/${jobId}/${filename}`, imageBuffer);
-        pageFilenames.push(filename);
-        pages.push({
-          page: pageNum,
-          downloadUrl: `/api/v1/download/${jobId}/${encodeURIComponent(filename)}`,
-          size: imageBuffer.length,
-        });
-      }
-
-      doc.destroy();
-      doc = null;
-
-      // Build ZIP by streaming each entry from object storage (O(1-entry) peak)
       const zipFilename = "pdf-pages.zip";
-      const archive = archiver("zip", { zlib: { level: 5 } });
-      const zipChunks: Buffer[] = [];
-      archive.on("data", (chunk: Buffer) => zipChunks.push(chunk));
-      const zipDone = new Promise<void>((resolve, reject) => {
-        archive.on("end", resolve);
-        archive.on("error", reject);
-      });
-      for (const fname of pageFilenames) {
-        const buf = await getObjectBuffer(`outputs/${jobId}/${fname}`);
-        archive.append(buf, { name: fname });
-      }
-      await archive.finalize();
-      await zipDone;
-      const zipBuffer = Buffer.concat(zipChunks);
+      const zipBuffer = await buildPagesZip(jobId, filenames);
       await putObject(`outputs/${jobId}/${zipFilename}`, zipBuffer);
       const zipUrl = `/api/v1/download/${jobId}/${encodeURIComponent(zipFilename)}`;
 
@@ -411,7 +693,9 @@ export function registerPdfToImageRoute(
         zipSize: zipBuffer.length,
       });
     } catch (err) {
-      doc?.destroy();
+      if (err instanceof PdfInputError) {
+        return reply.status(400).send({ error: err.message });
+      }
       return reply.status(422).send({
         error: "PDF conversion failed",
         details: err instanceof Error ? err.message : "Unknown error",

apps/api/src/routes/tools/svg-to-raster.ts

@@ -9,7 +9,7 @@ import { env } from "../../config.js";
 import { getSecurityHeaders } from "../../lib/csp.js";
 import { resolveConcurrency } from "../../lib/env.js";
 import { formatZodErrors } from "../../lib/errors.js";
-import { sanitizeFilename } from "../../lib/filename.js";
+import { createUniqueNamer, sanitizeFilename } from "../../lib/filename.js";
 import { encodeJxl } from "../../lib/format-encoders.js";
 import { decodeHeic, encodeHeic } from "../../lib/heic-converter.js";
 import { putObject } from "../../lib/object-storage.js";
@@ -312,24 +312,7 @@ export function registerSvgToRasterRoute(
     }
 
     // Deduplicate filenames and build X-File-Results header
-    const usedNames = new Set<string>();
-    function getUniqueName(name: string): string {
-      if (!usedNames.has(name)) {
-        usedNames.add(name);
-        return name;
-      }
-      const dotIdx = name.lastIndexOf(".");
-      const base = dotIdx > 0 ? name.slice(0, dotIdx) : name;
-      const ext = dotIdx > 0 ? name.slice(dotIdx) : "";
-      let counter = 1;
-      let candidate = `${base}_${counter}${ext}`;
-      while (usedNames.has(candidate)) {
-        counter++;
-        candidate = `${base}_${counter}${ext}`;
-      }
-      usedNames.add(candidate);
-      return candidate;
-    }
+    const getUniqueName = createUniqueNamer();
 
     const fileResultsMap: Record<string, string> = {};
     for (let i = 0; i < results.length; i++) {