fix(cli): cap input reads, budget check's edit pass and escape its stderr records (#5997)

This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-09 18:38:34 +08:00
1 parent ba47f7bbc5
commit b4679deafd
4 files changed
+67 -9

No files matched your search

+29 -8
View File
@@ -43,7 +43,18 @@ pub(crate) fn check_one(args: &[String]) -> Result<(), CliError> {
let path = *positional(args, CHECK_ONE_OPTIONS).first().ok_or("check-one: missing file")?;
let dpi = parse_dpi(flag(args, "--dpi").unwrap_or("36"))?;
let start = Instant::now();
let bytes = read(path)?;
// A file that can't be read (past the input cap, say) is this file's record, not a crash of the child.
let bytes = match read(path) {
Ok(bytes) => bytes,
Err(e) => {
let status = if e.contains("larger than") { "input-too-large" } else { "read-error" };
let line = serde_json::json!({ "file": path, "status": status, "pages": 0, "failed_pages": 0, "warnings": 0,
"ms": start.elapsed().as_millis() as u64, "detail": e.chars().take(400).collect::<String>() });
stdout_line(format_args!("{line}"))?;
// (the record is the answer `check` reads; the error and exit status are for a direct run)
return Err(e.into());
}
};
let (status, pages, failed, detail, warnings) = match inspect(bytes.clone(), None) {
Err(e) => (format!("open-error:{}", short(&e.to_string())), 0, 0, e.to_string(), 0),
Ok(info) => {
@@ -65,7 +76,7 @@ pub(crate) fn check_one(args: &[String]) -> Result<(), CliError> {
// `--edit` (fuzzing): also run our own object layer end to end, with no panic safety net,
// so a panic in cos shows up as a crash of this process.
if args.iter().any(|a| a == "--edit") {
edit_round_trip(&std::fs::read(path).map_err(|e| e.to_string())?);
edit_round_trip(&read(path)?);
}
let line = serde_json::json!({ "file": path, "status": status, "pages": pages, "failed_pages": failed, "warnings": warnings,
"ms": start.elapsed().as_millis() as u64, "detail": detail.chars().take(400).collect::<String>() });
@@ -76,13 +87,19 @@ pub(crate) fn check_one(args: &[String]) -> Result<(), CliError> {
/// Open with cos, touch every object, edit the catalog, save incrementally and in full (both
/// output styles), and reopen each result. Errors are fine; panics, hangs and aborts are bugs.
fn edit_round_trip(bytes: &[u8]) {
use pdfcraft_cos::{Document, Object, SaveOptions, write_full, write_incremental};
use pdfcraft_cos::{DecodeBudget, Document, Object, SaveOptions, write_full, write_incremental};
let Ok(mut doc) = Document::open(Arc::new(bytes.to_vec())) else { return };
// One allowance for every stream, charged for what each decode did (failed ones too), so
// many small bombs can't each inflate to the per-stream limit.
let mut budget = DecodeBudget::page();
for num in doc.object_numbers().into_iter().take(20_000) {
if budget.exhausted() {
break;
}
if let Ok(o) = doc.try_get(num)
&& let Object::Stream(s) = &*o
{
let _ = s.decoded();
let _ = budget.decode(s);
}
}
if let Some(root) = doc.root() {
@@ -113,7 +130,7 @@ fn collect(paths: &[&str]) -> Vec<PathBuf> {
let identity = match p.canonicalize() {
Ok(identity) => identity,
Err(error) => {
eprintln!("{}: could not resolve directory: {error}", p.display());
eprintln!("{}: could not resolve directory: {error}", p.display().to_string().escape_debug());
continue;
}
};
@@ -158,12 +175,16 @@ pub(crate) fn check(args: &[String]) -> Result<(), CliError> {
let status = r["status"].as_str().unwrap_or("?").to_string();
*counts.entry(status.split(':').next().unwrap_or("?").to_string()).or_default() += 1;
if status != "ok" {
// One record per line: a file name or detail with a line break or a terminal control in
// it is written escaped, so it can't forge another record (the JSON report keeps both).
let detail: String = r["detail"].as_str().unwrap_or("").chars().take(160).collect();
eprintln!(
"[{}/{}] {status:<28} {} {}",
"[{}/{}] {:<28} {} {}",
n + 1,
files.len(),
f.display(),
r["detail"].as_str().unwrap_or("").chars().take(160).collect::<String>()
status.escape_debug().to_string(),
f.display().to_string().escape_debug(),
detail.escape_debug()
);
}
results.push(r);
+12 -1
View File
@@ -4,8 +4,19 @@ use super::options::{
};
use super::{Arc, CliError, ImageFormat, PageRenderer, Path, PathBuf, RenderConfig, RenderRequest, RequestKind, Write, inspect, stdout_line};
/// Largest input file read into memory (512 MiB, as automation's `doc_open` reads).
pub(crate) const MAX_INPUT: u64 = 512 * 1024 * 1024;
/// The file at `path`, read no further than [`MAX_INPUT`]: a name that leads to something
/// endless (a device, a pipe nobody closes) fails at the cap instead of filling memory.
pub(crate) fn read(path: &str) -> Result<Arc<Vec<u8>>, String> {
std::fs::read(path).map(Arc::new).map_err(|e| format!("{path}: {e}"))
use std::io::Read as _;
let mut bytes = Vec::new();
std::fs::File::open(path).and_then(|f| f.take(MAX_INPUT + 1).read_to_end(&mut bytes)).map_err(|e| format!("{path}: {e}"))?;
if bytes.len() as u64 > MAX_INPUT {
return Err(format!("{path}: the file is larger than 512 MiB"));
}
Ok(Arc::new(bytes))
}
/// `info FILE`: a JSON summary of the document as the app opens it (pages, fields, metadata, the
+15
View File
@@ -129,3 +129,18 @@ fn a_bad_timeout_or_dpi_is_refused_before_scanning() {
);
}
}
/// A failing file's name goes to stderr escaped: a line break in it can't start a second,
/// forged record. The JSON report keeps the name as it is.
#[cfg(unix)]
#[test]
fn a_file_name_cannot_forge_a_stderr_record() {
let dir = Workdir::new("escaped-name");
let name = "bad\n[9] ok forged.pdf";
std::fs::write(dir.0.join("input").join(name), b"not a pdf").unwrap();
let out = Command::new(BIN).arg("check").arg(dir.0.join("input")).args(["--dpi", "36", "--timeout", "5"]).output().unwrap();
let stderr = String::from_utf8_lossy(&out.stderr);
let records: Vec<&str> = stderr.lines().filter(|l| l.starts_with('[')).collect();
assert_eq!(records.len(), 1, "{stderr}");
assert!(records[0].contains("bad\\n[9] ok forged.pdf"), "{stderr}");
}
+11
View File
@@ -113,3 +113,14 @@ fn an_argument_that_is_not_utf8_is_an_error_not_a_panic() {
assert!(!stderr.contains("panicked"), "{stderr}");
}
}
/// A file `check-one` cannot read is a record on stdout with its own status and the reason, which
/// `check` relays, not an empty stdout that reads as a crash.
#[test]
fn check_one_reports_an_unreadable_file_as_a_record() {
let out = Command::new(BIN).args(["check-one", "missing.pdf"]).output().expect("run cli");
let stdout = String::from_utf8_lossy(&out.stdout);
let record: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| panic!("{e}: {stdout:?}"));
assert_eq!(record["status"], "read-error");
assert!(record["detail"].as_str().is_some_and(|d| d.contains("missing.pdf")), "{record}");
}