diff --git a/make-pdf/src/pdftotext.ts b/make-pdf/src/pdftotext.ts index 5cdb51e81..df6d05335 100644 --- a/make-pdf/src/pdftotext.ts +++ b/make-pdf/src/pdftotext.ts @@ -26,7 +26,7 @@ * Only the CI gate and unit tests invoke pdftotext. */ -import { execFileSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -154,19 +154,25 @@ function isExecutable(p: string): boolean { function describeBinary(bin: string): PdftotextInfo { let version = "unknown"; let flavor: PdftotextInfo["flavor"] = "unknown"; - try { - // pdftotext -v writes to stderr and exits 0 on poppler, 99 on some xpdf builds. - const result = execFileSync(bin, ["-v"], { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - }); - version = (result || "").trim().split("\n")[0] || "unknown"; - } catch (err: any) { - // Many pdftotext builds exit non-zero on -v but still write to stderr. - const stderr = err?.stderr?.toString?.() ?? ""; - version = stderr.trim().split("\n")[0] || "unknown"; - } - const v = version.toLowerCase(); + + // spawnSync, not execFileSync: poppler writes -v output to STDERR and exits 0, + // so execFileSync neither returns it (it returns stdout, which is empty) nor + // throws (which is what made the stderr fallback below reachable). The result + // was version="unknown" flavor="unknown" on every poppler install. spawnSync + // hands back both streams regardless of exit status, which also covers the + // xpdf builds that exit 99. + const res = spawnSync(bin, ["-v"], { encoding: "utf8" }); + const raw = `${res.stdout ?? ""}\n${res.stderr ?? ""}`; + + // The version banner is not reliably the first line once both streams are in + // play, so match it rather than taking line 0. + const lines = raw.split("\n").map(l => l.trim()).filter(Boolean); + version = lines.find(l => /pdftotext\s+version/i.test(l)) ?? lines[0] ?? "unknown"; + + // Flavor comes from the WHOLE banner, never the version line alone: poppler + // prints "pdftotext version 26.06.0" on line 1 and only identifies itself on + // line 2, "Copyright ... The Poppler Developers". + const v = raw.toLowerCase(); if (v.includes("poppler")) flavor = "poppler"; else if (v.includes("xpdf")) flavor = "xpdf"; return { bin, version, flavor }; diff --git a/make-pdf/test/pdftotext.test.ts b/make-pdf/test/pdftotext.test.ts index 4ab5c4fb7..871325e6b 100644 --- a/make-pdf/test/pdftotext.test.ts +++ b/make-pdf/test/pdftotext.test.ts @@ -8,6 +8,8 @@ import { describe, expect, test } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; import * as path from "node:path"; import { normalize, copyPasteGate, findExecutable, resolvePdftotext, PdftotextUnavailableError } from "../src/pdftotext"; @@ -205,3 +207,96 @@ describe("resolvePdftotext (override resolution, v1.24-aligned)", () => { } }); }); + +// ─── Version + flavor probe (describeBinary via resolvePdftotext) ──── +// +// Regression cover for the probe returning version="unknown" flavor="unknown" +// on every poppler install. Two independent causes, both exercised here: +// +// 1. poppler writes its -v banner to STDERR and exits 0. The old code used +// execFileSync, which returns stdout (empty) and does not throw on a zero +// exit, so the stderr fallback in the catch block was unreachable. +// 2. flavor was matched against the first line only. poppler prints +// "pdftotext version X" on line 1 and identifies itself on line 2 +// ("Copyright ... The Poppler Developers"), so even a working stderr read +// yielded flavor="unknown". +// +// Real pdftotext binaries cannot be assumed present in CI, so these use shell +// shims that reproduce each vendor's exact banner, stream and exit status. + +describe("describeBinary (version + flavor probe)", () => { + const shimDir = fs.mkdtempSync(path.join(os.tmpdir(), "pdftotext-shim-")); + + function shim(name: string, body: string): string { + if (process.platform === "win32") return ""; + const p = path.join(shimDir, name); + fs.writeFileSync(p, `#!/bin/sh\n${body}\n`, { mode: 0o755 }); + return p; + } + + // poppler: banner on stderr, exit 0, vendor named on line 2. + const popplerShim = shim( + "poppler-pdftotext", + [ + 'if [ "$1" = "-v" ]; then', + ' echo "pdftotext version 26.06.0" >&2', + ' echo "Copyright 2005-2026 The Poppler Developers - http://poppler.freedesktop.org" >&2', + ' echo "Copyright 1996-2011, 2022 Glyph & Cog, LLC" >&2', + " exit 0", + "fi", + "exit 0", + ].join("\n"), + ); + + // xpdf: banner on stderr, non-zero exit, vendor named on line 1. + const xpdfShim = shim( + "xpdf-pdftotext", + [ + 'if [ "$1" = "-v" ]; then', + ' echo "pdftotext version 4.05 [xpdf]" >&2', + ' echo "Copyright 1996-2024 Glyph & Cog, LLC" >&2', + " exit 99", + "fi", + "exit 0", + ].join("\n"), + ); + + test.skipIf(process.platform === "win32")( + "reads a poppler banner from stderr on a zero exit", + () => { + const info = withEnv({ GSTACK_PDFTOTEXT_BIN: popplerShim }, () => resolvePdftotext()); + expect(info.version).toBe("pdftotext version 26.06.0"); + expect(info.flavor).toBe("poppler"); + }, + ); + + test.skipIf(process.platform === "win32")( + "identifies poppler from the copyright line, not the version line", + () => { + const info = withEnv({ GSTACK_PDFTOTEXT_BIN: popplerShim }, () => resolvePdftotext()); + // The line carrying the version does NOT contain the vendor name, which is + // exactly why a line-0-only match reported "unknown". + expect(info.version.toLowerCase()).not.toContain("poppler"); + expect(info.flavor).toBe("poppler"); + }, + ); + + test.skipIf(process.platform === "win32")( + "reads an xpdf banner from stderr on a non-zero exit", + () => { + const info = withEnv({ GSTACK_PDFTOTEXT_BIN: xpdfShim }, () => resolvePdftotext()); + expect(info.version).toBe("pdftotext version 4.05 [xpdf]"); + expect(info.flavor).toBe("xpdf"); + }, + ); + + test.skipIf(process.platform === "win32")( + "falls back to unknown when the binary emits no banner", + () => { + const silent = shim("silent-pdftotext", "exit 0"); + const info = withEnv({ GSTACK_PDFTOTEXT_BIN: silent }, () => resolvePdftotext()); + expect(info.version).toBe("unknown"); + expect(info.flavor).toBe("unknown"); + }, + ); +});