fix(pdf): a PDF pypdf can't read is a 400, not a 500 — LimitReachedError too

CHANGELOG.md

@@ -9,6 +9,47 @@ Versions follow [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed — a PDF that pypdf can't read gets a 400, not a 500
+
+pypdf raises `LimitReachedError` when a crafted file trips one of its safety
+limits (declared stream length, decompressed size, font widths, page-tree
+depth). It derives from `PyPdfError`, not from the `PdfReadError` the PDF paths
+caught, and pypdf reads most of a file only when a page is copied or its text
+is extracted, after the one guarded open. Such a file got the generic 500 and a
+logged traceback instead of the 400 a broken upload gets. So did a PDF that
+failed after the open with another pypdf error (in a fuzz run mostly a damaged
+page object) on page extraction and splitting, and any unreadable or
+password-protected PDF converted to TXT, which caught nothing. No detail
+reached the client; the status and the log were wrong.
+
+Every pypdf step that reads the upload now answers "Could not read the PDF.
+Verify the file is valid." for `PyPdfError` and pypdf's plain `ValueError`s,
+and logs one line naming the error's class (not its message, which can quote
+the file):
+
+- `/pdf/extract`, `/pdf/split`: `400` with `X-FileMorph-Error-Code:
+  invalid_pdf`. `/pdf/extract` used to send `invalid_page_selection` for an
+  unreadable file, so its web page asked the user to fix a page selection that
+  was fine; an unreadable file no longer gets that code.
+- `/convert` (PDF → TXT, and the PDF → PDF pass-through): `400` with
+  `invalid_input`; `/convert/batch` reports the same message for that file
+  instead of "Conversion failed. Verify the file is valid."
+
+An error reading the server's own copy of the upload is now a 500 on
+`/pdf/extract` and `/pdf/split` too, not a 400: pypdf reads the whole file into
+memory first, so an `OSError` there is never the PDF's fault. And PDF → TXT
+failed with a 500 on text containing an unpaired surrogate, which a broken font
+map produces and UTF-8 can't encode; that character is now written as `?`.
+
+Tests build tiny PDFs that fail at each stage (a page tree deeper than 100
+levels, an 80 MB `/Length`, a page object without `/Type`, a font `/W` range of
+100 000 glyphs, a content stream that isn't ASCII85) and check that each route
+answers 400 without logging a traceback.
+
+`scripts/make_testdata_pdf_unreadable.py` writes byte-stable fixtures for
+checking this by hand (seven small PDFs, among them a password-protected one)
+to a gitignored local folder; only the script ships.
+
 ### Fixed — deleting a free account works on PostgreSQL
 
 `DELETE /api/v1/auth/account` failed with a 500 on PostgreSQL for every

app/api/routes/pdf_pages.py

@@ -46,6 +46,7 @@
 from app.compressors.pdf import compress_pdf_to_target
 from app.converters.pdf_pages import (
     PageSelectionError,
+    UnreadablePdfError,
     extract_pages,
     split_pdf,
 )
@@ -146,11 +147,16 @@ async def _do_extract(
         except PageSelectionError as exc:
             # Caller-safe message already (no pypdf internals). 400 — the
             # client's page selection or PDF was the problem, not the server.
+            # The code tells the UI which one, so it doesn't blame the
+            # selection for a file pypdf can't read.
             logger.info("pdf extract rejected: %s", exc)
+            code = (
+                "invalid_pdf" if isinstance(exc, UnreadablePdfError) else "invalid_page_selection"
+            )
             raise HTTPException(
                 status_code=status.HTTP_400_BAD_REQUEST,
                 detail=str(exc),
-                headers={"X-FileMorph-Error-Code": "invalid_page_selection"},
+                headers={"X-FileMorph-Error-Code": code},
             )
         except Exception:
             logger.exception("PDF extract error")

app/converters/document.py

@@ -8,7 +8,7 @@
 import zipfile
 from pathlib import Path
 
-from app.converters.base import BaseConverter, read_utf8_text
+from app.converters.base import BaseConverter, InvalidInputError, read_utf8_text
 from app.converters.registry import register
 
 logger = logging.getLogger(__name__)
@@ -352,10 +352,22 @@ def convert(self, input_path: Path, output_path: Path, **kwargs) -> Path:
 class PdfToTxtConverter(BaseConverter):
     def convert(self, input_path: Path, output_path: Path, **kwargs) -> Path:
         from pypdf import PdfReader
-
-        reader = PdfReader(str(input_path))
-        parts = [page.extract_text() or "" for page in reader.pages]
-        output_path.write_text("\n\n".join(parts), encoding="utf-8")
+        from pypdf.errors import PyPdfError
+
+        # PyPdfError covers LimitReachedError (a crafted file tripping one of
+        # pypdf's resource limits), which PdfReadError alone would miss. The
+        # extraction sits inside the try: pypdf reads fonts and content
+        # streams lazily, so a broken one only fails here.
+        try:
+            reader = PdfReader(str(input_path))
+            parts = [page.extract_text() or "" for page in reader.pages]
+        except (PyPdfError, ValueError) as exc:
+            # The class only — pypdf's messages can quote the file.
+            logger.info("unreadable PDF: %s", type(exc).__name__)
+            raise InvalidInputError("Could not read the PDF. Verify the file is valid.") from exc
+        # A broken font map can yield an unpaired surrogate, which UTF-8
+        # can't encode: write that character as "?" instead of failing.
+        output_path.write_text("\n\n".join(parts), encoding="utf-8", errors="replace", newline="\n")
         return output_path
 
 

app/converters/pdf_pages.py

@@ -28,16 +28,22 @@
 pypdf parsing of the *input* PDF still happens inside ``convert()`` /
 ``split_pdf()``; the route invokes both through ``asyncio.to_thread`` so
 the (synchronous, C-accelerated) parse never blocks the event loop —
-identical to every other converter.
+identical to every other converter. A PDF pypdf cannot read raises
+:class:`UnreadablePdfError`, a ``PageSelectionError`` with a fixed message.
 """
 
 from __future__ import annotations
 
+import logging
+from collections.abc import Iterator
+from contextlib import contextmanager
 from pathlib import Path
 
-from app.converters.base import BaseConverter
+from app.converters.base import BaseConverter, InvalidInputError
 from app.converters.registry import register
 
+logger = logging.getLogger(__name__)
+
 # Defensive ceiling on how many distinct pages a single selection may
 # resolve to. A crafted "1-1000000" against a 2-page PDF is already
 # rejected by the page-count bound, but an explicit cap keeps the parser
@@ -46,8 +52,19 @@
 _MAX_SELECTION_PAGES = 10_000
 
 
-class PageSelectionError(ValueError):
-    """Malformed or out-of-range page selection (caller-safe message)."""
+class PageSelectionError(InvalidInputError):
+    """Malformed or out-of-range page selection (caller-safe message).
+
+    An ``InvalidInputError``, so ``/convert`` and ``/convert/batch``
+    (``pdf`` → ``pdf``) answer it as a 400, like the page routes do.
+    """
+
+
+class UnreadablePdfError(PageSelectionError):
+    """pypdf could not read the input PDF; raised with ``_UNREADABLE_PDF``."""
+
+
+_UNREADABLE_PDF = "Could not read the PDF. Verify the file is valid."
 
 
 def parse_page_ranges(spec: str, page_count: int) -> list[int]:
@@ -122,25 +139,43 @@ def _parse_int(value: str, token: str) -> int:
     return n
 
 
-def _open_reader(input_path: Path):
-    """Open a PDF with pypdf, normalising any parse failure to a safe error.
+@contextmanager
+def _reading_pdf() -> Iterator[None]:
+    """Normalise a pypdf failure on the input PDF to a safe error.
 
-    pypdf raises a small zoo of exception types (``PdfReadError``,
-    ``EmptyFileError``, plain ``ValueError`` from the tokenizer) on a
-    corrupt or non-PDF input. The magic-byte guard in the route already
+    pypdf raises a small zoo of exception types on a corrupt or non-PDF
+    input: those derived from ``PyPdfError`` (``PdfReadError``,
+    ``EmptyFileError``, ``LimitReachedError`` when a crafted file trips one
+    of its resource limits) and plain ``ValueError`` (the tokenizer, a
+    damaged page object). ``PdfReadError`` alone misses
+    ``LimitReachedError``. pypdf parses lazily, so they surface while pages
+    are copied or written as well as on open: every step that reads the
+    input runs under this guard. The magic-byte guard in the route already
     blocks executables; this catch turns a genuinely malformed PDF into a
     single caller-safe error instead of leaking pypdf internals.
+
+    ``OSError`` stays a server error: pypdf reads the whole file into
+    memory first, so one can only come from our own disk. Only the
+    exception's class is logged — pypdf's messages can quote the file.
     """
-    from pypdf import PdfReader
-    from pypdf.errors import PdfReadError
+    from pypdf.errors import PyPdfError
 
     try:
+        yield
+    except (PyPdfError, ValueError) as exc:
+        logger.info("unreadable PDF: %s", type(exc).__name__)
+        raise UnreadablePdfError(_UNREADABLE_PDF) from exc
+
+
+def _open_reader(input_path: Path):
+    """Open a PDF with pypdf under :func:`_reading_pdf`."""
+    from pypdf import PdfReader
+
+    with _reading_pdf():
         reader = PdfReader(str(input_path))
         # Touch the page tree so a lazily-parsed corrupt xref surfaces here,
         # inside our guarded block, rather than later at iteration time.
         _ = len(reader.pages)
-    except (PdfReadError, ValueError, OSError) as exc:
-        raise PageSelectionError("Could not read the PDF. Verify the file is valid.") from exc
     return reader
 
 
@@ -152,10 +187,11 @@ def extract_pages(input_path: Path, output_path: Path, pages_spec: str) -> Path:
     indices = parse_page_ranges(pages_spec, len(reader.pages))
 
     writer = PdfWriter()
-    for idx in indices:
-        writer.add_page(reader.pages[idx])
-    with output_path.open("wb") as f:
-        writer.write(f)
+    with _reading_pdf():
+        for idx in indices:
+            writer.add_page(reader.pages[idx])
+        with output_path.open("wb") as f:
+            writer.write(f)
     return output_path
 
 
@@ -185,12 +221,13 @@ def split_pdf(input_path: Path) -> list[tuple[str, bytes]]:
 
     width = len(str(total))
     outputs: list[tuple[str, bytes]] = []
-    for i, page in enumerate(reader.pages, start=1):
-        writer = PdfWriter()
-        writer.add_page(page)
-        buf = io.BytesIO()
-        writer.write(buf)
-        outputs.append((f"page_{i:0{width}d}.pdf", buf.getvalue()))
+    with _reading_pdf():
+        for i, page in enumerate(reader.pages, start=1):
+            writer = PdfWriter()
+            writer.add_page(page)
+            buf = io.BytesIO()
+            writer.write(buf)
+            outputs.append((f"page_{i:0{width}d}.pdf", buf.getvalue()))
     return outputs
 
 

docs/api-reference.md

@@ -536,9 +536,9 @@ can branch on (the `detail` text may change):
 | `output_cap_exceeded` | `413` | The result is larger than your tier's output cap |
 | `target_size_exceeds_cap` | `413` | `target_size_kb` (`/compress`, `/compress/batch`) or `target_kb` (`/pdf/compress`) is above your tier's output cap — rejected before any work |
 | `decompression_bomb` | `400` | The image's dimensions exceed the decoder's safety limit (`/convert`, `/compress`) |
-| `invalid_input` | `400` | A problem you can fix, named in `detail` — e.g. a Markdown, CSV or JSON file that isn't UTF-8 (`/convert`) |
-| `invalid_page_selection` | `400` | `/pdf/extract`: the `pages` selection is invalid, or the PDF can't be read |
-| `invalid_pdf` | `400` | `/pdf/split`, `/pdf/compress`: the PDF can't be read; `/pdf/split` also for a PDF with no pages or more than 10 000 |
+| `invalid_input` | `400` | A problem you can fix, named in `detail` — e.g. a Markdown, CSV or JSON file that isn't UTF-8, or a PDF that can't be read (`/convert`) |
+| `invalid_page_selection` | `400` | `/pdf/extract`: the `pages` selection is invalid, or the PDF has no pages |
+| `invalid_pdf` | `400` | `/pdf/extract`, `/pdf/split`, `/pdf/compress`: the PDF can't be read; `/pdf/split` also for a PDF with no pages or more than 10 000 |
 
 The redaction endpoints add codes of their own, listed under
 [AI operations](#ai-operations--pii-redaction-enterprise-edition-add-on).

docs/api-usage-guide.md

@@ -502,6 +502,11 @@ Common per-file `error_message` values:
   CSV or JSON file in another encoding (e.g. Excel's default CSV export
   on Windows). Single-file `/convert` returns the same message as a
   `400` with `X-FileMorph-Error-Code: invalid_input`.
+- `"Could not read the PDF. Verify the file is valid."` — a PDF
+  converted to `txt` or `pdf` that is damaged, password-protected or
+  over one of the PDF reader's safety limits. Single-file `/convert`
+  returns the same message as a `400` with
+  `X-FileMorph-Error-Code: invalid_input`.
 - `"Conversion failed. Verify the file is valid."` (compress:
   `"Compression failed. …"`) — any other error while processing that
   file, e.g. corrupt content. The details stay in the server log.