From 1b65f7198abd459ad9e4842b2add84de7b35198d Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty <62941615+Souptik96@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:13:20 +0530 Subject: [PATCH] fix(verify-layout): name both causes of a broken bounding-box extractor (#451) (#465) verify_layout.py's skipped: message blamed only the xpdf-based pdftotext that Git for Windows puts ahead of Poppler in PATH. A real Poppler can abort too: 26.0x before 26.05 crashes -bbox/-bbox-layout/-htmlmeta on a PDF whose Info dictionary carries an empty string in any field, and hyperref writes exactly that for every field it does not set. A lualatex/pdflatex document built with hyperref and no \hypersetup{pdftitle=...} - an ordinary /add-template CV template, not a malformed one - hits this with a working Poppler installed, and the old message sent the reader to check their PATH when nothing was wrong with it. Named both causes in the raised message, the code comment above it, and the module docstring. Behavior is unchanged: either cause still degrades to skipped: exit 2, not exit 1, since a broken extractor is still not a broken document. Added test_poppler_abort_on_empty_info_string_names_that_cause_too, mocking subprocess.run to raise CalledProcessError the way a real Poppler 26.0x abort does (exit 1, a libc++abi out_of_range trace on stderr) rather than the xpdf case's exit 99 - the two failures are the same exception type with different exit codes and stderr text, so the message has to actually distinguish them, not just catch the one class. Negative control: this test fails against the pre-fix message with "'Poppler aborted' not found in '...a pdftotext without -bbox is usually the xpdf build...'". Thanks to @main-sounds-audio for the report and the isolated repro (which field, which Poppler modes, five runs of five) that pinned this to Poppler's own upstream regression (issue #1699, fixed in 26.05.0) rather than an extraction-library swap. Fixes #451 --- CHANGELOG.md | 12 ++++++++++++ tests/test_verify_layout.py | 22 ++++++++++++++++++++++ tools/verify_layout.py | 27 ++++++++++++++++++++------- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68629a0..7bc3b73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,18 @@ per-file diff commands. ### Fixed +- **`tools/verify_layout.py`'s `skipped:` message named only one cause of a broken + extractor when there are two** (#451) - it blamed the xpdf-based `pdftotext` Git for + Windows puts ahead of Poppler in PATH (no `-bbox` flag, exits 99), but a real Poppler + can abort `-bbox`/`-bbox-layout`/`-htmlmeta` too: Poppler 26.0x before 26.05 crashes on + any PDF whose Info dictionary carries an empty string in any field, and `hyperref` + writes exactly that for every field it does not set. A `lualatex`/`pdflatex` document + built with `hyperref` and no `\hypersetup{pdftitle=...}` - an ordinary `/add-template` + CV template, not a malformed one - hits this with a working Poppler installed, and the + old message sent the reader to check their PATH when nothing was wrong with it. The + message now names both causes; behavior is unchanged, degrading to `skipped:` exit 2 + either way, since a broken extractor is still not a broken document. + - **The template-placeholder guard in `test_setup_command.py` now skips on forks** (#463) - `TemplatesStillCarryThePlaceholders` asserts that `05-cv-templates.md` and `06-cover-letter-templates.md` still contain `[FIRST_NAME]`, `[LAST_NAME]`, `[YOUR_EMAIL]`, diff --git a/tests/test_verify_layout.py b/tests/test_verify_layout.py index 06df97f..31ce49d 100644 --- a/tests/test_verify_layout.py +++ b/tests/test_verify_layout.py @@ -126,6 +126,28 @@ class TestExtractorFailure(unittest.TestCase): with self.assertRaisesRegex(RuntimeError, "bounding boxes"): parse_pdf(Path("cv/main_example.pdf")) + def test_poppler_abort_on_empty_info_string_names_that_cause_too(self): + """Poppler 26.0x before 26.05 aborts -bbox on an empty Info-dict string, e.g. + the empty /Title hyperref writes when pdftitle is unset (#451). That crash is + not the xpdf-shadowing case - it has no -bbox flag and exits 99 - so the + message must name both, not just the one the exit code happens to match. + """ + failure = subprocess.CalledProcessError( + 1, + "pdftotext", + stderr="libc++abi: terminating due to uncaught exception of type " + "std::out_of_range: basic_string", + ) + with patch("tools.verify_layout.shutil.which", return_value="/usr/bin/pdftotext"), patch( + "tools.verify_layout.subprocess.run", side_effect=failure + ): + with self.assertRaisesRegex(RuntimeError, "bounding boxes") as ctx: + parse_pdf(Path("cv/main_example.pdf")) + message = str(ctx.exception) + self.assertIn("xpdf", message) + self.assertIn("Poppler aborted", message) + self.assertIn("hyperref", message) + def test_missing_poppler_raises_a_skippable_error(self): with patch("tools.verify_layout.shutil.which", return_value=None): with self.assertRaisesRegex(RuntimeError, "not found"): diff --git a/tools/verify_layout.py b/tools/verify_layout.py index d0b10d6..0a330aa 100644 --- a/tools/verify_layout.py +++ b/tools/verify_layout.py @@ -26,9 +26,14 @@ that, and CI runs it. Two implementations of one rule drift. Geometry comes from Poppler word bounding boxes (`pdftotext -bbox`). Poppler is optional repo-wide - since #369 `verify_pdf.py` prefers pypdf and falls back to Poppler - but word bounding boxes have no pypdf equivalent, so this is the one step that still wants it. -Without it, or with an extractor that cannot do `-bbox` (Git-for-Windows ships an -xpdf-based `pdftotext` that shadows Poppler in PATH and rejects the flag), the check -reports `skipped:` and exits 2 rather than inventing a layout failure. Line height serves +Without it, or with a broken extractor, the check reports `skipped:` and exits 2 rather +than inventing a layout failure. A broken extractor has two distinct causes: an xpdf-based +`pdftotext` (Git for Windows ships one ahead of Poppler in PATH) has no `-bbox` flag at +all, and Poppler 26.0x before 26.05 aborts `-bbox`/`-bbox-layout`/`-htmlmeta` on a PDF +whose Info dictionary carries an empty string in any field - which `hyperref` writes for +every field it does not set, so any `lualatex`/`pdflatex` document built with `hyperref` +and no `\hypersetup{pdftitle=...}` triggers a real Poppler crashing on a legal PDF (#451). +Line height serves as a font-size proxy to spot section headings; left edge (xMin) separates bullet lines from entry headers. @@ -166,14 +171,22 @@ def parse_pdf(path: Path) -> list[Page]: check=True, ).stdout except subprocess.CalledProcessError as exc: - # An xpdf-based pdftotext has no -bbox and exits 99. That is a broken extractor, - # not a broken document, so it degrades to the skip path instead of exit 1. + # A broken extractor has two distinct causes, not one: an xpdf-based pdftotext + # has no -bbox at all and exits 99 (Git for Windows puts one ahead of Poppler + # in PATH); a real Poppler before 26.05 aborts -bbox on a PDF whose Info dict + # has an empty string in any field - which hyperref writes for every field it + # does not set, so a lualatex/pdflatex document with hyperref and no pdftitle + # triggers this even with a working Poppler (#451). Either way this is a + # broken extractor, not a broken document, so it degrades to the skip path + # instead of exit 1. stderr_lines = (exc.stderr or "").strip().splitlines() detail = stderr_lines[0] if stderr_lines else f"exit {exc.returncode}" raise RuntimeError( f"pdftotext could not produce bounding boxes for {path} ({detail}); " - "a pdftotext without -bbox is usually the xpdf build that Git for Windows " - "puts ahead of Poppler in PATH" + "either the pdftotext first in PATH is an xpdf build with no -bbox flag " + "(Git for Windows ships one ahead of Poppler), or Poppler aborted on this " + "document - Poppler 26.0x before 26.05 aborts on a PDF whose Info " + "dictionary carries an empty string, as hyperref writes when pdftitle is unset" ) from exc pages = [] for _w, h, body in PAGE_RE.findall(out):