fix(make-pdf): pdftotext version and flavor probe returns unknown on poppler

describeBinary reports version="unknown" flavor="unknown" for every poppler
install, so logDiagnostics prints nothing useful on the most common
implementation. Two independent causes:

1. poppler writes the -v banner to stderr and exits 0. execFileSync returns
   stdout (empty) and does not throw on a zero exit, so the stderr fallback in
   the catch block is unreachable. The in-code comment already notes poppler
   exits 0, but only the throwing path reads stderr.

2. flavor is matched against the version line alone. poppler prints
   "pdftotext version 26.06.0" on line 1 and names itself on line 2,
   "Copyright ... The Poppler Developers", so even a working stderr read
   yields "unknown".

Switch the probe to spawnSync, which returns both streams regardless of exit
status, match the version banner rather than assuming line 0, and derive the
flavor from the full output.

Measured on poppler 26.06.0 (Homebrew, macOS), same machine and binary:

  before: { version: "unknown",                  flavor: "unknown" }
  after:  { version: "pdftotext version 26.06.0", flavor: "poppler" }

xpdf is unaffected: it exits non-zero and names itself on line 1, so it
resolved correctly before and still does.

Tests use shell shims reproducing each vendor's banner, stream and exit status,
since a real pdftotext cannot be assumed present in CI. Two of the four fail on
this commit's parent; the xpdf and no-banner cases pass there and are included
as regression guards rather than red-proofs.
This commit is contained in:
Paul Snyman
2026-08-31 20:56:33 +00:00
committed by Garry Tan
parent 8c7ff15fc4
commit f7e5426792
2 changed files with 115 additions and 14 deletions
+20 -14
View File
@@ -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 };
+95
View File
@@ -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");
},
);
});