diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94a4c545fe..38ee53caf3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,10 @@ jobs: permissions: contents: read steps: + - name: Harden runner + uses: step-security/harden-runner@1d88384fd173af6ecb012dfd0264a306b711241a # v2.22.4 + with: + egress-policy: audit - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml index b7b1d409c8..78dc1b614b 100644 --- a/.github/workflows/fuzz.yml +++ b/.github/workflows/fuzz.yml @@ -24,13 +24,17 @@ jobs: name: fuzz (mutation, 15 min) runs-on: ubuntu-latest steps: + - name: Harden runner + uses: step-security/harden-runner@1d88384fd173af6ecb012dfd0264a306b711241a # v2.22.4 + with: + egress-policy: audit - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - uses: dtolnay/rust-toolchain@686976e191b89faba57d3206551f0f330d8cb249 # stable + - uses: step-security/dtolnay-rust-toolchain@8dd607977d06adf4ed49b190381e74c17a9dd25f # v1.0.0 with: toolchain: stable - - uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3 + - uses: step-security/rust-cache@8871c8ba063416da7c32246932ea6ae08700453e # v2.9.2 with: key: fuzz - name: Fetch the pdf.js corpus (seeds) diff --git a/.github/workflows/packaging-lint.yml b/.github/workflows/packaging-lint.yml index ea61d84303..1771a35fea 100644 --- a/.github/workflows/packaging-lint.yml +++ b/.github/workflows/packaging-lint.yml @@ -27,6 +27,10 @@ jobs: name: Windows installation policy (no app build) runs-on: windows-latest steps: + - name: Harden runner + uses: step-security/harden-runner@1d88384fd173af6ecb012dfd0264a306b711241a # v2.22.4 + with: + egress-policy: audit - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4b44d79b68..d214d6738b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,6 +1,10 @@ name: Release on: + # A nightly dry run on main: every release build except macOS, nothing signed or released + # (craft-common-actions release.yml `dry-run`). Finds release-only breakage before release day. + schedule: + - cron: "20 2 * * *" push: branches: [release] workflow_dispatch: @@ -30,6 +34,7 @@ jobs: # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. uses: storytold/craft-common-actions/.github/workflows/release.yml@main with: + dry-run: ${{ github.event_name == 'schedule' }} app-name: pdfcraft app-title: PdfCraft craft-fonts-ref: b076417b5880bd4154bd22f00695b1d28c6bea83 diff --git a/Cargo.lock b/Cargo.lock index c4e08c1086..0e4c910427 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1289,7 +1289,7 @@ dependencies = [ [[package]] name = "craft-update" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "chrono", "dunce", @@ -1299,6 +1299,7 @@ dependencies = [ "image", "log", "plist", + "rustix 1.1.6", "semver", "serde", "serde_json", @@ -4263,7 +4264,7 @@ dependencies = [ [[package]] name = "pdfcraft" -version = "0.21.0" +version = "0.21.1" dependencies = [ "chrono", "craft-update", @@ -4288,7 +4289,7 @@ dependencies = [ [[package]] name = "pdfcraft-a11y" -version = "0.21.0" +version = "0.21.1" dependencies = [ "pdfcraft-content", "pdfcraft-cos", @@ -4299,7 +4300,7 @@ dependencies = [ [[package]] name = "pdfcraft-annot" -version = "0.21.0" +version = "0.21.1" dependencies = [ "hayro-syntax", "pdfcraft-cos", @@ -4311,7 +4312,7 @@ dependencies = [ [[package]] name = "pdfcraft-audit" -version = "0.21.0" +version = "0.21.1" dependencies = [ "pdfcraft-content", "pdfcraft-cos", @@ -4322,7 +4323,7 @@ dependencies = [ [[package]] name = "pdfcraft-automation" -version = "0.21.0" +version = "0.21.1" dependencies = [ "base64 0.23.1", "flate2", @@ -4346,7 +4347,7 @@ dependencies = [ [[package]] name = "pdfcraft-cli" -version = "0.21.0" +version = "0.21.1" dependencies = [ "base64 0.23.1", "pdfcraft-audit", @@ -4364,11 +4365,11 @@ dependencies = [ [[package]] name = "pdfcraft-compare" -version = "0.21.0" +version = "0.21.1" [[package]] name = "pdfcraft-content" -version = "0.21.0" +version = "0.21.1" dependencies = [ "hayro-syntax", "pdfcraft-cos", @@ -4376,7 +4377,7 @@ dependencies = [ [[package]] name = "pdfcraft-cos" -version = "0.21.0" +version = "0.21.1" dependencies = [ "chrono", "hayro-syntax", @@ -4393,7 +4394,7 @@ dependencies = [ [[package]] name = "pdfcraft-create" -version = "0.21.0" +version = "0.21.1" dependencies = [ "encoding_rs", "hayro-ccitt", @@ -4413,7 +4414,7 @@ dependencies = [ [[package]] name = "pdfcraft-crypt" -version = "0.21.0" +version = "0.21.1" dependencies = [ "aes", "md-5", @@ -4426,7 +4427,7 @@ dependencies = [ [[package]] name = "pdfcraft-edit" -version = "0.21.0" +version = "0.21.1" dependencies = [ "hayro-syntax", "icu_segmenter", @@ -4445,7 +4446,7 @@ dependencies = [ [[package]] name = "pdfcraft-engine" -version = "0.21.0" +version = "0.21.1" dependencies = [ "chrono", "craft-docpath", @@ -4489,14 +4490,14 @@ dependencies = [ [[package]] name = "pdfcraft-export" -version = "0.21.0" +version = "0.21.1" dependencies = [ "flate2", ] [[package]] name = "pdfcraft-filters" -version = "0.21.0" +version = "0.21.1" dependencies = [ "fax", "flate2", @@ -4509,7 +4510,7 @@ dependencies = [ [[package]] name = "pdfcraft-fonts" -version = "0.21.0" +version = "0.21.1" dependencies = [ "harfrust", "pdfcraft-cos", @@ -4519,7 +4520,7 @@ dependencies = [ [[package]] name = "pdfcraft-forms" -version = "0.21.0" +version = "0.21.1" dependencies = [ "hayro-syntax", "pdfcraft-content", @@ -4532,11 +4533,11 @@ dependencies = [ [[package]] name = "pdfcraft-geom" -version = "0.21.0" +version = "0.21.1" [[package]] name = "pdfcraft-js" -version = "0.21.0" +version = "0.21.1" dependencies = [ "boa_engine", "chrono", @@ -4544,7 +4545,7 @@ dependencies = [ [[package]] name = "pdfcraft-measure" -version = "0.21.0" +version = "0.21.1" dependencies = [ "pdfcraft-annot", "pdfcraft-content", @@ -4557,14 +4558,14 @@ dependencies = [ [[package]] name = "pdfcraft-model" -version = "0.21.0" +version = "0.21.1" dependencies = [ "pdfcraft-cos", ] [[package]] name = "pdfcraft-ocr" -version = "0.21.0" +version = "0.21.1" dependencies = [ "ocrs", "pdfcraft-fonts", @@ -4579,7 +4580,7 @@ dependencies = [ [[package]] name = "pdfcraft-ocr-worker" -version = "0.21.0" +version = "0.21.1" dependencies = [ "js-sys", "pdfcraft-ocr", @@ -4589,7 +4590,7 @@ dependencies = [ [[package]] name = "pdfcraft-optimize" -version = "0.21.0" +version = "0.21.1" dependencies = [ "image", "pdfcraft-annot", @@ -4606,11 +4607,11 @@ dependencies = [ [[package]] name = "pdfcraft-oracle" -version = "0.21.0" +version = "0.21.1" [[package]] name = "pdfcraft-organize" -version = "0.21.0" +version = "0.21.1" dependencies = [ "hayro-syntax", "pdfcraft-content", @@ -4621,7 +4622,7 @@ dependencies = [ [[package]] name = "pdfcraft-platform" -version = "0.21.0" +version = "0.21.1" dependencies = [ "async-channel", "futures-lite", @@ -4632,7 +4633,7 @@ dependencies = [ [[package]] name = "pdfcraft-prc" -version = "0.21.0" +version = "0.21.1" dependencies = [ "flate2", "proptest", @@ -4641,7 +4642,7 @@ dependencies = [ [[package]] name = "pdfcraft-prc-render" -version = "0.21.0" +version = "0.21.1" dependencies = [ "flate2", "pdfcraft-prc", @@ -4650,7 +4651,7 @@ dependencies = [ [[package]] name = "pdfcraft-preflight" -version = "0.21.0" +version = "0.21.1" dependencies = [ "flate2", "pdfcraft-content", @@ -4661,7 +4662,7 @@ dependencies = [ [[package]] name = "pdfcraft-print" -version = "0.21.0" +version = "0.21.1" dependencies = [ "craft-print", "log", @@ -4676,7 +4677,7 @@ dependencies = [ [[package]] name = "pdfcraft-redact" -version = "0.21.0" +version = "0.21.1" dependencies = [ "aho-corasick", "pdfcraft-annot", @@ -4700,7 +4701,7 @@ dependencies = [ [[package]] name = "pdfcraft-render" -version = "0.21.0" +version = "0.21.1" dependencies = [ "bytemuck", "flate2", @@ -4724,7 +4725,7 @@ dependencies = [ [[package]] name = "pdfcraft-sign" -version = "0.21.0" +version = "0.21.1" dependencies = [ "aes", "aws-lc-rs", @@ -4763,11 +4764,11 @@ dependencies = [ [[package]] name = "pdfcraft-testkit" -version = "0.21.0" +version = "0.21.1" [[package]] name = "pdfcraft-ui-egui" -version = "0.21.0" +version = "0.21.1" dependencies = [ "arboard", "base64 0.23.1", @@ -4826,7 +4827,7 @@ dependencies = [ [[package]] name = "pdfcraft-web" -version = "0.21.0" +version = "0.21.1" dependencies = [ "craft-web-api", "craft-web-host", @@ -4841,7 +4842,7 @@ dependencies = [ [[package]] name = "pdfcraft-xfa" -version = "0.21.0" +version = "0.21.1" dependencies = [ "base64 0.23.1", "image", @@ -4855,7 +4856,7 @@ dependencies = [ [[package]] name = "pdfcraft-xfdf" -version = "0.21.0" +version = "0.21.1" dependencies = [ "base64 0.23.1", "pdfcraft-annot", diff --git a/Cargo.toml b/Cargo.toml index eaf28264e5..752f580475 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ exclude = ["vendor"] default-members = ["crates/*", "apps/pdfcraft", "apps/pdfcraft-cli", "xtask"] [workspace.package] -version = "0.21.0" +version = "0.21.1" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" @@ -30,7 +30,7 @@ craft-print = { git = "https://github.com/storytold/craft-libs", version = "0.1. # Whether a path stored in a document may be opened (GoToR link targets). craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "787a7a0f972fd1d7a7618fdbeca3a5267a0407ff" } # craft-update: app-specific pretend versions apply to startup and manual checks. -craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "1b34eba41e62138fc2438d2734523b302840b45d", features = ["egui"] } +craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "6e03f65bf45181db77ba6229716e6624db9eb25b", features = ["egui"] } # internal pdfcraft-geom = { path = "crates/geom", version = "0.21.0" } pdfcraft-filters = { path = "crates/filters", version = "0.21.0" } diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index 235ba4b452..a9de970b65 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -206,7 +206,9 @@ fn sub(doc: &Document, d: &Dict, key: &[u8]) -> Dict { pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { let res = sub(doc, page, b"Resources"); let (fonts, props) = (sub(doc, &res, b"Font"), sub(doc, &res, b"Properties")); - let mut metrics: HashMap, Metrics> = HashMap::new(); + let mut metrics: HashMap, std::rc::Rc> = HashMap::new(); + // Built once per font object, however many resource names alias it. + let mut by_font: HashMap> = HashMap::new(); // The id each open marked-content sequence is in: its own, else the enclosing one's. let mut stack: Vec> = Vec::new(); let mut out: HashMap = HashMap::new(); @@ -255,9 +257,30 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { gs.font = op.name(0).map(<[u8]>::to_vec); if let Some(name) = op.name(0) && !metrics.contains_key(name) - && let Some(f) = fonts.get(name).map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()) { - metrics.insert(name.to_vec(), Metrics::from_dict(doc, &f)); + // Keyed by the object at the end of the reference chain, so names that reach one + // font through different chains share its metrics (the hop limit is resolve's). + let font = fonts.get(name).and_then(|o| { + let (mut cur, mut last) = (o.clone(), None); + for _ in 0..16 { + let Object::Ref(r) = cur else { break }; + last = Some(r); + cur = (*doc.get(r)).clone(); + } + last + }); + let built = match font.and_then(|r| by_font.get(&r)) { + Some(m) => Some(m.clone()), + None => { + fonts.get(name).map(|f| doc.resolve(f)).and_then(|f| f.as_dict().map(|d| std::rc::Rc::new(Metrics::from_dict(doc, d)))) + } + }; + if let Some(m) = built { + if let Some(r) = font { + by_font.insert(r, m.clone()); + } + metrics.insert(name.to_vec(), m); + } } } b"Tc" => gs.char_spacing = op.num(0).unwrap_or(0.0), diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index ae2222b387..6566337ca1 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -857,19 +857,38 @@ pub fn set_points(doc: &mut Document, page: usize, index: usize, points: &[[f64; /// Scale a drawing's strokes into `rect`. The margin between the strokes and the old `/Rect` (half /// the line width and a little more) stays the same on each side, so the line isn't clipped. +/// Most stroke points a stored drawing may have for resizing or erasing it. Counted as each +/// stroke is resolved, so one stroke array referenced again and again can't expand past it. +const MAX_STORED_INK_POINTS: usize = 1_000_000; +/// Most points the eraser densifies a drawing into before testing them against its path. +const MAX_ERASE_SAMPLES: usize = 4_000_000; + +/// The strokes of an ink annotation's `/InkList`, at most [`MAX_STORED_INK_POINTS`] points. +fn ink_strokes(doc: &Document, d: &Dict) -> Result>, AnnotError> { + let too_many = || AnnotError::Invalid(format!("the drawing has more than {MAX_STORED_INK_POINTS} points")); + let list = d.get(b"InkList").map(|l| doc.resolve(l)); + let mut points = 0usize; + let mut strokes = Vec::new(); + for s in list.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default() { + let stroke = doc.resolve(s); + let Some(a) = stroke.as_array() else { + strokes.push(Vec::new()); + continue; + }; + // Charged before the coordinates are read: a shared array costs each time it is listed. + points = points.saturating_add(a.len() / 2); + if points > MAX_STORED_INK_POINTS { + return Err(too_many()); + } + let v: Vec = a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect(); + strokes.push(v.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect()); + } + Ok(strokes) +} + fn resize_ink(doc: &mut Document, r: ObjRef, rect: [f64; 4], meta: &Meta) -> Result<(), AnnotError> { let d = annot_dict(doc, r); - let strokes: Vec> = d - .get(b"InkList") - .map(|l| doc.resolve(l)) - .and_then(|l| l.as_array().cloned()) - .unwrap_or_default() - .iter() - .map(|s| { - let v: Vec = doc.resolve(s).as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect()).unwrap_or_default(); - v.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect() - }) - .collect(); + let strokes = ink_strokes(doc, &d)?; let Some(inner) = bounds(strokes.iter().flatten().copied()) else { return Err(AnnotError::Invalid("the drawing has no strokes".into())); }; @@ -1475,17 +1494,7 @@ pub fn erase_ink(doc: &mut Document, page: usize, index: usize, path: &[[f64; 2] if path.is_empty() || !path.iter().all(|p| finite(p)) || !radius.is_finite() || radius <= 0.0 { return Err(AnnotError::Invalid("invalid eraser path".into())); } - let strokes: Vec> = d - .get(b"InkList") - .map(|l| doc.resolve(l)) - .and_then(|l| l.as_array().cloned()) - .unwrap_or_default() - .iter() - .map(|s| { - let v: Vec = doc.resolve(s).as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect()).unwrap_or_default(); - v.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect() - }) - .collect(); + let strokes = ink_strokes(doc, &d)?; // Distance from a point to the eraser's path (segments, or a single point). let near = |p: [f64; 2]| -> bool { if path.len() == 1 { @@ -1501,17 +1510,27 @@ pub fn erase_ink(doc: &mut Document, page: usize, index: usize, path: &[[f64; 2] }; let mut kept: Vec> = Vec::new(); let mut erased = false; + let mut samples = 0usize; for s in &strokes { // Densify so long segments are cut where the eraser crosses them. let mut pts: Vec<[f64; 2]> = Vec::new(); for w in s.windows(2) { let n = ((w[1][0] - w[0][0]).hypot(w[1][1] - w[0][1]) / (radius / 2.0)).ceil().clamp(1.0, 2000.0) as usize; + samples = samples.saturating_add(n); + if samples > MAX_ERASE_SAMPLES { + return Err(AnnotError::Invalid("the drawing is too large to erase at this eraser size".into())); + } for k in 0..n { let t = k as f64 / n as f64; pts.push([w[0][0] + (w[1][0] - w[0][0]) * t, w[0][1] + (w[1][1] - w[0][1]) * t]); } } if let Some(last) = s.last() { + // The final point is a sample too. + samples = samples.saturating_add(1); + if samples > MAX_ERASE_SAMPLES { + return Err(AnnotError::Invalid("the drawing is too large to erase at this eraser size".into())); + } pts.push(*last); } let mut run: Vec<[f64; 2]> = Vec::new(); @@ -1537,6 +1556,12 @@ pub fn erase_ink(doc: &mut Document, page: usize, index: usize, path: &[[f64; 2] delete_annotation(doc, page, index)?; return Ok(true); } + // The retained runs are stored as the new `/InkList`: past the cap, writing them would leave a + // drawing that later `resize_ink` and `erase_ink` refuse to read. + let points: usize = kept.iter().map(Vec::len).sum(); + if points > MAX_STORED_INK_POINTS { + return Err(AnnotError::Invalid(format!("erasing would leave a drawing with more than {MAX_STORED_INK_POINTS} points"))); + } let width = d.get(b"BS").and_then(|b| b.as_dict()).and_then(|b| b.get(b"W")).and_then(|w| w.as_f64()).unwrap_or(1.0); let rect = grow(bounds(kept.iter().flatten().copied()).unwrap_or_default(), width / 2.0 + 1.0); doc.update_dict(r, |d| { diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index ce5a8c1604..980959d4d9 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -4797,6 +4797,73 @@ fn freehand_highlight_is_compatible_ink_with_marker_blending() { } } +/// A drawing whose /InkList lists one stroke array over and over is refused once its points +/// pass the cap, counted as each stroke is resolved, before they are collected or scaled. +#[test] +fn ink_points_are_counted_as_strokes_resolve() { + let numbers: String = (0..2000).map(|k| format!(" {}", k % 100)).collect(); + let refs = " 5 0 R".repeat(501); + let doc_with = |refs: &str| { + doc_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(), + format!("<< /Type /Annot /Subtype /Ink /Rect [0 0 100 100] /InkList [{refs}] >>"), + format!("[{numbers}]"), + ]) + }; + // 501 × 1,000 points is under the cap: the points are read (this bare drawing then has no + // appearance it can be redrawn with, which is another matter). + let mut doc = doc_with(&refs); + let under = set_rect(&mut doc, 0, 0, [0.0, 0.0, 50.0, 50.0], &meta("")); + assert!(!matches!(&under, Err(AnnotError::Invalid(m)) if m.contains("points")), "{under:?}"); + // 1,001 × 1,000 points is over it. + let mut doc = doc_with(&" 5 0 R".repeat(1001)); + match set_rect(&mut doc, 0, 0, [0.0, 0.0, 50.0, 50.0], &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("more than 1000000 points"), "{m}"), + other => panic!("refused: {other:?}"), + } + match erase_ink(&mut doc, 0, 0, &[[1.0, 1.0]], 1.0, &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("more than 1000000 points"), "{m}"), + other => panic!("refused: {other:?}"), + } +} + +/// The eraser's densified samples have a total: a tiny eraser over long strokes is refused +/// instead of interpolating millions of points. +#[test] +fn the_eraser_densifies_within_a_total() { + let zigzag: String = (0..2100).map(|k| format!(" {} {}", if k % 2 == 0 { 0 } else { 5000 }, k)).collect(); + let mut doc = doc_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(), + format!("<< /Type /Annot /Subtype /Ink /Rect [0 0 5000 2100] /InkList [[{zigzag}]] >>"), + ]); + match erase_ink(&mut doc, 0, 0, &[[1.0, 1.0]], 0.01, &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("too large to erase"), "{m}"), + other => panic!("refused: {other:?}"), + } +} + +/// Each stroke's final point is a sample too: 500,000 strokes of two points at 8 samples a segment +/// are 4,000,000 densified samples and as many final points again, past the total. +#[test] +fn the_eraser_counts_each_strokes_final_point() { + let refs = " 5 0 R".repeat(500_000); + let mut doc = doc_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(), + format!("<< /Type /Annot /Subtype /Ink /Rect [0 0 100 100] /InkList [{refs}] >>"), + "[0 0 8 0]".into(), + ]); + match erase_ink(&mut doc, 0, 0, &[[1.0, 1.0]], 2.0, &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("too large to erase"), "{m}"), + other => panic!("refused: {other:?}"), + } +} + /// A 3D annotation is PDF 1.6 and a PRC model 1.7 (ISO 32000-2 §13.6): an older document's /// catalog says so, and a newer one is left as it is. #[test] diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 1f2b925b46..b956cb7079 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -14042,6 +14042,39 @@ fn review_inside_scope_group_move_matches_direct_transform_under_outer_scale() { } } +/// A page that names one font under 300 resource names builds its metrics once: each alias +/// used to build (and keep) a set of its own. +#[test] +fn font_aliases_share_one_set_of_metrics() { + let fonts: String = (0..300).map(|i| format!(" /A{i} 5 0 R")).collect(); + let content: String = format!("BT 72 700 Td {} ET", (0..300).map(|i| format!("/A{i} 12 Tf (x) Tj ")).collect::()); + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font <<{fonts} >> >> >>"), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offs = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offs.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let x = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offs { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{x}\n%%EOF\n", objs.len() + 1).as_bytes()); + let doc = Document::open(Arc::new(out)).unwrap(); + text::METRICS_BUILT.with(|n| n.set(0)); + let lines = text::text_lines(&doc, 0).unwrap(); + assert!(lines.iter().map(|l| l.text.matches('x').count()).sum::() >= 300, "{lines:?}"); + let built = text::METRICS_BUILT.with(std::cell::Cell::get); + assert!(built < 10, "metrics built {built} times for one font"); +} + /// Each `Tj`/`TJ` moves the text matrix by the width of its text. A run shown later in the same /// text object without a `Td` or `Tm` of its own (a raised run in another colour, a column after /// a `TJ` gap) is placed by those advances. Rewriting the paragraph before it must keep them, as diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index b58c41303f..eba8834c2b 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -54,6 +54,8 @@ pub(crate) use fallback::{SOURCE_SPACE_CALLS, take_source_character_measurements #[cfg(test)] use fallback::{add_type3_font, is_type3_fallback}; pub(crate) use fallback::{glyph_path, type3_font_dict, unique_font_name}; +#[cfg(test)] +pub(crate) use interp::METRICS_BUILT; use interp::{Origin, Paint, union}; #[cfg(test)] pub(crate) use lines::dropped_advances_within; diff --git a/crates/edit/src/text/editor.rs b/crates/edit/src/text/editor.rs index 4f4abf3a43..e6df1dbfee 100644 --- a/crates/edit/src/text/editor.rs +++ b/crates/edit/src/text/editor.rs @@ -2,7 +2,7 @@ use super::blocks::{BlockStyle, TextBlock}; use super::fallback::MAX_TYPE3_GLYPHS; use super::lines::{page_dict, text_lines}; use super::rewrite::rewrite_block_from; -use super::{DecodeBudget, Dict, Document, EditError, Metrics, Object, TextLine, TextSession}; +use super::{DecodeBudget, Dict, Document, EditError, Object, TextLine, TextSession}; /// An embedded source font or an approved bundled document-font choice. /// Source bytes stay in memory and are never installed or copied into another document. @@ -45,7 +45,7 @@ pub fn text_editor_fonts(doc: &Document, page: usize) -> Result, Option, Result, String>); +#[cfg(test)] +thread_local! { + /// Font metrics built by [`interpret`] on this thread, for the test that keeps font aliases + /// sharing one set. + pub(crate) static METRICS_BUILT: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// A `Do` met while interpreting: its operator index, XObject name and the state there. type Invocation = (usize, Vec, Ts); @@ -976,6 +983,7 @@ pub(super) fn interpret( let fonts_res = super::resource_dict(doc, resources, b"Font", &mut held_fonts).unwrap_or(&none); let states = super::resource_dict(doc, resources, b"ExtGState", &mut held_states).unwrap_or(&none); let mut out: Vec = Vec::new(); + let mut by_font: HashMap = HashMap::new(); let mut ts = carry.ts.clone(); let mut at_bt = (0usize, ts.clone()); let mut stack: Vec<(Ts, Matrix, Matrix)> = std::mem::take(&mut carry.stack); @@ -1105,11 +1113,23 @@ pub(super) fn interpret( ts.size = op.num(1).unwrap_or(ts.size); if let Some(name) = op.name(0) { let (m, face, vertical) = cache.entry(name.to_vec()).or_insert_with(|| { + // Names that refer to one font object share what was built for it: a + // thousand aliases of one font are one set of metrics, not a thousand. + let font = fonts_res.get(name).and_then(Object::as_ref); + if let Some(built) = font.and_then(|r| by_font.get(&r)) { + return built.clone(); + } + #[cfg(test)] + METRICS_BUILT.with(|n| n.set(n.get() + 1)); let dict = fonts_res.get(name).and_then(|f| doc.resolve(f).as_dict().cloned()); let metrics = dict.as_ref().map_or_else(Metrics::fallback, |d| Metrics::from_dict(doc, d)); let vertical = dict.as_ref().map_or(Ok(None), |d| vertical_advance(doc, d).map_err(|e| e.to_string())); let face = dict.filter(is_type3_fallback).map(|_| metrics.base_font.clone()); - (Rc::new(metrics), face, vertical) + let built: CachedFont = (Rc::new(metrics), face, vertical); + if let Some(r) = font { + by_font.insert(r, built.clone()); + } + built }); ts.font = Some((name.to_vec(), m.clone())); ts.face = face.clone(); diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index 602410922f..805e089dd7 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -958,41 +958,60 @@ pub(crate) fn recovered_render_password(cos: &pdfcraft_cos::Document) -> Option< /// the document as it was. pub(crate) fn shown_under_redactions( after: &pdfcraft_cos::Document, - marks: &[pdfcraft_redact::Mark], + marks: &[(pdfcraft_cos::ObjRef, pdfcraft_redact::Mark)], overlays: &[pdfcraft_cos::ObjRef], - pages: &[pdfcraft_render::PageInfo], password: Option<&str>, ) -> Result<(), EditError> { use pdfcraft_cos::Object; use pdfcraft_redact::RedactError; - let mut marked: Vec = marks.iter().map(|m| m.page).collect(); + let mut shown = after.clone(); + let model = pdfcraft_model::pages(&shown); + // A batch can move pages after the marks were collected: a mark is found by the page object it + // was placed on, not by the index that page had when it was drawn. A page the batch deleted + // takes its marks with it, so they are not checked against whatever page now holds the index. + // One index, built once: each mark is resolved against it instead of scanning every page, + // and the marks are grouped by resolved page in the same pass. + let mut index: std::collections::HashMap = std::collections::HashMap::new(); + for (i, p) in model.iter().enumerate() { + index.entry(p.obj).or_insert(i); + } + let mut marked: Vec = Vec::new(); + let mut rects_of: std::collections::HashMap> = std::collections::HashMap::new(); + for (obj, m) in marks { + let Some(&pi) = index.get(obj) else { continue }; + if !rects_of.contains_key(&pi) { + marked.push(pi); + } + rects_of.entry(pi).or_default().extend(m.rects.iter().map(|r| [r[0].min(r[2]), r[1].min(r[3]), r[0].max(r[2]), r[1].max(r[3])])); + } marked.sort_unstable(); - marked.dedup(); if marked.is_empty() { return Ok(()); } - let mut shown = after.clone(); - let model = pdfcraft_model::pages(&shown); for &pi in &marked { let Some(page) = model.get(pi) else { continue }; let Some(Object::Array(list)) = page.dict.get(b"Contents").map(|c| (*shown.resolve(c)).clone()) else { continue }; let kept: Vec = list.into_iter().filter(|o| !o.as_ref().is_some_and(|r| overlays.contains(&r))).collect(); shown.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(kept))).map_err(|e| EditError::Write(e.to_string()))?; } - let bytes = write_incremental(&shown, &SaveOptions::default()).map_err(|e| EditError::Write(e.to_string()))?; + let bytes = Arc::new(write_incremental(&shown, &SaveOptions::default()).map_err(|e| EditError::Write(e.to_string()))?); + // Page geometry comes from the document as it is now written: a page moved earlier in the batch + // brings its own size and rotation with it. + let pages = pdfcraft_render::inspect(bytes.clone(), password).map_err(|e| EditError::Write(e.to_string()))?.pages; + // The model counts one entry per `/Kids` entry; the renderer one per page it draws. If they + // disagree, a mark's index could land on another page's text, and the check cannot confirm + // the redaction. + if pages.len() != model.len() { + return Err(RedactError::AmbiguousPages.into()); + } let config = RenderConfig { password: password.map(Arc::from), ..Default::default() }; - let mut renderer = pdfcraft_render::PageRenderer::new(Arc::new(bytes), config); + let mut renderer = pdfcraft_render::PageRenderer::new(bytes, config); let mut residue = 0; for &pi in &marked { let request = pdfcraft_render::RenderRequest { page: pi, kind: pdfcraft_render::RequestKind::Text, scale: 1.0, ..Default::default() }; // A page that cannot be read back cannot be confirmed clean. let (Some(info), Some(text)) = (pages.get(pi), renderer.render(request).text) else { return Err(RedactError::Unreadable(pi + 1).into()) }; - let rects: Vec<[f64; 4]> = marks - .iter() - .filter(|m| m.page == pi) - .flat_map(|m| m.rects.iter()) - .map(|r| [r[0].min(r[2]), r[1].min(r[3]), r[0].max(r[2]), r[1].max(r[3])]) - .collect(); + let rects = rects_of.get(&pi).cloned().unwrap_or_default(); for glyph in text.glyphs.iter().filter(|g| !g.text.trim().is_empty()) { let [x0, y0, x1, y1] = info.view_rect_to_user(glyph.rect).map(f64::from); let area = (x1 - x0) * (y1 - y0); diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 57e5729fc8..0a7a348fca 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -253,6 +253,11 @@ pub(crate) struct EditCtx { /// The overlay streams the edit's applied redactions added, which alone the renderer's check /// leaves out. pub(crate) redaction_overlays: Vec, + /// The marks the edit's applied redactions took out (a batch's included), as they were when + /// each apply ran, for the renderer's check. + /// Each applied mark with the page object it was drawn on, which a later page move changes the + /// index of but not the identity. + pub(crate) redaction_marks: Vec<(pdfcraft_cos::ObjRef, pdfcraft_redact::Mark)>, /// What the edit's applied redactions left as it was that still holds the removed text. pub(crate) redaction_elsewhere: Option, /// What became of the PDF/A and PDF/UA identification when the edit's applied redactions @@ -310,6 +315,7 @@ impl EditCtx { rng: getrandom::fill, renamed_fields: Vec::new(), redaction_overlays: Vec::new(), + redaction_marks: Vec::new(), redaction_elsewhere: None, redaction_identification: pdfcraft_redact::Identification::default(), prepared: None, @@ -529,7 +535,17 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E pdfcraft_organize::insert_blank_page(doc, *at, *width, *height)?; } Edit::SetInfo { key, value } => pdfcraft_organize::set_info(doc, key, value)?, - Edit::DuplicatePages { pages, at } => pdfcraft_organize::duplicate_pages(doc, pages, *at)?, + Edit::DuplicatePages { pages, at } => { + // A copy of a page cannot be tied back to the marks an earlier redaction of this edit + // applied: text put back under a mark would sit on the copy, unchecked. Fail closed. + if !cx.redaction_marks.is_empty() { + return Err(EditError::Invalid( + "a batch that applies redactions can't also copy pages: the copies can't be checked against the marks; the document was not changed" + .into(), + )); + } + pdfcraft_organize::duplicate_pages(doc, pages, *at)? + } Edit::ReplacePages { pages, name, bytes, src_pages } => { let src = crate::sources::open_source_into(name, bytes, doc)?; pdfcraft_organize::replace_pages(doc, pages, &src, src_pages)?; @@ -991,8 +1007,15 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E pdfcraft_edit::add_content(doc, *page, &content)?; } Edit::ApplyRedactions { pages } => { + let model = pdfcraft_model::pages(doc); + cx.redaction_marks.extend( + pdfcraft_redact::marks(doc) + .into_iter() + .filter(|m| pages.as_ref().is_none_or(|p| p.contains(&m.page))) + .filter_map(|m| model.get(m.page).map(|p| (p.obj, m))), + ); let report = pdfcraft_redact::apply(doc, pages.as_deref())?; - cx.redaction_overlays = report.overlays; + cx.redaction_overlays.extend(report.overlays); cx.redaction_elsewhere = Some(report.still_holding); cx.redaction_identification = report.identification; } diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index 4a6f372fb4..a039f56798 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -185,6 +185,7 @@ impl Session { } cx.renamed_fields.clear(); cx.redaction_overlays.clear(); + cx.redaction_marks.clear(); // `next` is a copy: if the edit fails or crashes, the document is unchanged. run_checked_edit(&mut next, &edit, &mut cx)?; // A field's enter or exit scripts that changed nothing leave no edit behind (no undo step, @@ -195,11 +196,20 @@ impl Session { return Ok(None); } // Redaction removed what its own interpreter places under the marks. The marks were drawn - // over the page as the renderer shows it, so the renderer has the last word. - if let Edit::ApplyRedactions { pages } = &edit { - let marks: Vec = - pdfcraft_redact::marks(&editor.cos).into_iter().filter(|m| pages.as_ref().is_none_or(|p| p.contains(&m.page))).collect(); - guard(|| shown_under_redactions(&next, &marks, &cx.redaction_overlays, &doc.info.pages, doc.password.as_deref())) + // over the page as the renderer shows it, so the renderer has the last word: on every + // apply the edit ran, a batch's included. + if !cx.redaction_marks.is_empty() { + // A batch that deletes a marked page and adds pages cannot be tied back to the marks: + // a copy of a page may carry the text the redaction removed from the original only. + // Fail closed. + let before: std::collections::HashSet<_> = pdfcraft_model::pages(&editor.cos).iter().map(|p| p.obj).collect(); + let after: std::collections::HashSet<_> = pdfcraft_model::pages(&next).iter().map(|p| p.obj).collect(); + if cx.redaction_marks.iter().any(|(o, _)| !after.contains(o)) && after.iter().any(|o| !before.contains(o)) { + return Err(EditError::Invalid( + "a batch that deletes a page with a redaction mark and adds pages can't be verified; the document was not changed".into(), + )); + } + guard(|| shown_under_redactions(&next, &cx.redaction_marks, &cx.redaction_overlays, doc.password.as_deref())) .unwrap_or_else(|m| Err(EditError::Invalid(format!("{} failed unexpectedly ({m}); the document was not changed", edit.label()))))?; } // Renamed fields are said, not slipped in: a repair-log note (with the caveat that diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 4e24a4f608..91b00df4cc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3542,9 +3542,10 @@ fn the_renderer_check_refuses_text_still_shown_under_a_mark() { mark_shown_text(&mut s, id, "SECRET42"); let doc = s.get(id).unwrap(); let mut cos = doc.editor.as_ref().unwrap().cos.clone(); - let marks = pdfcraft_redact::marks(&cos); + let model = pdfcraft_model::pages(&cos); + let marks: Vec<_> = pdfcraft_redact::marks(&cos).into_iter().filter_map(|m| model.get(m.page).map(|p| (p.obj, m))).collect(); let refused = |cos: &pdfcraft_cos::Document| { - let shown = shown_under_redactions(cos, &marks, &[], &doc.info.pages, None); + let shown = shown_under_redactions(cos, &marks, &[], None); matches!(shown, Err(EditError::Redact(pdfcraft_redact::RedactError::Residue(n))) if n > 0) }; assert!(refused(&cos)); @@ -13778,6 +13779,66 @@ fn image_transform_engine_refuses_singular_matrices_without_mutation() { } } +/// The renderer's check covers a redaction applied inside a batch too, with the marks as they +/// were when it ran: one the batch itself added is checked, and text a later edit of the batch +/// puts back under a mark refuses the whole batch. +#[test] +fn redactions_applied_in_a_batch_get_the_renderer_check() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page_with("/Font << /F1 4 0 R >>", "", "BT /F1 16 Tf 20 150 Td (Public words then SECRET42) Tj ET"); + let id = s.open("batch.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let apply = Edit::ApplyRedactions { pages: None }; + // Text added where the mark was shows under it again: the renderer sees it and refuses. + let under = AddedText { rect: [150.0, 130.0, 300.0, 160.0], text: "SECRET42 SECRET42".into(), ..Default::default() }; + let refused = s.apply(id, Edit::Batch { label: "Redact then write".into(), edits: vec![apply.clone(), Edit::AddText { page: 0, text: under }] }); + assert!(matches!(refused, Err(EditError::Redact(pdfcraft_redact::RedactError::Residue(n))) if n > 0), "{refused:?}"); + assert!(page_texts(&s, id)[0].contains("SECRET42"), "nothing changed"); + assert_eq!(s.get(id).unwrap().redaction_marks(), 1); + // Applied alone in a batch, the redaction goes through, and the text is gone. + s.apply(id, Edit::Batch { label: "Redact".into(), edits: vec![apply] }).unwrap(); + assert!(!page_texts(&s, id)[0].contains("SECRET42"), "{:?}", page_texts(&s, id)); + assert_eq!(s.get(id).unwrap().redaction_marks(), 0); +} + +/// A batch that deletes a page holding a redaction mark and adds pages is refused whole: the +/// marks cannot be tied to copies, so redaction fails closed. Without added pages it goes through. +#[test] +fn a_batch_that_deletes_a_marked_page_and_adds_pages_is_refused() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page_with("/Font << /F1 4 0 R >>", "", "BT /F1 16 Tf 20 150 Td (Public words then SECRET42) Tj ET"); + let id = s.open("batch.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let edits = vec![ + Edit::ApplyRedactions { pages: None }, + Edit::InsertBlankPage { at: 1, width: 612.0, height: 792.0 }, + Edit::DeletePages { pages: vec![0] }, + ]; + let refused = s.apply(id, Edit::Batch { label: "x".into(), edits }); + assert!(matches!(refused, Err(EditError::Invalid(_))), "{refused:?}"); + assert!(page_texts(&s, id)[0].contains("SECRET42"), "nothing changed"); + assert_eq!(s.get(id).unwrap().redaction_marks(), 1); +} + +/// A batch that applies redactions and then copies a page is refused whole: the copy can't be tied +/// to the marks, so text restored under one would be unchecked. Copying first, or alone, is fine. +#[test] +fn a_batch_that_applies_redactions_and_copies_a_page_is_refused() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page_with("/Font << /F1 4 0 R >>", "", "BT /F1 16 Tf 20 150 Td (Public words then SECRET42) Tj ET"); + let id = s.open("batch.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let apply = Edit::ApplyRedactions { pages: None }; + let copy = Edit::DuplicatePages { pages: vec![0], at: None }; + let refused = s.apply(id, Edit::Batch { label: "x".into(), edits: vec![apply.clone(), copy.clone()] }); + assert!(matches!(&refused, Err(EditError::Invalid(m)) if m.contains("copy pages")), "{refused:?}"); + assert!(page_texts(&s, id)[0].contains("SECRET42"), "nothing changed"); + assert_eq!(s.get(id).unwrap().redaction_marks(), 1); + // Copying before the redaction is applied puts the mark on the copy too, and both are checked. + s.apply(id, Edit::Batch { label: "y".into(), edits: vec![copy, apply] }).unwrap(); + assert!(page_texts(&s, id).iter().all(|t| !t.contains("SECRET42")), "{:?}", page_texts(&s, id)); +} + /// A reset in an opening script reaches a field of a row an earlier opening script added, before /// the form is laid out again with the row's widgets: its data goes back to its default and stays /// so when the form is saved. diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 37bbc52819..7ae75ddf31 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -821,6 +821,20 @@ fn number(t: &Tok) -> Option { } } +/// The font's `/BaseFont` (a Type 3 font's descriptor `/FontName` when it has none), read without +/// building its metrics, so it decodes no CMap. +pub fn base_font_name(doc: &Document, font: &Dict) -> String { + font.name(b"BaseFont") + .map(|name| String::from_utf8_lossy(name).into_owned()) + .or_else(|| { + (font.name(b"Subtype") == Some(b"Type3")) + .then(|| dict(doc, font.get(b"FontDescriptor"))) + .flatten() + .and_then(|d| d.name(b"FontName").map(|name| String::from_utf8_lossy(name).into_owned())) + }) + .unwrap_or_default() +} + /// Codespace ranges and CID mappings from an embedded CMap stream, at most /// [`MAX_CMAP_RANGES`] of each (every glyph searches them). #[allow(clippy::type_complexity)] @@ -909,16 +923,7 @@ impl Metrics { pub fn from_dict_within(doc: &Document, font: &Dict, budget: &mut DecodeBudget) -> Self { let mut reads = Reads::new(doc, budget); let mut m = Self::read_metrics(doc, font, &mut reads); - let base = font - .name(b"BaseFont") - .map(|name| String::from_utf8_lossy(name).into_owned()) - .or_else(|| { - (font.name(b"Subtype") == Some(b"Type3")) - .then(|| dict(doc, font.get(b"FontDescriptor"))) - .flatten() - .and_then(|d| d.name(b"FontName").map(|name| String::from_utf8_lossy(name).into_owned())) - }) - .unwrap_or_default(); + let base = base_font_name(doc, font); m.subset = base.len() > 7 && base.as_bytes()[6] == b'+' && base[..6].bytes().all(|b| b.is_ascii_uppercase()); let (bold, italic) = style_from_name(&base); m.bold |= bold; diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 4c5f90044f..74880c94a4 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -1823,6 +1823,16 @@ fn low_codes_resolve_to_the_covering_range() { assert_eq!((u.covering(0xFFFF).is_some(), u.covering(0x1_0001).is_some(), u.covering(0x1_0002).is_none()), (true, true, true)); } +#[test] +fn base_font_name_matches_the_metrics_name() { + let doc = Document::new_empty(); + let mut f = Dict::new(); + f.set(b"BaseFont".to_vec(), Object::name("ABCDEF+Body")); + assert_eq!(base_font_name(&doc, &f), "ABCDEF+Body"); + assert_eq!(base_font_name(&doc, &f), Metrics::from_dict(&doc, &f).base_font); + assert_eq!(base_font_name(&doc, &Dict::new()), ""); +} + #[test] fn refused_subset_reads_do_not_encode_unverified_glyphs() { let mut doc = Document::new_empty(); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 75dd075889..2ce02626c9 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -5601,6 +5601,75 @@ fn a_page_widget_inheriting_password_flag_beyond_depth_32_is_recognized() { assert!(!has_password_values(&doc)); } +/// A field reachable both near the top of the tree and at the end of a long branch: the field +/// walk reaches it near the top and reads the password field below it, so the quick check must +/// not stop at the long branch's depth limit and conclude there is none. The password field's +/// widget is on the page but names no `/Parent`, so the page widgets alone don't find it either. +#[test] +fn a_password_field_past_the_depth_limit_on_one_branch_is_still_found() { + let chain = 64; + let x = 6 + chain; // the shared field + let y = x + 1; // the password field + let w = y + 1; // its widget + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + format!("<< /Type /Page /Parent 2 0 R /Annots [{w} 0 R] >>"), + "<< /Fields [5 0 R 6 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), + format!("<< /T (short) /Kids [{x} 0 R] >>"), + ]; + for d in 0..chain { + let next = if d + 1 == chain { x } else { 7 + d }; + objs.push(format!("<< /T (n{d}) /Kids [{next} 0 R] >>")); + } + objs.push(format!("<< /T (shared) /Kids [{y} 0 R] >>")); + objs.push(format!("<< /FT /Tx /T (pin) /Ff 8192 /V (deepsecret) /Kids [{w} 0 R] >>")); + objs.push("<< /Type /Annot /Subtype /Widget /Rect [50 600 150 620] /P 3 0 R >>".into()); + let mut doc = document(&objs); + assert!(fields(&doc).iter().any(|f| f.has(flags::PASSWORD)), "the field walk reads the password field"); + assert!(has_password_flag(&doc)); + assert!(has_password_values(&doc)); + assert_eq!(clear_password_values(&mut doc).unwrap(), 1); + assert!(!has_password_values(&doc)); +} + +/// A password field below an unlisted field whose other widget is on the page: the field walk +/// adopts the whole unlisted field from that widget, so the quick check looks below it too. +#[test] +fn a_password_field_below_an_adopted_page_field_is_found() { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R] >>".into(), + "<< /Fields [] /DA (/Helv 0 Tf 0 g) >>".into(), + "<< /T (grp) /Kids [6 0 R 7 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (name) /Parent 5 0 R /Rect [50 700 150 720] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (pin) /Ff 8192 /V (secret8) /Parent 5 0 R /Rect [0 0 0 0] >>".into(), + ]); + assert!(fields(&doc).iter().any(|f| f.has(flags::PASSWORD)), "the field walk adopts the password field"); + assert!(has_password_flag(&doc)); + assert!(has_password_values(&doc)); + assert_eq!(clear_password_values(&mut doc).unwrap(), 1); + assert!(!has_password_values(&doc)); +} + +/// A nameless widget listed in `/Fields` and on no page, whose `/Parent` is an unlisted password +/// field: the field walk adopts the parent, so the quick check follows the entry's `/Parent` too. +#[test] +fn a_password_parent_of_a_nameless_fields_entry_is_found() { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R >>".into(), + "<< /Fields [6 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), + "<< /T (pin) /FT /Tx /Ff 8192 /V (secret8) /Kids [6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /Rect [0 0 0 0] >>".into(), + ]); + assert!(fields(&doc).iter().any(|f| f.has(flags::PASSWORD) && f.value.iter().any(|v| v == "secret8")), "the field walk reads the parent"); + assert!(has_password_flag(&doc)); + assert_eq!(clear_password_values(&mut doc).unwrap(), 1); +} + /// #4018: a value that doesn't fit its box at the field's font size is reported (it is drawn cut /// at the box). An auto-sized field shrinks to fit, and a short or empty value fits. #[test] @@ -6802,6 +6871,64 @@ fn locked_button_export_edits_leave_the_document_unchanged() { } } +/// A widget reached from `/Fields` as an unnamed kid of a plain field, whose own `/Parent` is an +/// unlisted password field: the quick check must still read that `/Parent` chain. +#[test] +fn a_page_widget_seen_under_fields_still_has_its_parent_chain_checked() { + let doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R] >>".into(), + "<< /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), + "<< /T (grp) /Kids [6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 7 0 R /Rect [50 700 150 720] /P 3 0 R >>".into(), + "<< /FT /Tx /T (pin) /Ff 8192 /V (secret9) /Kids [6 0 R] >>".into(), + ]); + assert!(has_password_flag(&doc)); +} + +/// A radio button that is read-only on its own (its own `/Ff`) can't be picked, and a selected one +/// can't be switched away from, though the group as a whole is writable. +#[test] +fn a_read_only_radio_button_is_neither_picked_nor_left() { + let lock = |doc: &mut Document, num: u32| doc.update_dict(pdfcraft_cos::ObjRef::new(num, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1))).unwrap(); + let mut doc = radio_group_form(); + lock(&mut doc, 7); // Medium + assert!(matches!(set_value(&mut doc, "size", &FieldValue::Radio(Some("M".into()))), Err(FormError::ReadOnly(_)))); + set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into()))).expect("an unlocked button is picked"); + // Small (selected) locked: the group stays on it. + let mut doc = radio_group_form(); + lock(&mut doc, 6); + assert!(matches!(set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into()))), Err(FormError::ReadOnly(_)))); + assert_eq!(field(&fields(&doc), "size").value, ["S"]); +} + +/// The group's `/V` names its selection even when the button's `/AS` says nothing: a read-only +/// button that `/V` selects is not switched away from either. +#[test] +fn a_read_only_radio_button_selected_by_the_group_value_is_not_left() { + let mut doc = radio_group_form(); + doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| { + d.set(b"Ff".to_vec(), Object::Int(1)); + d.set(b"AS".to_vec(), Object::name("Off")); + }) + .unwrap(); + assert!(matches!(set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into()))), Err(FormError::ReadOnly(_)))); + assert_eq!(field(&fields(&doc), "size").value, ["S"]); +} + +/// Clearing the form skips a group held by a read-only button and still clears the rest. +#[test] +fn clearing_the_form_goes_on_past_a_group_held_by_a_read_only_button() { + let mut doc = radio_group_form(); + doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1))).unwrap(); + doc.update_dict(pdfcraft_cos::ObjRef::new(9, 0), |d| d.set(b"DV".to_vec(), pdfcraft_cos::PdfString::text("Bob"))).unwrap(); + assert_eq!(reset(&mut doc, None).unwrap(), 1); + let all = fields(&doc); + assert_eq!(field(&all, "size").value, ["S"]); + assert_eq!(field(&all, "name").value, ["Bob"]); +} + #[test] /// Cosmetic field updates skip calculations, while a value write still triggers the chain. fn script_cosmetic_mutations_do_not_run_the_calculation_chain() { @@ -6937,6 +7064,26 @@ fn radio_buttons_without_as_sharing_the_value_show_one_on() { assert_eq!(on_widgets(&doc, "r").0, [false, true, false]); } +/// Two buttons share the on state `Yes`; the first is on by its `/AS` and the second is read-only +/// and off. `/V` is `Yes` for both, but only the first is selected: picking another option switches +/// the first off, which is allowed, and the read-only one never changes. +#[test] +fn a_read_only_button_sharing_the_on_state_of_the_selected_one_does_not_block() { + let mut doc = radio_without_as(49152, "/Ff 1 /AS /Off"); + doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"AS".to_vec(), Object::name("Yes"))).unwrap(); + set_value(&mut doc, "r", &FieldValue::Radio(Some("No".into()))).expect("the writable selected button may be switched off"); + assert_eq!(on_widgets(&doc, "r").0, [false, false, true]); +} + +/// Selecting another button through `FieldValue::Widget` can't switch a read-only selected one off. +#[test] +fn a_read_only_selected_radio_button_is_not_left_by_picking_a_widget() { + let mut doc = radio_group_form(); + doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1))).unwrap(); + assert!(matches!(set_value(&mut doc, "size", &FieldValue::Widget(2)), Err(FormError::ReadOnly(_)))); + assert_eq!(field(&fields(&doc), "size").value, ["S"]); +} + /// Table 227, ReadOnly: a processor "shall not allow a user to change the value". A button whose own /// dictionary sets it can't be turned on by number. #[test] diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index af2c5e1aca..3600695c03 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -356,6 +356,9 @@ fn write_value_impl(doc: &mut Document, f: &Field, value: &FieldValue, scripts: let Some(state) = f.widgets.get(*i).and_then(|w| w.on_state.clone()) else { return invalid(format!("{:?} has no button {} with an on state", f.name, i.saturating_add(1))); }; + if f.kind == FieldKind::Radio && radio_change_blocked(f, Some(&state), Some(*i)) { + return Err(FormError::ReadOnly(f.name.clone())); + } set_states(doc, f, Some(&state), Some(*i)) } (FieldKind::CheckBox, v) => { @@ -406,6 +409,9 @@ fn write_value_impl(doc: &mut Document, f: &Field, value: &FieldValue, scripts: if choice.is_none() && f.has(flags::NO_TOGGLE_TO_OFF) && !f.value.is_empty() { return invalid(format!("{:?} must keep one option selected", f.name)); } + if radio_change_blocked(f, choice.as_deref(), None) { + return Err(FormError::ReadOnly(f.name.clone())); + } set_states(doc, f, choice.as_deref(), None) } (FieldKind::Combo | FieldKind::List, FieldValue::Choice(_) | FieldValue::Text(_) | FieldValue::ChoiceIndices(_)) => { @@ -541,17 +547,49 @@ pub(crate) fn scroll_widgets(doc: &mut Document, f: &Field, picked: &[usize], co Ok(()) } +/// The one widget of an exclusive radio group that turns on when `on` is chosen (`pick` when it has +/// that state, else the one already on, else the first); `None` when `on` is `None` or the group +/// turns every widget with that state on together. +fn chosen_widget(f: &Field, on: Option<&str>, pick: Option) -> Option { + let exclusive = f.kind == FieldKind::Radio && !f.has(flags::RADIOS_IN_UNISON); + on.filter(|_| exclusive).and_then(|c| { + let has = |i: &usize| f.widgets.get(*i).is_some_and(|w| w.on_state.as_deref() == Some(c)); + let n = f.widgets.len(); + pick.filter(has).or_else(|| (0..n).filter(has).find(|&i| f.widget_on(i))).or_else(|| (0..n).find(has)) + }) +} + +/// Whether setting a radio group to `on` (through widget `pick`, when the caller chose a widget) +/// would turn a read-only button on or off: such a button can't be picked, and a selected one can't +/// be switched off by picking another. Judged per widget, by the state `set_states` would give it, +/// so buttons sharing an on-state name are told apart by their `/AS`; the group's `/V` stands for +/// the selection only when no button's `/AS` says which is on. +fn radio_change_blocked(f: &Field, on: Option<&str>, pick: Option) -> bool { + let chosen = chosen_widget(f, on, pick); + let before = |i: usize, w: &Widget| { + let Some(s) = w.on_state.as_deref() else { return false }; + if f.widget_on(i) { + return true; + } + // No button sharing this on state is on by its `/AS` and the group's `/V` names it: the + // first of them holds the selection, whatever its own `/AS` says (an `Off` among them). + let sharing = |x: &Widget| x.on_state.as_deref() == Some(s); + f.value.first().map(String::as_str) == Some(s) + && !f.widgets.iter().any(|x| sharing(x) && x.state.as_deref() == Some(s)) + && f.widgets.iter().position(sharing) == Some(i) + }; + f.widgets.iter().enumerate().any(|(i, w)| { + let after = on.is_some() && w.on_state.as_deref() == on && chosen.is_none_or(|k| k == i); + w.read_only && before(i, w) != after + }) +} + /// Check boxes and radio buttons: `/V` on the field, `/AS` on each widget. The widgets whose on /// state is `on` turn on together in a check box (ISO 32000-2 §12.7.5.2.3, EXAMPLE 2) and in a /// radio group with RadiosInUnison. Without it, at most one radio button is on (Table 229, /// §12.7.5.2.3): widget `pick` when it has that state, else the one already on, else the first. fn set_states(doc: &mut Document, f: &Field, on: Option<&str>, pick: Option) -> Result<(), FormError> { - let exclusive = f.kind == FieldKind::Radio && !f.has(flags::RADIOS_IN_UNISON); - let chosen = on.filter(|_| exclusive).and_then(|c| { - let has = |i: &usize| f.widgets.get(*i).is_some_and(|w| w.on_state.as_deref() == Some(c)); - let n = f.widgets.len(); - pick.filter(has).or_else(|| (0..n).filter(has).find(|&i| f.widget_on(i))).or_else(|| (0..n).find(has)) - }); + let chosen = chosen_widget(f, on, pick); // `/V` names the on state, the same for every widget that has it: which of them is on is // their `/AS` alone (§12.7.5.2.4). let v = on.unwrap_or("Off"); @@ -720,29 +758,69 @@ fn password_fields(doc: &Document) -> Vec { /// again only makes the answer cautious. It reads only `/Ff`, `/Kids` and `/Parent`, so a save /// or recovery snapshot of a form without such a field doesn't build every [`Field`]. Like /// [`fields`], it looks at the form's field tree and at the widgets on the pages, which may be -/// missing from `/Fields`. Each dictionary is visited once and at most 64 levels deep. +/// missing from `/Fields`, together with the whole tree above and below them that [`fields`] +/// adopts. Each dictionary is visited once. +/// +/// It must never answer `false` for a form whose [`fields`] include a password field, or a typed +/// password would be saved. So where it can't look as far as [`fields`] may (a branch deeper than +/// the field walk's limit, which depends on the order the tree is visited in, or a `/Parent` +/// chain too long to follow), it answers `true` and the caller reads every field. pub(crate) fn has_password_flag(doc: &Document) -> bool { + const MAX_FIELD_DEPTH: usize = 64; let sets_it = |r: ObjRef| doc.get(r).as_dict().and_then(|d| field_flags(doc, d)).is_some_and(|ff| ff & flags::PASSWORD != 0); - let top = acroform(doc).and_then(|af| af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned())).unwrap_or_default(); - let mut stack: Vec<(Object, usize)> = top.into_iter().map(|o| (o, 0)).collect(); + let kids_of = + |r: ObjRef| doc.get(r).as_dict().and_then(|d| d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned())).unwrap_or_default(); let mut seen = std::collections::HashSet::new(); - while let Some((o, depth)) = stack.pop() { - let Object::Ref(r) = o else { continue }; - if depth > 64 || !seen.insert(r) { - continue; + // Whether a dictionary below `start` sets the flag, or the walk was cut short. + let subtree = |start: Vec, seen: &mut std::collections::HashSet| { + let mut stack: Vec<(Object, usize)> = start.into_iter().map(|o| (o, 0)).collect(); + while let Some((o, depth)) = stack.pop() { + let Object::Ref(r) = o else { continue }; + if !seen.insert(r) { + continue; + } + if depth > MAX_FIELD_DEPTH || sets_it(r) { + return true; + } + stack.extend(kids_of(r).into_iter().map(|k| (k, depth + 1))); } - if sets_it(r) { + false + }; + let top = acroform(doc).and_then(|af| af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned())).unwrap_or_default(); + let top_refs: Vec = top.iter().filter_map(Object::as_ref).collect(); + if subtree(top, &mut seen) { + return true; + } + // A `/Fields` entry may itself name a field above it through `/Parent` (one `fields` adopts when + // the entry has no name of its own), whether or not a page lists it. + for r in top_refs { + let chain = field_chain(doc, r); + if chain.len() >= MAX_FIELD_DEPTH || chain.iter().copied().any(sets_it) { + return true; + } + if subtree(vec![Object::Ref(field_root(doc, &chain, r))], &mut seen) { return true; } - let kids = doc.get(r).as_dict().and_then(|d| d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned())).unwrap_or_default(); - stack.extend(kids.into_iter().map(|k| (k, depth + 1))); } - // Widgets on the pages, and the fields above them. - page_refs(doc).into_iter().any(|p| { + // Widgets on the pages, the fields above them, and the fields below the topmost one. + for p in page_refs(doc) { let annots = doc.get(p).as_dict().and_then(|d| d.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned())).unwrap_or_default(); - annots.iter().filter_map(Object::as_ref).filter(|r| !seen.contains(r)).any(|r| field_chain(doc, r).into_iter().any(sets_it)) - }) + // Every widget's `/Parent` chain is read, even one already reached from `/Fields`: the + // field walk can adopt it under a different parent there. Each chain is at most + // MAX_FIELD_DEPTH long, and `subtree` returns at once for a root it has seen. + for r in annots.iter().filter_map(Object::as_ref) { + let chain = field_chain(doc, r); + if chain.len() >= MAX_FIELD_DEPTH || chain.iter().copied().any(sets_it) { + return true; + } + let root = field_root(doc, &chain, r); + if subtree(vec![Object::Ref(root)], &mut seen) { + return true; + } + } + } + false } /// Whether any text field with the Password flag holds a value in `doc`. @@ -768,6 +846,12 @@ pub fn restore_password_values(doc: &mut Document, values: &[(String, String)]) Ok(()) } +/// The topmost dictionary of `chain` (from [`field_chain`]): a `/Parent` that is not a dictionary +/// ends the field tree, so the walk below it starts at the last one that is. +fn field_root(doc: &Document, chain: &[ObjRef], r: ObjRef) -> ObjRef { + chain.iter().rev().copied().find(|c| doc.get(*c).as_dict().is_some()).unwrap_or(r) +} + /// `r` and its field ancestors (`/Parent`), nearest first, at most 64 deep and without repeats. fn field_chain(doc: &Document, r: ObjRef) -> Vec { let mut chain = vec![r]; @@ -919,10 +1003,17 @@ pub fn reset_with_formatting( // A reset is not blocked by NoToggleToOff: go through the writer directly. let mut tmp = f.clone(); tmp.flags &= !flags::NO_TOGGLE_TO_OFF; - write_value_impl(doc, &tmp, &v, scripts, true)?; - changed += 1; - if matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List) { - redraw_fields.insert(f.obj); + match write_value_impl(doc, &tmp, &v, scripts, true) { + Ok(()) => { + changed += 1; + if matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List) { + redraw_fields.insert(f.obj); + } + } + // a read-only button of the group holds the value: this field is left as it is, and the + // rest of the form is still cleared + Err(FormError::ReadOnly(_)) => {} + Err(e) => return Err(e), } } let calculation_runner: &mut dyn Scripts = if calculate_with_scripts { scripts } else { &mut NoScripts }; diff --git a/crates/js/src/document.rs b/crates/js/src/document.rs index f13c9434e9..cccf89a7a2 100644 --- a/crates/js/src/document.rs +++ b/crates/js/src/document.rs @@ -1,4 +1,6 @@ -use super::host::{FieldKey, arg, colour, error, function, host, read, s, set_value, strings, text, text_line, value_of, write}; +use super::host::{ + FieldKey, arg, charge, colour, error, function, host, kind_of, read, s, strings, text, text_bytes, text_line, value_for, value_of, write, +}; use super::model::{DISPLAY_HIDDEN, DISPLAY_VISIBLE, FieldType, Request}; use super::{Attribute, Context, JsArray, JsObject, JsResult, JsString, JsValue, NativeFunction, ObjectInitializer, js_string}; @@ -41,8 +43,9 @@ fn field_object_new(ctx: &mut Context, name: &str, id: Option, token: u64) let value_get = getter!(ctx, n, |f, c| value_of(&f, c)); let value_set = { fn set(_: &JsValue, args: &[JsValue], name: &FieldKey, c: &mut Context) -> JsResult { - let vals = strings(&arg(args, 0), c)?; - write(c, name, |f| set_value(f, vals))?; + let vals = value_for(kind_of(c, name)?, strings(&arg(args, 0), c)?); + charge(c, name, |f| text_bytes(&f.value), text_bytes(&vals))?; + write(c, name, |f| f.value = vals)?; Ok(JsValue::undefined()) } function(ctx, NativeFunction::from_copy_closure_with_captures(set, n.clone())) @@ -104,6 +107,7 @@ fn field_object_new(ctx: &mut Context, name: &str, id: Option, token: u64) let text_color_set = { fn set(_: &JsValue, args: &[JsValue], name: &FieldKey, c: &mut Context) -> JsResult { let v = strings(&arg(args, 0), c)?; + charge(c, name, |f| f.text_color.as_deref().map_or(0, text_bytes), text_bytes(&v))?; write(c, name, |f| f.text_color = Some(v.into()))?; Ok(JsValue::undefined()) } @@ -113,6 +117,7 @@ fn field_object_new(ctx: &mut Context, name: &str, id: Option, token: u64) let fill_color_set = { fn set(_: &JsValue, args: &[JsValue], name: &FieldKey, c: &mut Context) -> JsResult { let v = strings(&arg(args, 0), c)?; + charge(c, name, |f| f.fill_color.as_deref().map_or(0, text_bytes), text_bytes(&v))?; write(c, name, |f| f.fill_color = Some(v.into()))?; Ok(JsValue::undefined()) } diff --git a/crates/js/src/formcalc/access.rs b/crates/js/src/formcalc/access.rs index 26de7ba9eb..3c323ca088 100644 --- a/crates/js/src/formcalc/access.rs +++ b/crates/js/src/formcalc/access.rs @@ -33,9 +33,12 @@ impl Interp<'_> { Ok(()) } - /// `more` nodes into `out`, each once, never past [`MAX_LIST`]. + /// `more` nodes into `out`, each once, never past [`MAX_LIST`]. Every node visited costs a + /// step, kept or not, and the call itself one more: a search that matches nothing still + /// costs work when it is repeated over a long list. pub(super) fn gather(&mut self, out: &mut Vec, seen: &mut HashSet, more: impl IntoIterator) -> R<()> { for i in more { + self.step()?; if seen.insert(i) { out.push(i); if out.len() > MAX_LIST { @@ -43,7 +46,7 @@ impl Interp<'_> { } } } - self.charge(1) + self.step() } /// The nodes of `v`, each once. @@ -294,8 +297,10 @@ impl Interp<'_> { } } - /// `v` onto `out`: a node only once, never past [`MAX_LIST`]. + /// `v` onto `out`: a node only once, never past [`MAX_LIST`]. A duplicate costs its step + /// too: `all` on a list of same-named siblings makes one for every pair. fn list_push(&mut self, out: &mut Vec, seen: &mut HashSet, v: Val) -> R<()> { + self.step()?; if let Val::Node(i) = v && !seen.insert(i) { @@ -305,7 +310,7 @@ impl Interp<'_> { if out.len() > MAX_LIST { return Err(LIST_TOO_LONG.into()); } - self.charge(1) + Ok(()) } /// Property `name` of form object `i` as FormCalc reads it: its value, attributes, diff --git a/crates/js/src/formcalc/dates.rs b/crates/js/src/formcalc/dates.rs index 4d9befeab2..f201a23525 100644 --- a/crates/js/src/formcalc/dates.rs +++ b/crates/js/src/formcalc/dates.rs @@ -56,6 +56,9 @@ const CENTURY_SPLIT: i64 = 30; const MONTHS: [&str; 12] = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"]; +/// Characters of the longest month name: all a month token needs to read of the text. +const LONGEST_MONTH: usize = 9; + /// Parse `text` as a date by `picture` (`YYYY-MM-DD`, `MM/DD/YYYY`, `MMM D, YYYY`, …). pub(super) fn parse_date(text: &str, picture: &str) -> Option<(i64, u32, u32)> { let pic: Vec = picture.chars().collect(); @@ -71,8 +74,9 @@ pub(super) fn parse_date(text: &str, picture: &str) -> Option<(i64, u32, u32)> { } i += run; if c == 'M' && run >= 3 { - // Month name. - let rest: String = txt.get(j..).unwrap_or(&[]).iter().collect(); + // Month name: only as much of the text as a name can be, so a picture of many + // month tokens does not copy the rest of a long text for each of them. + let rest: String = txt.get(j..).unwrap_or(&[]).iter().take(LONGEST_MONTH).collect(); let lower = rest.to_ascii_lowercase(); let found = MONTHS .iter() diff --git a/crates/js/src/host.rs b/crates/js/src/host.rs index 5966e63545..d829041ce6 100644 --- a/crates/js/src/host.rs +++ b/crates/js/src/host.rs @@ -27,6 +27,10 @@ pub(crate) struct Host { pub(crate) reset_limit_exceeded: bool, /// Bytes of text kept in alerts, console lines and requests, against [`xfa::MAX_PAYLOAD`]. pub(crate) kept: usize, + /// Bytes of text the script has added to the fields' values and colours, net of what it + /// replaced, against [`xfa::MAX_PAYLOAD`]: each assignment alone is capped, so without this + /// many fields could each hold a maximal array. + pub(crate) retained: usize, /// Alerts, console lines and requests the limits left out. pub(crate) dropped: usize, } @@ -262,12 +266,42 @@ pub(crate) fn value_of(f: &FieldState, ctx: &mut Context) -> JsValue { } } -pub(crate) fn set_value(f: &mut FieldState, vals: Vec) { - f.value = match f.kind { +/// Bytes of text in `v`, counting each string's heap size and its handle. +pub(crate) fn text_bytes(v: &[String]) -> usize { + v.iter().fold(0usize, |n, t| n.saturating_add(t.len()).saturating_add(std::mem::size_of::())) +} + +/// Charge the replacement of the text `old` selects from field `name` with `new_bytes` of text to +/// the run's retained-text budget; an assignment the budget has no room for is an error and +/// charges nothing. +pub(crate) fn charge(ctx: &Context, name: &FieldKey, old: impl FnOnce(&FieldState) -> usize, new_bytes: usize) -> JsResult<()> { + let i = field_index(ctx, name)?; + let mut h = host(ctx)?.borrow_mut(); + let freed = old(&h.fields[i]); + let total = h.retained.saturating_sub(freed).saturating_add(new_bytes); + if total > xfa::MAX_PAYLOAD { + return Err(error(format!("the fields' values and colours hold more than {} bytes of text", xfa::MAX_PAYLOAD))); + } + h.retained = total; + Ok(()) +} + +pub(crate) fn kind_of(ctx: &Context, name: &FieldKey) -> JsResult { + let i = field_index(ctx, name)?; + Ok(host(ctx)?.borrow().fields[i].kind) +} + +/// What a field of `kind` keeps of the values assigned to it. +pub(crate) fn value_for(kind: FieldType, vals: Vec) -> Vec { + match kind { FieldType::CheckBox | FieldType::RadioButton => vals.into_iter().filter(|v| v != "Off" && !v.is_empty()).take(1).collect(), FieldType::ListBox => vals, _ => vals.into_iter().take(1).collect(), - }; + } +} + +pub(crate) fn set_value(f: &mut FieldState, vals: Vec) { + f.value = value_for(f.kind, vals); } #[cfg(test)] diff --git a/crates/js/src/run.rs b/crates/js/src/run.rs index ececa107a3..7019bd768c 100644 --- a/crates/js/src/run.rs +++ b/crates/js/src/run.rs @@ -270,6 +270,7 @@ pub(crate) fn run_in_context( reset_calls: setup.reset_calls, reset_limit_exceeded: false, kept: 0, + retained: 0, dropped: 0, })); let mut out = Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), ..Default::default() }; diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 1a41afeea8..ece4443b7d 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -87,6 +87,17 @@ fn calculate_scripts_read_fields_and_set_the_value() { assert!(o.rc); } +/// An array of field values copies each entry: one long string listed many times is held to +/// the run's byte budget, not copied once per listing. +#[test] +fn an_array_of_values_stops_at_the_byte_budget() { + let fill = "var s = 'x'; while (s.length < 65536) s += s; var a = []; for (var i = 0; i < N; i++) a.push(s); getField('qty').value = a;"; + let o = go(&fill.replace('N', "600"), &Event::field("Calculate", "total", "")); + assert!(o.error.as_deref().is_some_and(|e| e.contains("byte limit exceeded")), "{:?}", o.error); + let o = go(&fill.replace('N', "400"), &Event::field("Calculate", "total", "")); + assert_eq!(o.error, None); +} + /// `event.value` reads and writes as a plain value, numbers included, and the outcome says whether /// the script assigned it, even to the value it already had. #[test] @@ -528,6 +539,7 @@ fn app_alert_titles_are_clipped_and_charged() { reset_calls: 0, reset_limit_exceeded: false, kept: 0, + retained: 0, dropped: 0, }; h.alert(Message { title: Some("Order form".into()), ..Message::plain("Saved") }); @@ -1113,6 +1125,24 @@ mod xfa_model { assert_eq!(o.result.as_deref(), Some("7")); } + /// A copy of a subform costs its nodes, names and SOMs: instances of one with a long name + /// stop at the run's byte budget, well before the instance cap. + #[test] + fn instances_of_a_subform_with_long_names_stop_at_the_byte_budget() { + let mut f = form(); + let table = &mut f.children[0].children[4]; + assert_eq!(table.name, "table"); + for row in &mut table.children { + row.children.push(field(&"n".repeat(1 << 20), "", "", false)); + } + let ev = XfaEvent { activity: "click".into(), target: "form1[0].page1[0].go[0]".into(), ..Default::default() }; + let o = run_xfa("for (var i = 0; i < 100; i++) table._row.addInstance(1); table._row.count", &ev, &XfaDoc::default(), &f, Limits::default()); + assert_eq!(o.error, None, "{:?}", o.error); + let count: usize = o.result.as_deref().unwrap_or_default().parse().expect("a count"); + assert!((3..50).contains(&count), "{count} instances"); + assert_eq!(o.effects.iter().filter(|e| matches!(e, XfaEffect::AddInstance { .. })).count(), count - 2); + } + /// On screen every page is a sheet of its own: `xfa.layout.sheet` counts it from 0, as /// `absPage` does (forms show `sheet(this) + 1`), and `sheetCount` is the page count. #[test] @@ -1625,6 +1655,10 @@ mod formcalc { #[test] fn date2num_rejects_days_past_the_end_of_the_month() { + // A month name is read from the text's next few characters only, whatever follows. + let serial = calc(r#"Date2Num("2024-09-01")"#); + assert_eq!(calc(r#"Date2Num("September 1, 2024, and a note that runs on", "MMMM D, YYYY")"#), serial); + assert_eq!(calc(r#"Date2Num("Sep 1, 2024", "MMM D, YYYY")"#), serial); assert_eq!(calc(r#"Date2Num("2023-02-30")"#), "0"); assert_eq!(calc(r#"Date2Num("31/04/2024", "DD/MM/YYYY")"#), "0"); assert_eq!(calc(r#"Date2Num("2023-02-29")"#), "0"); @@ -2641,6 +2675,18 @@ fn a_plain_form_sees_the_xfa_version_but_not_installed() { assert_eq!(o.alerts, [Message::plain("old viewer")]); } +/// Arrays assigned to several fields are held to one run-wide budget, and replacing a field's +/// array frees its charge. +#[test] +fn retained_array_text_is_charged_across_fields() { + let fill = "var s = 'x'; while (s.length < 65536) s += s; var a = []; for (var i = 0; i < 320; i++) a.push(s);"; + let one = |tail: &str| go(&format!("{fill} {tail}"), &Event::field("Calculate", "total", "")); + let o = one("getField('qty').textColor = a; getField('qty').textColor = a; getField('zip').textColor = ['RGB'];"); + assert_eq!(o.error, None, "replacing an array frees its charge: {:?}", o.error); + let o = one("getField('qty').textColor = a; getField('zip').fillColor = a;"); + assert!(o.error.as_deref().is_some_and(|e| e.contains("hold more than")), "{:?}", o.error); +} + #[test] fn string_arrays_refuse_aggregate_text_before_request_clipping() { let o = go("resetForm(new Array(65).fill('x'.repeat(1 << 19)));", &Event::doc("Console")); diff --git a/crates/js/src/xfa/globals.rs b/crates/js/src/xfa/globals.rs index de57d7fd49..46dc8a713b 100644 --- a/crates/js/src/xfa/globals.rs +++ b/crates/js/src/xfa/globals.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use super::host::host; use super::instances::instance_manager; use super::model::{MAX_DEPTH, XfaDoc, XfaEffect, XfaEvent, clip, payload, reset_names}; @@ -379,7 +381,7 @@ pub(super) fn install(ctx: &mut Context, event: &XfaEvent, doc: &XfaDoc, current "DataView", "Intl", ]; - let mut defined: Vec = RESERVED.iter().map(ToString::to_string).collect(); + let mut defined: HashSet = RESERVED.iter().map(ToString::to_string).collect(); for scope in scopes { let kids: Vec<(usize, String, bool)> = { let h = host(ctx)?.borrow(); @@ -398,7 +400,7 @@ pub(super) fn install(ctx: &mut Context, event: &XfaEvent, doc: &XfaDoc, current let im = instance_manager(ctx, scope, &name)?; ctx.register_global_property(JsString::from(format!("_{name}").as_str()), im, rw)?; } - defined.push(name); + defined.insert(name); } } Ok(()) diff --git a/crates/js/src/xfa/instances.rs b/crates/js/src/xfa/instances.rs index 2985794dc3..c737ea9db9 100644 --- a/crates/js/src/xfa/instances.rs +++ b/crates/js/src/xfa/instances.rs @@ -137,6 +137,17 @@ fn child_som(parent_som: String, name: &str, index: usize) -> String { } } +/// Bytes a copy of `src`'s subtree keeps: a node, its name and its SOM (the copy's SOMs differ +/// in their indices only) for every node [`clone_subtree`] copies. +fn subtree_bytes(h: &XHost, src: usize, depth: usize) -> usize { + let Some(n) = h.nodes.get(src).filter(|_| depth <= MAX_DEPTH) else { return 0 }; + let own = std::mem::size_of::().saturating_add(n.name.len()).saturating_add(n.som.len()); + n.children + .iter() + .filter(|&&k| !h.nodes.get(k).is_some_and(|c| c.gone || c.repeatable && c.index > 0)) + .fold(own, |sum, &k| sum.saturating_add(subtree_bytes(h, k, depth + 1))) +} + /// A copy of `src`'s subtree (values cleared) under `parent`, returning the copy's index. fn clone_subtree(h: &mut XHost, src: usize, parent: Option, depth: usize) -> Option { if depth > MAX_DEPTH || h.nodes.len() >= MAX_NODES { @@ -214,7 +225,10 @@ impl XHost { let name = h.nodes.get(first).map(|n| n.name.to_string()).unwrap_or_default(); let parent_som = h.nodes.get(parent).map(|p| p.som.to_string()).unwrap_or_default(); let som = child_som(parent_som, &name, live.len()); - if !h.reserve(&XfaEffect::AddInstance { som: String::new() }, som.len()) { + // The copy's nodes, their names and SOMs, are charged to the run's byte budget before + // they are made: a subtree with long names must not be copied past it. + let copied = subtree_bytes(h, first, 0); + if !h.reserve(&XfaEffect::AddInstance { som: String::new() }, som.len().saturating_add(copied)) { return None; } let new = clone_subtree(h, first, Some(parent), 0)?; diff --git a/crates/optimize/src/audit.rs b/crates/optimize/src/audit.rs index 789bf2e89d..b06d76d6c7 100644 --- a/crates/optimize/src/audit.rs +++ b/crates/optimize/src/audit.rs @@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet}; -use pdfcraft_cos::{Dict, Document, MAX_DECODED, Object, XrefEntry}; +use pdfcraft_cos::{Dict, Document, MAX_DECODED, ObjRef, Object, XrefEntry}; /// The categories, in Acrobat's order. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -91,6 +91,9 @@ const MAX_FORM_DEPTH: usize = 64; struct Audit<'a> { doc: &'a Document, owner: HashMap, + /// Resource dictionaries (by reference) already walked: one shared by many pages, forms or + /// appearances is walked once, as claiming is the same each time. + walked: HashSet, } impl Audit<'_> { @@ -124,6 +127,18 @@ impl Audit<'_> { o.map(|x| self.doc.resolve(x)).and_then(|x| x.as_dict().cloned()).unwrap_or_default() } + /// The resource dictionary `holder` points at (if it does), by kind: [`Self::resources`], + /// once per dictionary given by reference. + fn resources_of(&mut self, holder: Option<&Object>, depth: usize) { + if let Some(Object::Ref(r)) = holder + && !self.walked.insert(*r) + { + return; + } + let res = self.resolve_dict(holder); + self.resources(&res, depth); + } + /// A resource dictionary, by kind; `depth` counts the form XObjects it is nested in. fn resources(&mut self, res: &Dict, depth: usize) { let xobjects = self.resolve_dict(res.get(b"XObject")); @@ -135,8 +150,7 @@ impl Audit<'_> { // A form: its own resources first, by kind, then the rest of it. Owning it // first stops a form that draws itself. self.owner.insert(r.num, SpaceCategory::XObjectForms); - let inner = self.resolve_dict(s.dict.get(b"Resources")); - self.resources(&inner, depth + 1); + self.resources_of(s.dict.get(b"Resources"), depth + 1); self.claim(&Object::Dict(s.dict.clone()), SpaceCategory::XObjectForms); } _ => self.claim(x, SpaceCategory::XObjectForms), @@ -172,8 +186,7 @@ impl Audit<'_> { _ => Vec::new(), }; for dict in streams { - let res = self.resolve_dict(dict.get(b"Resources")); - self.resources(&res, 1); + self.resources_of(dict.get(b"Resources"), 1); } } } @@ -216,19 +229,17 @@ fn object_stream(doc: &Document, num: u32) -> Option { /// Audit the space a document uses; `file_len` is the size of the file on disk (the rest is /// overhead). pub fn audit_space(doc: &Document, file_len: u64) -> Vec { - let mut a = Audit { doc, owner: HashMap::new() }; + let mut a = Audit { doc, owner: HashMap::new(), walked: HashSet::new() }; let cat = doc.root().and_then(|r| doc.get(r).as_dict().cloned()).unwrap_or_default(); let pages = pdfcraft_model::pages(doc); // Order matters: the most specific owners first. Resources by kind before anything that // uses them: a content stream's dictionary may point at what it draws (PdfCraft's /PCAdded // records an added image), and form fields and comments draw with fonts and images too. for p in &pages { - let res = a.resolve_dict(p.dict.get(b"Resources")); - a.resources(&res, 0); + a.resources_of(p.dict.get(b"Resources"), 0); } let form = a.resolve_dict(cat.get(b"AcroForm")); - let form_resources = a.resolve_dict(form.get(b"DR")); - a.resources(&form_resources, 0); + a.resources_of(form.get(b"DR"), 0); for p in &pages { for x in annotations(doc, &p.dict) { a.appearance_resources(&x); diff --git a/crates/optimize/src/fragments.rs b/crates/optimize/src/fragments.rs index aa52c20750..9ccca4791f 100644 --- a/crates/optimize/src/fragments.rs +++ b/crates/optimize/src/fragments.rs @@ -30,17 +30,21 @@ pub const MIN_FRAGMENTS: usize = 4; /// Most pixels a merged image may have (memory). const MAX_PIXELS: u64 = 80_000_000; -/// Fragment groups a page's search for runs may read per content operator, and at least. A run -/// that is not one picture is searched again from each of its groups, so without a limit images -/// drawn over one another take time quadratic in their number; past it, the rest of the page is -/// left as it is. -const SCAN_PER_OP: usize = 16; +/// Content operators a page's search for runs may read per content operator of the page, and at +/// least. A run that is not one picture is searched again from each of its groups, so without a +/// limit images drawn over one another take time quadratic in their number; past it, the rest of +/// the page is left as it is. Every operator of a group counts (its `cm` operators are composed +/// each time), not just the group; a group of four operators costs four, so the allowance is 16 +/// groups per operator, as before, and groups with many `cm` operators pay for them. +const SCAN_PER_OP: usize = 64; const MIN_SCAN: usize = 1 << 16; #[cfg(test)] thread_local! { /// Fragment groups read, for the test that keeps a page's search linear in its content. static GROUP_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; + /// Operators group reads walked, for the test that keeps the search linear in operators. + static OP_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; } /// One fragment: its image, its box in the coordinates the run is drawn in, and its size. @@ -53,21 +57,44 @@ struct Fragment { /// The ops `q`, one or more `cm`, `Do`, `Q` starting at `i`: the index after them, the matrix /// and the name drawn. -fn group_at(ops: &[Op], i: usize) -> Option<(usize, Matrix, Vec)> { +fn group_at(ops: &[Op], i: usize, reads: &std::cell::Cell) -> Option<(usize, Matrix, Vec)> { #[cfg(test)] GROUP_READS.with(|c| c.set(c.get().saturating_add(1))); - if !ops.get(i)?.is("q") { + let mut looked = 0usize; + let found = group_scan(ops, i, &mut looked); + // Every operator looked at is charged to the page's search allowance, whether or not the + // group turns out to be one. + reads.set(reads.get().saturating_add(looked)); + #[cfg(test)] + OP_READS.with(|c| c.set(c.get().saturating_add(looked))); + found +} + +/// [`group_at`]'s search, counting in `looked` each operator it examines. +fn group_scan(ops: &[Op], i: usize, looked: &mut usize) -> Option<(usize, Matrix, Vec)> { + let q = ops.get(i)?; + *looked += 1; + if !q.is("q") { return None; } let mut j = i + 1; let mut m = Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0]); while let Some(op) = ops.get(j).filter(|o| o.is("cm")) { + *looked += 1; m = Matrix::from_operands(&op.operands)?.then(&m); j += 1; } - let draw = ops.get(j).filter(|o| o.is("Do"))?; + let draw = ops.get(j)?; + *looked += 1; + if !draw.is("Do") { + return None; + } let name = draw.name(0)?.to_vec(); - ops.get(j + 1).filter(|o| o.is("Q"))?; + let end = ops.get(j + 1)?; + *looked += 1; + if !end.is("Q") { + return None; + } (j > i + 1).then_some((j + 2, m, name)) } @@ -152,7 +179,7 @@ fn stacked(run: &mut [Fragment]) -> Option>> { /// The fragment groups from `i` on (at most `limit`) while they form a picture drawn row by row /// or strip by strip in one direction, up to a gap or a jump. -fn drawn_in_sequence(ops: &[Op], i: usize, limit: usize, xobjects: &Dict, doc: &Document) -> Vec { +fn drawn_in_sequence(ops: &[Op], i: usize, limit: usize, xobjects: &Dict, doc: &Document, reads: &std::cell::Cell) -> Vec { let mut run: Vec = Vec::new(); let mut row_x = [0.0f64, 0.0]; let mut cur_x = [0.0f64, 0.0]; @@ -163,7 +190,7 @@ fn drawn_in_sequence(ops: &[Op], i: usize, limit: usize, xobjects: &Dict, doc: & let mut complete = 0usize; let mut j = i; while run.len() < limit { - let Some((next, m, name)) = group_at(ops, j) else { break }; + let Some((next, m, name)) = group_at(ops, j, reads) else { break }; let Some((image, width, height, ..)) = fragment_image(doc, xobjects, &name) else { break }; let [a, _, _, d, e, f] = m.0; let f2 = Fragment { image, rect: [e, f, e + a, f + d], width, height }; @@ -240,10 +267,10 @@ fn drawn_in_sequence(ops: &[Op], i: usize, limit: usize, xobjects: &Dict, doc: & } /// The op index after the first `count` fragment groups from `i`. -fn run_end(ops: &[Op], i: usize, count: usize) -> usize { +fn run_end(ops: &[Op], i: usize, count: usize, reads: &std::cell::Cell) -> usize { let mut j = i; for _ in 0..count { - match group_at(ops, j) { + match group_at(ops, j, reads) { Some((next, ..)) => j = next, None => break, } @@ -386,9 +413,9 @@ fn merge_page( let mut out: Vec = Vec::with_capacity(ops.len()); let (mut i, mut merged, mut k) = (0, 0, 0); let budget = ops.len().saturating_mul(SCAN_PER_OP).max(MIN_SCAN); - let mut scanned = 0usize; + let reads = std::cell::Cell::new(0usize); while i < ops.len() { - if scanned > budget { + if reads.get() > budget { out.extend(ops.get(i..).unwrap_or_default().iter().cloned()); break; } @@ -396,7 +423,7 @@ fn merge_page( let mut run = Vec::new(); let mut kind: Option<(Object, usize)> = None; let mut j = i; - while let Some((next, m, name)) = group_at(&ops, j) { + while let Some((next, m, name)) = group_at(&ops, j, &reads) { let Some((image, width, height, cs, n)) = fragment_image(doc, &xobjects, &name) else { break }; let [a, b, c, d, e, f] = m.0; if b.abs() > 1e-9 || c.abs() > 1e-9 || !(a > 0.0 && d > 0.0) { @@ -410,13 +437,11 @@ fn merge_page( run.push(Fragment { image, rect: [e, f, e + a, f + d], width, height }); j = next; } - scanned = scanned.saturating_add(run.len()).saturating_add(1); // Not one picture as a whole: a prefix drawn row by row or strip by strip may be. let mut rows = if run.len() >= MIN_FRAGMENTS { stacked(&mut run) } else { None }; if run.len() >= MIN_FRAGMENTS && rows.is_none() { - run = drawn_in_sequence(&ops, i, run.len(), &xobjects, doc); - j = run_end(&ops, i, run.len()); - scanned = scanned.saturating_add(run.len().saturating_mul(2)).saturating_add(2); + run = drawn_in_sequence(&ops, i, run.len(), &xobjects, doc, &reads); + j = run_end(&ops, i, run.len(), &reads); rows = stacked(&mut run); } let Some(((cs, n), rows)) = kind.zip(rows).filter(|_| run.len() >= MIN_FRAGMENTS) else { @@ -773,4 +798,37 @@ mod tests { assert_eq!((merged, report.merged_images), (0, 0)); } } + + /// The same with many `cm` operators in each group: the search reads a bounded number of operators + /// for each one of the page, however the matrices are spread over the groups. + #[test] + fn groups_with_many_matrices_are_charged_for_each_operator() { + let mut doc = Document::new_empty(); + let mut image = Dict::new(); + for (k, v) in + [("Subtype", Object::name("Image")), ("Width", Object::Int(1)), ("Height", Object::Int(1)), ("BitsPerComponent", Object::Int(8))] + { + image.set(k.as_bytes().to_vec(), v); + } + image.set(b"ColorSpace".to_vec(), Object::name("DeviceGray")); + let image = doc.add(Object::Stream(Stream::from_raw(image, vec![0]))); + let groups = 400; + let content = doc.add(Object::Stream(Stream::flate( + Dict::new(), + format!("q 10 0 0 10 0 0 cm {}/Im Do Q\n", "1 0 0 1 0 0 cm ".repeat(49)).repeat(groups).as_bytes(), + ))); + let mut xobjects = Dict::new(); + xobjects.set(b"Im".to_vec(), Object::Ref(image)); + let mut resources = Dict::new(); + resources.set(b"XObject".to_vec(), Object::Dict(xobjects)); + let mut page = Dict::new(); + page.set(b"Contents".to_vec(), Object::Ref(content)); + page.set(b"Resources".to_vec(), Object::Dict(resources)); + let page = doc.add(Object::Dict(page)); + OP_READS.with(|c| c.set(0)); + let merged = merge_page(&mut doc, page, &mut Report::default(), &mut |_| true, &mut ImageBudget::with_work(u64::MAX)); + assert_eq!(merged.ok(), Some(0)); + let reads = OP_READS.with(|c| c.get()); + assert!(reads <= 80 * 53 * groups, "{reads} operator reads"); + } } diff --git a/crates/optimize/src/links.rs b/crates/optimize/src/links.rs index 5b74ad7023..bef69c43b7 100644 --- a/crates/optimize/src/links.rs +++ b/crates/optimize/src/links.rs @@ -52,8 +52,12 @@ fn tree_entries(doc: &Document, node: &Object) -> Vec<(Vec, Object)> { } } } - if let Some(kids) = d.get(b"Kids").map(|x| doc.resolve(x)).and_then(|x| x.as_array().cloned()) { - queue.extend(kids.into_iter().map(|k| (k, depth + 1))); + // A kids array shared by reference is read once, and only kids not yet seen are queued: + // nodes sharing one array can't multiply the queue. + if let Some(kids) = + d.get(b"Kids").filter(|k| !matches!(k, Object::Ref(r) if !seen.insert(*r))).map(|x| doc.resolve(x)).and_then(|x| x.as_array().cloned()) + { + queue.extend(kids.into_iter().filter(|k| !matches!(k, Object::Ref(r) if seen.contains(r))).map(|k| (k, depth + 1))); } } out diff --git a/crates/optimize/tests/optimize.rs b/crates/optimize/tests/optimize.rs index 7da6151deb..eee2e9570c 100644 --- a/crates/optimize/tests/optimize.rs +++ b/crates/optimize/tests/optimize.rs @@ -5613,6 +5613,70 @@ fn a_picture_whose_mask_is_not_a_stencil_is_left_alone() { } } +/// A resource dictionary shared by reference between thousands of pages is walked once by the +/// space audit, not once per page. +#[test] +fn space_audit_walks_a_shared_resource_dictionary_once() { + let mut doc = Document::new_empty(); + let img = image(&mut doc, 4, 4, 3, false, None); + let mut x = Dict::new(); + for i in 0..5_000 { + x.set(format!("Im{i}").into_bytes(), Object::Ref(img)); + } + let mut res = Dict::new(); + res.set(b"XObject".to_vec(), Object::Dict(x)); + let shared = doc.add(Object::Dict(res)); + let pages = doc.root().and_then(|r| doc.get(r).as_dict().and_then(|d| d.reference(b"Pages"))).unwrap(); + let mut kids = Vec::new(); + for _ in 0..2_000 { + let mut p = Dict::new(); + p.set(b"Type".to_vec(), Object::name("Page")); + p.set(b"Parent".to_vec(), Object::Ref(pages)); + p.set(b"MediaBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 612.into(), 792.into()])); + p.set(b"Resources".to_vec(), Object::Ref(shared)); + kids.push(Object::Ref(doc.add(Object::Dict(p)))); + } + doc.update_dict(pages, |d| { + d.set(b"Count".to_vec(), Object::Int(kids.len() as i64)); + d.set(b"Kids".to_vec(), Object::Array(kids)); + }) + .unwrap(); + let started = std::time::Instant::now(); + let audit = audit_space(&doc, 1 << 20); + assert!(started.elapsed() < std::time::Duration::from_secs(5), "{:?}", started.elapsed()); + assert!(audit.iter().any(|u| u.category == SpaceCategory::Images && u.bytes > 0), "{audit:?}"); +} + +/// A name tree whose nodes all share one kids array (by reference) is read once per node, not +/// once per node per kid: the queue can't grow to the nodes times the kids. +#[test] +fn a_name_tree_sharing_one_kids_array_is_walked_once() { + let mut doc = Document::new_empty(); + let p = page(&mut doc, &[], "BT ET"); + let s = |t: &str| Object::String(pdfcraft_cos::PdfString::literal(t.as_bytes().to_vec())); + let fit = Object::Array(vec![Object::Ref(p), Object::name("Fit")]); + let kids = doc.add(Object::Array(Vec::new())); + let mut nodes = Vec::new(); + for i in 0..3_000 { + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Ref(kids)); + node.set(b"Names".to_vec(), Object::Array(vec![s(&format!("d{i}")), fit.clone()])); + nodes.push(Object::Ref(doc.add(Object::Dict(node)))); + } + doc.set(kids, Object::Array(nodes)); + let mut tree = Dict::new(); + tree.set(b"Kids".to_vec(), Object::Ref(kids)); + let tree = doc.add(Object::Dict(tree)); + let mut names = Dict::new(); + names.set(b"Dests".to_vec(), Object::Ref(tree)); + let root = doc.root().unwrap(); + doc.update_dict(root, |c| c.set(b"Names".to_vec(), Object::Dict(names))).unwrap(); + let started = std::time::Instant::now(); + let report = optimize(&mut doc, &Settings::default()).unwrap(); + assert!(started.elapsed() < std::time::Duration::from_secs(10), "{:?}", started.elapsed()); + assert_eq!(report.unreferenced_dests, 3_000, "every node's destination was found, once"); +} + /// Pictures run on several threads: what comes out doesn't depend on how many. #[test] fn the_same_file_comes_out_with_any_number_of_threads() { diff --git a/crates/preflight/src/lib.rs b/crates/preflight/src/lib.rs index 7b6e960006..90c6d9db83 100644 --- a/crates/preflight/src/lib.rs +++ b/crates/preflight/src/lib.rs @@ -203,6 +203,7 @@ pub const WORDINGS: &[&str] = &[ "A graphics state has the rendering intent {}, which isn't a standard one", "A graphics state has a rendering intent that isn't a name", "The form asks viewers to make appearances (NeedAppearances)", + "Some content could not be read for device colour", // Pages, actions, annotations and fields. "Page {} has additional actions", "A {} action isn't allowed", @@ -229,7 +230,8 @@ pub struct Declared { /// `pdfaid:part` and `pdfaid:conformance`, e.g. (2, "B"). pub pdfa: Option<(u8, String)>, pub pdfua: Option, - /// Output intents' condition identifiers. + /// Output intents' condition identifiers: every intent the document lists (each object + /// once), each identifier cut to [`MAX_IDENTIFIER_BYTES`]. pub output_intents: Vec, } @@ -276,10 +278,27 @@ pub fn declared(doc: &Document) -> Declared { d.pdfua = xmp::identifier(&x, xmp::PDFUA_ID, "part").and_then(|p| p.parse().ok()); } let cat = catalog(doc); - if let Some(list) = cat.get(b"OutputIntents").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()) { - for oi in list { - if let Some(od) = doc.resolve(&oi).as_dict() { - let id = od.get(b"OutputConditionIdentifier").and_then(|v| doc.resolve(v).as_string().map(|s| s.to_text())).unwrap_or_default(); + let list = cat.get(b"OutputIntents").map(|o| doc.resolve(o)); + if let Some(list) = list.as_deref().and_then(Object::as_array) { + // One intent object listed again is one declaration: its identifier is not copied once + // per listing. Every distinct one is kept (verification reads them all too), and only + // the start of an identifier longer than any real one. + let mut seen = std::collections::HashSet::new(); + for oi in list.iter().filter(|oi| oi.as_ref().is_none_or(|r| seen.insert(r))) { + if let Some(od) = doc.resolve(oi).as_dict() { + let id = od + .get(b"OutputConditionIdentifier") + .and_then(|v| { + doc.resolve(v).as_string().map(|s| { + if s.bytes.len() <= MAX_IDENTIFIER_BYTES { + s.to_text() + } else { + // An even cut keeps UTF-16 text whole code units. + pdfcraft_cos::PdfString { bytes: s.bytes[..MAX_IDENTIFIER_BYTES & !1].to_vec(), hex: s.hex }.to_text() + } + }) + }) + .unwrap_or_default(); d.output_intents.push(id); } } @@ -287,6 +306,14 @@ pub fn declared(doc: &Document) -> Declared { d } +/// The most bytes of an output condition identifier [`declared`] keeps (a real one is a few +/// dozen). +const MAX_IDENTIFIER_BYTES: usize = 1024; + +/// Decoded bytes one verification reads looking for device colour, all content streams +/// together. Past it, content is not inspected, and the report says so. +const MAX_COLOUR_DECODED: usize = 1 << 30; + /// Every indirect object, with its reference. fn objects(doc: &Document) -> Vec<(ObjRef, std::sync::Arc)> { doc.object_numbers() @@ -361,6 +388,9 @@ struct DeviceColour { /// space, which PDF/A accepts whatever the output intent (ISO 19005-2 6.2.4.3). default_rgb: bool, default_cmyk: bool, + /// Content that could not be read (undecodable, or past [`MAX_COLOUR_DECODED`]) may use + /// device colour that was not seen. + incomplete: bool, } impl DeviceColour { @@ -388,27 +418,35 @@ const MAX_DIRECT_DEPTH: u8 = 8; /// The glyph procedures of every Type 3 font in `d`: `d` itself, or a font held directly inside /// it (a font can sit inline in a resource dictionary). -fn type3_procs(doc: &Document, d: &Dict, depth: u8, out: &mut std::collections::HashSet) { +fn type3_procs(doc: &Document, d: &Dict, depth: u8, read: &mut std::collections::HashSet, out: &mut std::collections::HashSet) { if d.name(b"Subtype") == Some(b"Type3") - && let Some(procs) = d.get(b"CharProcs").map(|c| doc.resolve(c)) - && let Some(procs) = procs.as_dict() + && let Some(c) = d.get(b"CharProcs") + // One glyph dictionary shared by many fonts is read once: its entries are the same. + && c.as_ref().is_none_or(|r| read.insert(r)) + && let Some(procs) = doc.resolve(c).as_dict() { out.extend(procs.iter().filter_map(|(_, p)| p.as_ref())); } for (_, v) in d.iter() { - type3_procs_within(doc, v, depth.saturating_add(1), out); + type3_procs_within(doc, v, depth.saturating_add(1), read, out); } } -fn type3_procs_within(doc: &Document, o: &Object, depth: u8, out: &mut std::collections::HashSet) { +fn type3_procs_within( + doc: &Document, + o: &Object, + depth: u8, + read: &mut std::collections::HashSet, + out: &mut std::collections::HashSet, +) { if depth > MAX_DIRECT_DEPTH { return; } match o { - Object::Dict(d) => type3_procs(doc, d, depth, out), + Object::Dict(d) => type3_procs(doc, d, depth, read, out), Object::Array(a) => { for v in a { - type3_procs_within(doc, v, depth.saturating_add(1), out); + type3_procs_within(doc, v, depth.saturating_add(1), read, out); } } _ => {} @@ -430,9 +468,10 @@ fn referenced_content(doc: &Document, objs: &[(ObjRef, std::sync::Arc)]) }; out.extend(contents.iter().filter_map(Object::as_ref)); } + let mut read = std::collections::HashSet::new(); for (_, o) in objs { if let Some(d) = dict_of(o) { - type3_procs(doc, d, 0, &mut out); + type3_procs(doc, d, 0, &mut read, &mut out); } } out @@ -560,6 +599,7 @@ fn mark_cs_object(doc: &Document, cs: &Object, used: &mut DeviceColour, depth: u fn uses_device_colour(doc: &Document, objs: &[(ObjRef, std::sync::Arc)]) -> DeviceColour { let mut used = DeviceColour::default(); let referenced = referenced_content(doc, objs); + let mut budget = pdfcraft_cos::DecodeBudget::new(MAX_COLOUR_DECODED, usize::MAX); for (r, o) in objs { let Some(d) = dict_of(o) else { continue }; // A resource dictionary's /ColorSpace, whether it is this object or the page's inline @@ -586,9 +626,14 @@ fn uses_device_colour(doc: &Document, objs: &[(ObjRef, std::sync::Arc)]) if !(used.gray && used.rgb && used.cmyk) && let Object::Stream(s) = &**o && (referenced.contains(r) || is_content(&s.dict)) - && let Ok(data) = s.decoded() { - let _ = content_colour(&data, &mut used); + match budget.decode(s) { + Ok(data) => { + let _ = content_colour(&data, &mut used); + } + // Not read: it may draw in a device space nothing above saw. + Err(_) => used.incomplete = true, + } } } used @@ -830,6 +875,9 @@ fn check_output_intent( issue: &mut impl FnMut(&'static str, &'static str, Option, Option, bool), ) { let used = uses_device_colour(doc, objs); + if used.incomplete { + issue("6.2.3", "Some content could not be read for device colour", None, None, false); + } match pdfa_intent_profile(doc) { None if used.any() => { issue("6.2.3", "Device colour is used, but there is no PDF/A output intent", None, None, !used.cmyk); @@ -1215,22 +1263,56 @@ const MAX_ACTION_CHAIN: usize = 1024; fn chain_forbidden(doc: &Document, a: &Dict) -> Option { let mut read = 0; let mut seen = std::collections::HashSet::new(); - // Actions written in an indirect `/Next` array, copied out of it. - let mut held: Vec = Vec::new(); + // Objects `/Next` refers to (an action, or an array of them), each read once and borrowed: + // one array shared by many chains is not copied per chain. + let mut held: Vec> = Vec::new(); if let Some(s) = walk_chain(doc, a, &mut read, &mut seen, &mut held) { return Some(s); } - while let Some(d) = held.pop() { - if let Some(s) = walk_chain(doc, &d, &mut read, &mut seen, &mut held) { - return Some(s); + while let Some(o) = held.pop() { + let dicts: Vec<&Dict> = match &*o { + Object::Dict(d) => vec![d], + Object::Array(list) => { + // An array read is work too, with its entries: a chain of arrays alone must + // still reach the limit. + read = read.saturating_add(1 + list.len().min(MAX_ACTION_CHAIN)); + if read > MAX_ACTION_CHAIN { + return Some("too deeply chained".into()); + } + // More than the chain limit can't be read anyway. + hold_referenced(doc, list, &mut seen, &mut held); + list.iter().filter_map(Object::as_dict).take(MAX_ACTION_CHAIN).collect() + } + _ => continue, + }; + for d in dicts { + if let Some(s) = walk_chain(doc, d, &mut read, &mut seen, &mut held) { + return Some(s); + } } } None } -/// [`chain_forbidden`] for the actions reachable from `a` through direct `/Next` entries. The -/// actions of each indirect `/Next` array (each read once) go to `held`. -fn walk_chain(doc: &Document, a: &Dict, read: &mut usize, seen: &mut std::collections::HashSet, held: &mut Vec) -> Option { +/// Queue the objects the references in a `/Next` array name, each once. +fn hold_referenced(doc: &Document, list: &[Object], seen: &mut std::collections::HashSet, held: &mut Vec>) { + for r in list.iter().filter_map(Object::as_ref).take(MAX_ACTION_CHAIN) { + if seen.insert(r) { + // Resolved through any alias: a reference to a reference names its object's value. + held.push(doc.resolve(&Object::Ref(r))); + } + } +} + +/// [`chain_forbidden`] for the actions reachable from `a` through direct `/Next` entries. What +/// an indirect `/Next`, or a reference inside a `/Next` array, names goes to `held`. +fn walk_chain( + doc: &Document, + a: &Dict, + read: &mut usize, + seen: &mut std::collections::HashSet, + held: &mut Vec>, +) -> Option { let mut stack = vec![a]; while let Some(d) = stack.pop() { *read += 1; @@ -1242,13 +1324,16 @@ fn walk_chain(doc: &Document, a: &Dict, read: &mut usize, seen: &mut std::collec } match d.get(b"Next") { Some(Object::Dict(next)) => stack.push(next), - Some(Object::Array(list)) => stack.extend(list.iter().filter_map(Object::as_dict)), - Some(Object::Ref(r)) if seen.insert(*r) => { - if let Object::Array(list) = &*doc.get(*r) { - // More than the chain limit can't be read anyway. - held.extend(list.iter().filter_map(Object::as_dict).take(MAX_ACTION_CHAIN).cloned()); + Some(Object::Array(list)) => { + *read = read.saturating_add(list.len().min(MAX_ACTION_CHAIN)); + // A wide array is refused before any of it is queued. + if *read > MAX_ACTION_CHAIN { + return Some("too deeply chained".into()); } + hold_referenced(doc, list, seen, held); + stack.extend(list.iter().filter_map(Object::as_dict)); } + Some(Object::Ref(r)) if seen.insert(*r) => held.push(doc.resolve(&Object::Ref(*r))), _ => {} } } @@ -1291,6 +1376,26 @@ fn strip_actions(doc: &Document, d: &mut Dict) -> bool { changed } +/// Take the annotations `gone` off every page that lists them, each page's list rewritten once, and +/// replace their indirect objects with `Null`. +fn remove_from_pages(doc: &mut Document, gone: &std::collections::HashSet) -> Result<(), pdfcraft_cos::CosError> { + if gone.is_empty() { + return Ok(()); + } + for p in pdfcraft_model::pages(doc) { + let annots = p.dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default(); + let listed = |a: &Object| a.as_ref().is_some_and(|r| gone.contains(&r)); + if annots.iter().any(listed) { + let kept: Vec = annots.into_iter().filter(|a| !listed(a)).collect(); + doc.update_dict(p.obj, |pd| pd.set(b"Annots".to_vec(), Object::Array(kept)))?; + } + } + for r in gone { + doc.set(*r, Object::Null); + } + Ok(()) +} + /// Collects every indirect annotation reference listed in any page's `/Annots` array. fn page_annot_refs(doc: &Document) -> std::collections::HashSet { let mut refs = std::collections::HashSet::new(); @@ -1304,19 +1409,6 @@ fn page_annot_refs(doc: &Document) -> std::collections::HashSet { refs } -/// Removes annotation `r` from every page's `/Annots` list and replaces its indirect object with `Null`. -fn remove_annot_from_pages(doc: &mut Document, r: ObjRef) -> Result<(), pdfcraft_cos::CosError> { - for p in pdfcraft_model::pages(doc) { - let annots = p.dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default(); - if annots.iter().any(|a| a.as_ref() == Some(r)) { - let kept: Vec = annots.into_iter().filter(|a| a.as_ref() != Some(r)).collect(); - doc.update_dict(p.obj, |pd| pd.set(b"Annots".to_vec(), Object::Array(kept)))?; - } - } - doc.set(r, Object::Null); - Ok(()) -} - fn fix_annot_or_field(doc: &Document, nd: &mut Dict, is_annot: bool, sub: Option<&[u8]>, actions: &mut usize, flags: &mut usize) -> bool { let mut changed = strip_actions(doc, nd); *actions += changed as usize; @@ -1413,8 +1505,10 @@ pub fn convert(doc: &mut Document, level: Level) -> Result Result::to_vec); let sub = d.name(b"Subtype").map(<[u8]>::to_vec); - let is_annot = matches!(&*o, Object::Dict(_)) && is_annotation(doc, &d, ty.as_deref(), sub.as_deref(), page_annots.contains(&r)); + let is_annot = matches!(&*o, Object::Dict(_)) + // A widget is an annotation whether or not it says `/Type /Annot` (the form reader + // finds it by its field), so its actions go the same way. + && (is_annotation(doc, &d, ty.as_deref(), sub.as_deref(), page_annots.contains(&r)) || sub.as_deref() == Some(b"Widget")); if is_annot && sub.as_deref().is_some_and(|s| FORBIDDEN_ANNOTS.contains(&s)) { - remove_annot_from_pages(doc, r)?; + // Taken off its page below, all of them in one pass over the pages. + forbidden_annots.insert(r); removed += 1; continue; } @@ -1489,6 +1588,7 @@ pub fn convert(doc: &mut Document, level: Level) -> Result>", pages, page, &link(&format!("<< /S /GoTo /D [3 0 R /Fit] /Next {js} >>"))], ""), // `/Next` refers to an array that holds the action directly. build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link("<< /S /GoTo /D [3 0 R /Fit] /Next 5 0 R >>"), &format!("[{js}]")], ""), + // `/Next` refers to the action itself, which carries `/P` and `/K` as a structure + // element would: those keys must not exempt it. + build( + &[ + "<< /Type /Catalog /Pages 2 0 R >>", + pages, + page, + &link("<< /S /GoTo /D [3 0 R /Fit] /Next 5 0 R >>"), + "<< /S /JavaScript /JS (app.alert(1)) /P 3 0 R /K 0 >>", + ], + "", + ), + // A `/Next` array refers to the action. + build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link("<< /S /GoTo /D [3 0 R /Fit] /Next [5 0 R] >>"), js], ""), ]; for (i, mut doc) in cases.into_iter().enumerate() { let found = messages(&verify(&doc, Level::A2b)).contains(&"A JavaScript action isn't allowed"); @@ -495,6 +509,116 @@ fn an_output_intent_profile_header_is_read_within_a_size_cap() { } } +/// A widget the form reader finds by its field is an annotation whether or not it writes +/// `/Type /Annot`: conversion takes its additional actions off all the same. +#[test] +fn a_widget_without_a_type_loses_its_additional_actions() { + let pages = "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>"; + let page = "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>"; + let widget = "<< /Subtype /Widget /Rect [0 0 10 10] /F 4 /T (f) /AA << /K << /S /JavaScript /JS (1) >> >> >>"; + let mut doc = build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, widget], ""); + convert(&mut doc, Level::A2b).unwrap(); + let w = doc.get(pdfcraft_cos::ObjRef::new(4, 0)); + assert!(w.as_dict().is_some_and(|d| d.get(b"AA").is_none()), "{w:?}"); +} + +/// Forbidden annotations on one page are taken off it in one pass, all of them. +#[test] +fn forbidden_annotations_leave_their_page_together() { + let n = 300; + let annots: Vec = (0..n).map(|i| format!("{} 0 R", i + 4)).collect(); + let page = format!("<< /Type /Page /Parent 2 0 R /Annots [{} 2 0 R] >>", annots.join(" ")); + let mut objs = + vec!["<< /Type /Catalog /Pages 2 0 R >>".to_string(), "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>".into(), page]; + objs.extend((0..n).map(|_| "<< /Type /Annot /Subtype /Sound /Rect [0 0 0 0] /P 3 0 R >>".to_string())); + let refs: Vec<&str> = objs.iter().map(String::as_str).collect(); + let mut doc = build(&refs, ""); + let r = convert(&mut doc, Level::A2b).unwrap(); + assert!(r.fixed.iter().any(|f| f.starts_with(&format!("{n}: removed multimedia"))), "{:?}", r.fixed); + let left = pdfcraft_model::pages(&doc)[0].dict.get(b"Annots").and_then(|a| doc.resolve(a).as_array().map(Vec::len)); + assert_eq!(left, Some(1)); +} + +/// Content that can't be read may draw in DeviceCMYK: convert then adds no sRGB output intent. +#[test] +fn convert_adds_no_srgb_intent_when_colour_could_not_all_be_read() { + let mut doc = build( + &[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>", + "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << >> >>", + // A /Filter that isn't a name can't be decoded: the content isn't read. + "<< /Length 9 /Filter 5 >>\nstream\nnot flate\nendstream", + ], + "", + ); + convert(&mut doc, Level::A2b).unwrap(); + assert!(declared(&doc).output_intents.is_empty()); +} + +/// A `/Next` chain of arrays alone (each array naming the next) still reaches the chain limit, +/// so the walk's work is bounded however the arrays are linked. +#[test] +fn an_array_only_next_chain_is_too_deeply_chained() { + let mut doc = Document::new_empty(); + let mut next = Object::Array(Vec::new()); + for _ in 0..2 * MAX_ACTION_CHAIN { + next = Object::Array(vec![Object::Ref(doc.add(next))]); + } + let mut action = Dict::new(); + action.set(b"S".to_vec(), Object::name("URI")); + action.set(b"Next".to_vec(), Object::Ref(doc.add(next))); + assert_eq!(chain_forbidden(&doc, &action).as_deref(), Some("too deeply chained")); + // A short chain of arrays is read through. + let mut short = Object::Array(Vec::new()); + for _ in 0..10 { + short = Object::Array(vec![Object::Ref(doc.add(short))]); + } + action.set(b"Next".to_vec(), Object::Ref(doc.add(short))); + assert_eq!(chain_forbidden(&doc, &action), None); +} + +/// A direct `/Next` array wider than the chain limit is refused as soon as it is charged, and a +/// `/Next` that reaches its array through a reference alias is read through it. +#[test] +fn a_wide_direct_next_array_is_refused_and_an_alias_is_followed() { + let uri = || { + let mut d = Dict::new(); + d.set(b"S".to_vec(), Object::name("URI")); + d + }; + let mut action = uri(); + action.set(b"Next".to_vec(), Object::Array((0..=MAX_ACTION_CHAIN).map(|_| Object::Dict(uri())).collect())); + let doc = Document::new_empty(); + assert_eq!(chain_forbidden(&doc, &action).as_deref(), Some("too deeply chained")); + // `/Next` -> 1 0 R -> 2 0 R -> [ << /S /JavaScript >> ] + let mut doc = Document::new_empty(); + let mut js = Dict::new(); + js.set(b"S".to_vec(), Object::name("JavaScript")); + let list = doc.add(Object::Array(vec![Object::Dict(js)])); + let alias = doc.add(Object::Ref(list)); + let mut action = uri(); + action.set(b"Next".to_vec(), Object::Ref(alias)); + assert_eq!(chain_forbidden(&doc, &action).as_deref(), Some("JavaScript")); +} + +/// Every distinct output intent is declared, past eight; an identifier longer than any real one +/// keeps its start. +#[test] +fn every_output_intent_is_declared_with_its_identifier_cut_to_the_bound() { + let long = "x".repeat(4 * MAX_IDENTIFIER_BYTES); + let intents: Vec = (0..9) + .map(|i| { + format!("<< /Type /OutputIntent /S /GTS_PDFA1 /OutputConditionIdentifier ({}) >>", if i == 8 { long.clone() } else { format!("c{i}") }) + }) + .collect(); + let catalog = format!("<< /Type /Catalog /Pages 2 0 R /OutputIntents [{}] >>", intents.join(" ")); + let doc = build(&[catalog.as_str(), "<< /Type /Pages /Kids [] /Count 0 >>"], ""); + let d = declared(&doc); + assert_eq!(d.output_intents.len(), 9); + assert_eq!(d.output_intents[8].len(), MAX_IDENTIFIER_BYTES); +} + #[test] fn typeless_annotations_indexed_cmyk_and_indirect_filter_elements_are_handled() { // Annotations in /Annots that omit optional /Type /Annot and /P (while keeping required /Subtype diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 80117f451d..12636dc88f 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -93,6 +93,12 @@ struct Gs { scale: f64, leading: f64, rise: f64, + /// The stroke parameters that widen a painted path beyond its construction points. + line_width: f64, + miter_limit: f64, + line_join: i64, + /// `J` / ExtGState `/LC`: a projecting square cap (2) reaches past the endpoint's corner. + line_cap: i64, /// The tiling or shading pattern selected as the fill colour (`/Name scn`), and as the stroke /// colour (`/Name SCN`). Part of the graphics state, so `q`/`Q` save and restore them with /// everything else. A pattern's cell is a content stream of its own and this pass does not @@ -394,6 +400,10 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< scale: 1.0, leading: 0.0, rise: 0.0, + line_width: 1.0, + miter_limit: 10.0, + line_join: 0, + line_cap: 0, fill_pattern: None, stroke_pattern: None, type3_inherits: false, @@ -438,6 +448,7 @@ fn process_from(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], } if !walk.path.is_empty() { done.ops.append(&mut walk.path); + done.ops.append(&mut walk.pending); } processed.push((done.ops, done.changed)); } @@ -505,6 +516,10 @@ struct Walk { path: Vec, path_box: Option<[f64; 4]>, clip: bool, + /// Line-state operators (`w`, `M`, `j`, `J`, `d`, `gs`) met while a path is open: they apply to + /// the stroke that ends it, so the path stays whole for the hit test, and they are written + /// after it, kept even when the path goes. + pending: Vec, /// Painting operators written again under a clip that cuts the regions out: unchanged as /// operators, but what they paint is no longer all there. clipped: HashSet, @@ -526,6 +541,7 @@ impl Walk { path: Vec::new(), path_box: None, clip: false, + pending: Vec::new(), clipped: HashSet::new(), out: Output::default(), } @@ -542,14 +558,20 @@ impl Walk { self.paint_path(scope, op, piece); return; } + if !self.path.is_empty() && matches!(o, b"w" | b"M" | b"j" | b"J" | b"d" | b"gs") { + self.graphics_state(doc, scope, &op); + self.pending.push(op); + return; + } if !self.path.is_empty() { // A path not ended by a painting operator: pass it through. piece.ops.append(&mut self.path); + piece.ops.append(&mut self.pending); self.path_box = None; self.clip = false; } let kept = match o { - b"q" | b"Q" | b"cm" | b"Tf" | b"gs" => { + b"q" | b"Q" | b"cm" | b"Tf" | b"gs" | b"w" | b"M" | b"j" | b"J" => { self.graphics_state(doc, scope, &op); Some(op) } @@ -627,11 +649,35 @@ impl Walk { self.path.push(op); } + /// The box a stroke of the current path can paint, grown from its construction box `b` by + /// what the pen adds: half the line width (a width of 0 is one device pixel, taken as one + /// unit) scaled by the CTM's largest stretch, and never less than half a unit of page space + /// (a renderer paints a hairline, or a line a CTM shrinks, about a device pixel wide); times + /// the miter limit when the path has joins and they are mitred (a rectangle has four), or + /// √2 for a projecting square cap (its outer corner). Caps and joins never reach further, so + /// a hit test on it is conservative, and a horizontal or vertical line no longer has a + /// zero-area box. + fn stroke_box(&self, b: [f64; 4]) -> [f64; 4] { + let m = self.gs.ctm.0; + let scale = (m[0] * m[0] + m[1] * m[1] + m[2] * m[2] + m[3] * m[3]).sqrt(); + let joins = self.path.iter().filter(|op| matches!(op.op.as_slice(), b"l" | b"c" | b"v" | b"y" | b"h" | b"re")).count() >= 2 + || self.path.iter().any(|op| op.op.as_slice() == b"re"); + let miter = if joins && self.gs.line_join == 0 { self.gs.miter_limit.max(1.0) } else { 1.0 }; + let reach = if self.gs.line_cap == 2 { miter.max(std::f64::consts::SQRT_2) } else { miter }; + let pad = (self.gs.line_width.max(1.0) / 2.0 * scale).max(0.5) * reach; + [b[0] - pad, b[1] - pad, b[2] + pad, b[3] + pad] + } + /// A painting operator ends the path; one under a region is removed (or kept as a clip). fn paint_path(&mut self, scope: &mut Scope<'_>, op: Op, piece: &mut Piece) { + let strokes = matches!(op.op.as_slice(), b"S" | b"s" | b"B" | b"B*" | b"b" | b"b*"); + // A `W`/`W*` makes the path as constructed the clip; the stroke expansion only describes + // what the painting itself covers. + let path_box = self.path_box.take(); + let bbox = path_box.map(|b| if strokes { self.stroke_box(b) } else { b }); let body = std::mem::take(&mut self.path); - let bbox = self.path_box.take(); let clips = std::mem::replace(&mut self.clip, false); + let pending = std::mem::take(&mut self.pending); let hit = scope.mode == Mode::Apply && op.op.as_slice() != b"n" && bbox.is_some_and(|b| scope.hits(b)); if !hit { // The paint survives, so the pattern it draws with is still in use. `n` paints @@ -641,25 +687,30 @@ impl Walk { let fills = matches!(o, b"f" | b"F" | b"f*" | b"B" | b"B*" | b"b" | b"b*"); uses_patterns(&mut self.out, &self.gs, fills, strokes); piece.ops.extend(body); + piece.ops.extend(pending); piece.ops.push(op); return; } let b = bbox.unwrap_or_default(); piece.changed = true; scope.report.paths_removed += 1; - if scope.covered(b) && clips { + if scope.covered(path_box.unwrap_or(b)) && clips { // Everything this clip lets through is under a mark, so an empty clip hides the same // content. The original outline is confidential and must not stay in the stream. + piece.ops.extend(pending); for o in ["re", "W", "n"] { let operands = if o == "re" { vec![num(0.0); 4] } else { vec![] }; piece.ops.push(Op::new(o, operands)); } + } else { + piece.ops.extend(pending); } // Partial vector redaction must not retain underlying drawing commands // behind clipping (F-008). Refuse partial clip retention and remove the path. } fn graphics_state(&mut self, doc: &Document, scope: &mut Scope<'_>, op: &Op) { + let last = op.nums::<1>().map(|[v]| v); match op.op.as_slice() { b"q" => self.stack.push((self.gs.clone(), self.tm, self.tlm)), b"Q" => { @@ -673,12 +724,24 @@ impl Walk { } } b"Tf" => self.select_font(doc, scope, op), + // The operand before the operator, as the renderer reads it: `1 12 w` is a width of 12. + b"w" => self.gs.line_width = last.unwrap_or(self.gs.line_width), + b"M" => self.gs.miter_limit = last.unwrap_or(self.gs.miter_limit), + b"j" => self.gs.line_join = last.map_or(self.gs.line_join, |j| j as i64), + b"J" => self.gs.line_cap = last.map_or(self.gs.line_cap, |j| j as i64), _ => { if let Some((font, vertical, size)) = op.last_name().and_then(|n| extgstate_font(doc, scope, &self.ext_gstates, n, &self.fallback)) { (self.gs.font, self.gs.vertical, self.gs.size) = (font, vertical, size); // As for `Tf`: a Type 3 font without /Resources paints through this scope's. self.gs.type3_inherits = op.last_name().is_some_and(|n| extgstate_type3_inherits(doc, &self.ext_gstates, n)); } + if let Some(state) = op.name(0).and_then(|n| self.ext_gstates.get(n)).and_then(|s| doc.resolve(s).as_dict().cloned()) { + let num = |key: &[u8]| state.get(key).and_then(|v| doc.resolve(v).as_f64()); + self.gs.line_width = num(b"LW").unwrap_or(self.gs.line_width); + self.gs.miter_limit = num(b"ML").unwrap_or(self.gs.miter_limit); + self.gs.line_join = num(b"LJ").map_or(self.gs.line_join, |j| j as i64); + self.gs.line_cap = num(b"LC").map_or(self.gs.line_cap, |j| j as i64); + } } } } diff --git a/crates/redact/src/interp/layers.rs b/crates/redact/src/interp/layers.rs index 362cb36620..7b8ef82120 100644 --- a/crates/redact/src/interp/layers.rs +++ b/crates/redact/src/interp/layers.rs @@ -20,7 +20,7 @@ impl<'a> Scope<'a> { // groups. A membership dictionary's default policy is AnyOn. let d = doc.resolve(o); let Some(d) = d.as_dict() else { return false }; - if d.name(b"Type") != Some(b"OCMD") { + if !has_type(doc, d, b"OCMD") { return false; } let groups: Vec = match d.get(b"OCGs").map(|g| (*doc.resolve(g)).clone()) { @@ -102,7 +102,7 @@ impl<'a> Scope<'a> { } let resolved = doc.resolve(expression); let complete = match &*resolved { - Object::Dict(d) if d.name(b"Type") == Some(b"OCG") => { + Object::Dict(d) if has_type(doc, d, b"OCG") => { if let Some(r) = reference && hidden_layers.contains(&r) { @@ -144,6 +144,11 @@ impl<'a> Scope<'a> { } *budget -= 1; let reference = expression.as_ref(); + // A reference to a reference is no listed group: the renderer rejects the alias, so it + // is not taken as visible because the object it names isn't hidden. + if reference.is_some_and(|r| matches!(*doc.get(r), Object::Ref(_))) { + return None; + } if let Some(r) = reference && !path.insert(r) { @@ -151,7 +156,7 @@ impl<'a> Scope<'a> { } let resolved = doc.resolve(expression); let result = match &*resolved { - Object::Dict(d) if d.name(b"Type") == Some(b"OCG") => reference.map(|r| !self.hidden_layers.contains(&r)), + Object::Dict(d) if has_type(doc, d, b"OCG") => reference.map(|r| !self.hidden_layers.contains(&r)), Object::Array(items) if (2..=4097).contains(&items.len()) => { let operator = match items.first() { Some(Object::Name(n)) => n.as_slice(), @@ -195,6 +200,9 @@ impl<'a> Scope<'a> { if self.hidden_layers.contains(&r) { return true; } + if matches!(*doc.get(r), Object::Ref(_)) { + return true; + } if !path.insert(r) { return true; } @@ -202,7 +210,7 @@ impl<'a> Scope<'a> { let resolved = doc.resolve(expression); let found = match &*resolved { Object::Array(items) => items.iter().any(|item| self.expression_mentions_hidden(doc, item, depth + 1, path, budget)), - Object::Dict(d) if d.name(b"Type") == Some(b"OCG") => expression.as_ref().is_none_or(|r| self.hidden_layers.contains(&r)), + Object::Dict(d) if has_type(doc, d, b"OCG") => expression.as_ref().is_none_or(|r| self.hidden_layers.contains(&r)), Object::Dict(_) => true, _ => false, }; @@ -256,3 +264,9 @@ pub(super) fn strip_hidden_layers(doc: &Document, scope: &mut Scope<'_>, ops: Ve } (out, removed) } + +/// Is this dictionary's `/Type` the given name? An indirect `/Type` is followed, as a viewer +/// follows it: a group or membership dictionary must not escape classification through one. +fn has_type(doc: &Document, d: &Dict, name: &[u8]) -> bool { + d.get(b"Type").is_some_and(|t| crate::resolve_chain(doc, t).as_name() == Some(name)) +} diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 7fd34dae12..819cae99cd 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -58,6 +58,12 @@ pub enum RedactError { NothingToApply, #[error("page {0} has content that can't be read, so it can't be redacted safely")] Unreadable(usize), + /// The page tree and the renderer count a different number of pages (a page repeated in + /// `/Kids`, say), so a mark's page can't be matched to the page the renderer drew. + #[error( + "the document's page tree and its rendered pages disagree on how many pages there are, so the redaction could not be verified; nothing was changed" + )] + AmbiguousPages, /// Redact annotations with no readable area (neither usable `/QuadPoints` nor a usable `/Rect`). /// Skipping them would leave the text under them in the file while the others report success. #[error( @@ -1137,6 +1143,19 @@ pub fn clear_marks(doc: &mut Document, pages: Option<&[usize]>) -> Result std::sync::Arc { + let mut o = doc.resolve(o); + for _ in 0..16 { + if !matches!(*o, pdfcraft_cos::Object::Ref(_)) { + break; + } + o = doc.resolve(&o); + } + o +} + /// The name to save a document under after [`apply`]: `_Redacted` before the extension, as /// Acrobat appends it ("contract.pdf" gives "contract_Redacted.pdf"). The `.pdf` extension is /// matched in any letter case. A name that already ends in `_Redacted` keeps it once. diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index cd38b9eb19..61c7cedda4 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -479,12 +479,13 @@ fn hidden_layers(doc: &Document) -> Vec { o.and_then(|g| doc.resolve(g).as_array().map(|a| a.iter().filter_map(Object::as_ref).collect())).unwrap_or_default() }; let all = refs(oc.get(b"OCGs")); - let (on, off) = (refs(d.get(b"ON")), refs(d.get(b"OFF"))); - let mut hidden: Vec = if d.name(b"BaseState") == Some(b"OFF") { - all.iter().copied().filter(|g| !on.contains(g)).collect() - } else { - off.into_iter().filter(|g| all.contains(g)).collect() - }; + // sets: the lists are the document's, and a filter over every group would scan them each time + let (on, off): (std::collections::HashSet, std::collections::HashSet) = + (refs(d.get(b"ON")).into_iter().collect(), refs(d.get(b"OFF")).into_iter().collect()); + // The default state is /BaseState, then /ON turns groups on and /OFF turns them off, so a + // group both lists name ends up off (§8.11.4.3, Table 101) — the viewer reads it the same way. + let base_off = d.get(b"BaseState").map(|b| crate::resolve_chain(doc, b)).is_some_and(|b| b.as_name() == Some(b"OFF".as_slice())); + let mut hidden: Vec = all.iter().copied().filter(|g| off.contains(g) || (base_off && !on.contains(g))).collect(); // Each rule, layer list and layer is read once, however often the rules name them: many rules // can share one long list. let known: std::collections::HashSet = all.iter().copied().collect(); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index dc94a2c5f5..43b6caaadc 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -837,6 +837,22 @@ fn vectors_are_removed_or_clipped_and_inline_images_go() { assert!(c.contains("150 150 m"), "the line elsewhere stays"); } +/// A horizontal stroke's construction box has no area, so it overlapped nothing and stayed under +/// the mark. The painted stroke is as wide as the line: a 12 pt line centred 5 pt above the mark +/// reaches into it and goes; a 1 pt line there does not and stays. +#[test] +fn a_stroke_is_removed_by_its_painted_width() { + let src = b"12 w 10 60 m 100 60 l S 1 w 10 60 m 100 60 l S 4 w 10 50 m 100 50 l S"; + let mut doc = one_page(src, "", vec![]); + mark(&mut doc, 0, &[[40.0, 45.0, 60.0, 55.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.paths_removed, 2, "{r:?}"); + let c = content(&doc, 0); + assert!(!c.contains("12 w\n10 60 m"), "the wide line reaches the mark: {c}"); + assert!(c.contains("1 w\n10 60 m\n100 60 l\nS"), "the thin line beside it stays: {c}"); + assert!(!c.contains("10 50 m"), "the line through the mark goes: {c}"); +} + /// A tiling pattern's cell is a content stream of its own and can draw text. Redacting a shape /// filled with such a pattern removed the fill from the page but left the pattern in the page's /// resources, so its cell -- with the text that was under the mark -- stayed in the saved file @@ -4151,6 +4167,8 @@ fn hidden_ocmd_policy_matches_the_default_configuration() { (Some("AllOn"), false, Some("not_off"), false), (None, false, Some("malformed_not"), true), (None, false, Some("malformed_without_ocgs"), true), + // `/VE` names the hidden group through an alias object (10 0 R is a reference to 8 0 R). + (None, false, Some("alias"), true), ]; for (policy, malformed_policy, visibility, should_remove) in cases { let mut doc = one_page( @@ -4188,9 +4206,11 @@ fn hidden_ocmd_policy_matches_the_default_configuration() { Some("malformed_without_ocgs") => { membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("UnknownOperator"), Object::Ref(ObjRef::new(8, 0))])) } + Some("alias") => membership.set(b"VE".to_vec(), Object::Array(vec![Object::name("Or"), Object::Ref(ObjRef::new(10, 0))])), _ => {} } doc.set(ObjRef::new(9, 0), Object::Dict(membership)); + doc.set(ObjRef::new(10, 0), Object::Ref(ObjRef::new(8, 0))); let root = doc.root().unwrap(); doc.update_dict(root, |d| { d.set( @@ -6670,6 +6690,131 @@ fn a_reply_on_another_page_to_a_removed_popup_goes_too() { } } +/// A hairline under a shrinking CTM still paints about a device pixel: its pad must not shrink +/// with the CTM, or a mark on that ink misses it. +#[test] +fn a_hairline_under_a_shrinking_ctm_is_padded_by_half_a_unit() { + // Device line from (10, 10) to (50, 10); the mark lies on its painted pixel, above its axis. + let mut doc = one_page(b"0.01 0 0 0.01 0 0 cm 0 w 1000 1000 m 5000 1000 l S", "", vec![]); + mark(&mut doc, 0, &[[20.0, 10.2, 30.0, 10.4]], ""); + assert_eq!(apply(&mut doc, None).unwrap().paths_removed, 1); +} + +/// One `re` has four mitred corners: it is padded by the miter limit like any joined path. +#[test] +fn a_rectangle_stroke_is_padded_for_its_mitred_corners() { + let mut doc = one_page(b"2 w 100 100 50 50 re S", "", vec![]); + // Past half the line width from the rectangle, inside the miter reach. + mark(&mut doc, 0, &[[155.0, 120.0, 160.0, 130.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().paths_removed, 1); +} + +/// A projecting square cap's outer corner reaches lw/2 x sqrt(2) from the endpoint. Under a shear +/// it maps past a pad of lw/2 times the CTM's stretch (the review's example: corner at x = 12). +#[test] +fn a_square_cap_corner_under_a_shear_is_inside_the_pad() { + let mut doc = one_page(b"1 0 1 1 0 0 cm 2 J 2 w 0 0 m 10 0 l S", "", vec![]); + mark(&mut doc, 0, &[[11.8, 0.8, 11.9, 0.95]], ""); + assert_eq!(apply(&mut doc, None).unwrap().paths_removed, 1); +} + +/// A line width set between the path and its `S` still applies to that stroke, so the stroke is +/// tested at the width it is painted with; the width itself stays, the path goes. +#[test] +fn a_width_set_between_the_path_and_its_stroke_does_not_hide_it_from_the_test() { + let mut doc = one_page(b"10 60 m 100 60 l 12 w S 5 5 m 9 9 l S", "", vec![]); + mark(&mut doc, 0, &[[40.0, 45.0, 60.0, 55.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.paths_removed, 1, "{r:?}"); + let c = content(&doc, 0); + assert!(!c.contains("10 60 m") && !c.contains("100 60 l"), "the wide line reaches the mark: {c}"); + assert!(c.contains("12 w"), "the width stays: {c}"); + assert!(c.contains("5 5 m"), "the line elsewhere stays: {c}"); +} + +/// The renderer takes a line-state operator's operand from the end: `1 12 w` strokes 12 points wide, +/// so the stroke is tested at that width. +#[test] +fn a_surplus_operand_before_the_line_width_does_not_hide_the_stroke() { + let mut doc = one_page(b"1 12 w 10 60 m 100 60 l S", "", vec![]); + mark(&mut doc, 0, &[[40.0, 45.0, 60.0, 55.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.paths_removed, 1, "{r:?}"); + assert!(!content(&doc, 0).contains("100 60 l")); +} + +/// One page drawing `SECRET` in the `/OC /L` block, with layer 7 0 R in the default +/// configuration `config` (objects 8 and up are `extra`). +fn layer_state_page(config: Vec<(&str, Object)>, resource: &str, extra: Vec>) -> Document { + let mut objects = vec![b"<< /Type /OCG /Name (Secret) >>".to_vec()]; + objects.extend(extra); + let mut doc = one_page(b"/OC /L BDC BT /F1 10 Tf 10 100 Td (SECRET) Tj ET EMC", &format!("/Properties << /L {resource} >>"), objects); + let config = Object::Dict(Dict::from_iter(config.into_iter().map(|(k, v)| (k.as_bytes().to_vec(), v)))); + let root = doc.root().unwrap(); + doc.update_dict(root, |d| { + d.set( + b"OCProperties".to_vec(), + Object::Dict(Dict::from_iter([(b"OCGs".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0))])), (b"D".to_vec(), config)])), + ); + }) + .unwrap(); + doc +} + +fn assert_layer_secret_removed(mut doc: Document) { + use crate::sanitize::{Hidden, remove_hidden, scan}; + let counts = scan(&doc); + assert!(counts.iter().any(|(h, n)| *h == Hidden::HiddenLayers && *n > 0), "{counts:?}"); + remove_hidden(&mut doc, &[Hidden::HiddenLayers]).unwrap(); + let saved = Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap(); + assert!(!pdfcraft_edit::text_lines(&saved, 0).unwrap().iter().any(|line| line.text.contains("SECRET"))); +} + +#[test] +fn an_indirect_base_state_off_hides_the_layer() { + assert_layer_secret_removed(layer_state_page(vec![("BaseState", Object::Ref(ObjRef::new(8, 0)))], "7 0 R", vec![b"/OFF".to_vec()])); +} + +#[test] +fn a_chained_base_state_off_hides_the_layer() { + assert_layer_secret_removed(layer_state_page( + vec![("BaseState", Object::Ref(ObjRef::new(8, 0)))], + "7 0 R", + vec![b"9 0 R".to_vec(), b"/OFF".to_vec()], + )); +} + +#[test] +fn a_chained_membership_type_is_evaluated() { + assert_layer_secret_removed(layer_state_page( + vec![("BaseState", Object::name("ON"))], + "8 0 R", + vec![b"<< /Type 9 0 R /OCGs [7 0 R] /P /AllOff >>".to_vec(), b"10 0 R".to_vec(), b"/OCMD".to_vec()], + )); +} + +#[test] +fn off_wins_over_on_when_the_base_state_is_off() { + assert_layer_secret_removed(layer_state_page( + vec![ + ("BaseState", Object::name("OFF")), + ("ON", Object::Array(vec![Object::Ref(ObjRef::new(7, 0))])), + ("OFF", Object::Array(vec![Object::Ref(ObjRef::new(7, 0))])), + ], + "7 0 R", + Vec::new(), + )); +} + +#[test] +fn a_membership_dictionary_with_an_indirect_type_is_evaluated() { + assert_layer_secret_removed(layer_state_page( + vec![("BaseState", Object::name("ON"))], + "8 0 R", + vec![b"<< /Type 9 0 R /OCGs [7 0 R] /P /AllOff >>".to_vec(), b"/OCMD".to_vec()], + )); +} + /// An invisible OCR-style text layer drawn in a Type3 font (what the OCR output writes for /// characters beyond WinAnsi) is redacted like any other text: its glyphs are hit-tested by /// their real advances, removed, and the verifier passes. diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index d02e76832e..bec850e302 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -390,6 +390,35 @@ fn type3_font_used_in_a_form_xobject_has_its_redacted_glyph_removed() { assert!(!full_save_holds(&doc, b"SECRET_FORM_GLYPH_PAYLOAD"), "redacted glyph program survived"); } +/// Type 3 cleanup decoded a glyph program again for every font listing it: 2 000 fonts sharing +/// one large program read it 2 000 times. It is read once now, and the cleanup still keeps the +/// glyph every font shows. +#[test] +fn a_glyph_program_many_type3_fonts_share_is_read_once() { + const FONTS: usize = 2_000; + let names: String = (0..FONTS).map(|i| format!("/T{i} {} 0 R ", 6 + i)).collect(); + let shows: String = (0..FONTS).map(|i| format!("/T{i} 10 Tf (A) Tj ")).collect(); + let mut objs = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << {names}>> >> >>").into_bytes(), + stream("", format!("BT 10 100 Td {shows}ET").as_bytes()), + stream("", [b"SHARED_GLYPH ".as_slice(), &b"0 0 m 1 1 l S ".repeat(50_000)].concat()), + ]; + objs.extend((0..FONTS).map(|_| { + b"<< /Type /Font /Subtype /Type3 /FontBBox [0 0 1000 1000] /FontMatrix [0.001 0 0 0.001 0 0] /FirstChar 65 /LastChar 65 /Widths [0] /Encoding << /Differences [65 /A] >> /CharProcs << /A 5 0 R >> >>".to_vec() + })); + let mut doc = pdf(objs); + // The glyphs are drawn at x 10 with no width, so a mark away from them leaves them all. + mark(&mut doc, 0, &[[200.0, 200.0, 250.0, 250.0]], ""); + let started = std::time::Instant::now(); + apply(&mut doc, None).unwrap(); + assert!(started.elapsed() < std::time::Duration::from_secs(60), "{:?}", started.elapsed()); + let font = doc.get(ObjRef::new(6, 0)); + let procs = doc.resolve(font.as_dict().unwrap().get(b"CharProcs").unwrap()); + assert!(procs.as_dict().unwrap().contains(b"A"), "the glyph every font shows was unlinked"); +} + /// A Type 3 font with glyphs A (its program object 7) and B (object 8), at object 6. const TYPE3: &str = "<< /Type /Font /Subtype /Type3 /FontBBox [0 -200 1000 800] /FontMatrix [0.001 0 0 0.001 0 0] /FirstChar 65 /LastChar 66 /Widths [500 500] /Encoding << /Differences [65 /A /B] >> /CharProcs << /A 7 0 R /B 8 0 R >> >>"; diff --git a/crates/redact/src/type3.rs b/crates/redact/src/type3.rs index 11d5a76f6a..1817fb3c39 100644 --- a/crates/redact/src/type3.rs +++ b/crates/redact/src/type3.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use pdfcraft_content::Pieces; -use pdfcraft_cos::{CosError, Dict, Document, ObjRef, Object}; +use pdfcraft_cos::{CosError, DecodeBudget, Dict, Document, ObjRef, Object}; use crate::{RedactError, page_streams}; @@ -130,6 +130,25 @@ struct UsedInGlyphs { properties: HashSet>, } +impl UsedInGlyphs { + fn merge(&mut self, other: &UsedInGlyphs) { + self.xobjects.extend(other.xobjects.iter().cloned()); + self.colorspaces.extend(other.colorspaces.iter().cloned()); + self.patterns.extend(other.patterns.iter().cloned()); + self.shadings.extend(other.shadings.iter().cloned()); + self.extgstates.extend(other.extgstates.iter().cloned()); + self.fonts.extend(other.fonts.iter().cloned()); + self.properties.extend(other.properties.iter().cloned()); + } +} + +/// What one Type 3 cleanup reads of glyph programs: the resource names each indirect program +/// uses, read once however many fonts share it, and one decoding allowance for all of them. +struct GlyphReads { + used: HashMap, + budget: DecodeBudget, +} + fn collect_used_resources_in_stream(data: &[u8], used: &mut UsedInGlyphs) -> Result<(), CosError> { // One operator at a time, so a glyph program of millions of them is never held whole. A byte // that can't be read anywhere in it still refuses, once the loop has read it all. @@ -731,9 +750,10 @@ pub(crate) fn unlink_type3(doc: &mut Document) -> Result<(), RedactError> { } // For each Type 3 font: unlink unused glyphs from CharProcs and prune exclusive resources. + let mut reads = GlyphReads { used: HashMap::new(), budget: DecodeBudget::page() }; for (font_key, font_dict) in &type3_fonts { let surviving = used_glyphs.get(font_key).cloned().unwrap_or_default(); - clean_type3_font(doc, &holders, &form_holders, font_key, font_dict, &surviving)?; + clean_type3_font(doc, &holders, &form_holders, font_key, font_dict, &surviving, &mut reads)?; } Ok(()) @@ -808,6 +828,7 @@ fn clean_type3_font( font_key: &FontKey, font_dict: &Dict, surviving: &HashSet>, + reads: &mut GlyphReads, ) -> Result<(), RedactError> { let Some(char_procs_val) = font_dict.get(b"CharProcs") else { return Err(CosError::Syntax { offset: 0, detail: "redaction could not establish safe Type 3 font cleanup: missing CharProcs".into() }.into()); @@ -851,6 +872,10 @@ fn clean_type3_font( let mut used_in_glyphs = UsedInGlyphs::default(); for (_, proc_val) in char_procs.iter() { + if let Some(used) = proc_val.as_ref().and_then(|r| reads.used.get(&r)) { + used_in_glyphs.merge(used); + continue; + } let resolved = doc.resolve(proc_val); let Object::Stream(s) = &*resolved else { return Err(CosError::Syntax { @@ -859,8 +884,14 @@ fn clean_type3_font( } .into()); }; - let data = doc.with_direct_parms(s).decoded()?; - collect_used_resources_in_stream(&data, &mut used_in_glyphs)?; + // Past the allowance the cleanup can't be shown complete, and the redaction is refused. + let data = reads.budget.decode(&doc.with_direct_parms(s))?; + let mut used = UsedInGlyphs::default(); + collect_used_resources_in_stream(&data, &mut used)?; + used_in_glyphs.merge(&used); + if let Some(r) = proc_val.as_ref() { + reads.used.insert(r, used); + } } if prune_type3_font_resources(doc, &mut f_res, &used_in_glyphs) { diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 7313716a2e..673d930743 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -144,12 +144,7 @@ impl<'a> Inspector<'a> { info.attachments.push(self.attachment(fs, AttachmentSource::Annotation { page, index }, text_budget, fallback)); } if subtype == "Link" { - if let Some(link) = self.link(page, index, d, info.uri_base.as_deref(), text_budget, &mut unfit_quads) { - if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") { - info.has_javascript = true; - } - info.links.push(link); - } + self.collect_link(info, (page, index), d, text_budget, &mut unfit_quads); continue; } // Not comments: form widgets, pop-ups and non-markup annotations such as the @@ -161,6 +156,20 @@ impl<'a> Inspector<'a> { if comments_full { continue; } + // Text that trims to nothing is not kept, so `retained` never sees it: charge what is + // read, so many comments sharing one huge blank string stop being decoded over and over. + let raw: usize = [&b"T"[..], b"Contents", b"M", b"NM", b"State", b"DA"] + .iter() + .filter_map(|k| match d.get(k).map(|o| self.resolve(o)) { + Ok(Object::String(b, _) | Object::Name(b)) => Some(b.len()), + _ => None, + }) + .fold(0, usize::saturating_add); + // (charged once: below, by the larger of these raw bytes and what the annotation keeps) + if kept.saturating_add(raw) > MAX_ANNOTATION_BYTES { + comments_full = true; + continue; + } let rect = rect4(self.resolve(d.get(b"Rect").unwrap_or(&Object::Null))); let c = match d.get(b"C").map(|o| self.resolve(o)) { Ok(Object::Array(c)) => Some(c), @@ -262,7 +271,7 @@ impl<'a> Inspector<'a> { text_box, popup, }; - kept = kept.saturating_add(retained(&annotation)); + kept = kept.saturating_add(retained(&annotation).max(raw)); if kept > MAX_ANNOTATION_BYTES { comments_full = true; continue; @@ -356,6 +365,23 @@ impl<'a> Inspector<'a> { Some(Link { page, index, rect, quads, target }) } + /// Lists the link annotation `d`, and notes the JavaScript it runs. JavaScript is read from the + /// action, not the listed link: once the text budget is spent the action's name is not kept. + fn collect_link(&self, info: &mut DocInfo, at: (usize, usize), d: &Dictionary, text_budget: &mut usize, unfit_quads: &mut HashSet) { + info.has_javascript |= self.runs_javascript(d); + if let Some(link) = self.link(at.0, at.1, d, info.uri_base.as_deref(), text_budget, unfit_quads) { + if matches!(&link.target, LinkTarget::Other(s) if s == "JavaScript") { + info.has_javascript = true; + } + info.links.push(link); + } + } + + /// Whether the link's own action is a JavaScript action. + fn runs_javascript(&self, d: &Dictionary) -> bool { + d.get(b"A").ok().and_then(|a| self.dict(a)).is_some_and(|a| self.name(a, b"S").as_deref() == Some("JavaScript")) + } + /// What an action does when a link or a bookmark runs it. Only a go-to's `/D` is a place in /// this document; a remote go-to's names a page or destination of another file (ISO 32000-2 /// §12.6.4.3). `None` for an action without a type, or a go-to whose destination is not here. @@ -376,7 +402,7 @@ impl<'a> Inspector<'a> { "SetOCGState" => self.layer_state(a), "GoToR" => self.remote_go_to(a).unwrap_or_else(|| LinkTarget::Other("GoToR".into())), "Launch" => self.launch_pdf(a).unwrap_or_else(|| LinkTarget::Other("Launch".into())), - other => LinkTarget::Other(other.to_string()), + other => LinkTarget::Other(spend(other.as_bytes(), text_budget, |b| String::from_utf8_lossy(b).into_owned()).unwrap_or_default()), }) } @@ -492,6 +518,53 @@ impl<'a> Inspector<'a> { // ── form fields ───────────────────────────────────────────────────────────────────────── + /// Whether `o` or one of its descendants under `/Kids` carries an action (`/A` or `/AA`). It + /// walks the same tree `field` does, bounded the same way: a field left out of the listing for + /// want of text allowance can still have its action on a widget several levels down. + pub(super) fn has_action(&self, o: &Object, depth: u32) -> bool { + self.has_action_seen(o, depth, &mut HashMap::new()) + } + + /// The walk behind [`Self::has_action`]. Each indirect descendant is read again only from a + /// shallower depth than before (the depth limit may have cut its subtree): a field tree whose + /// `/Kids` repeat the same objects would otherwise be walked exponentially, and this reads each + /// object at most once per depth. + fn has_action_seen(&self, o: &Object, depth: u32, seen: &mut HashMap) -> bool { + if depth > 32 { + return false; + } + if let Object::Reference(id) = o { + if seen.get(id).is_some_and(|&first| first <= depth) { + return false; + } + seen.insert(*id, depth); + } + let Some(d) = self.dict(o) else { return false }; + if self.carries_action(d) { + return true; + } + let kids = match d.get(b"Kids").map(|k| self.resolve(k)) { + Ok(Object::Array(k)) => k, + _ => return false, + }; + kids.iter().any(|k| self.has_action_seen(k, depth + 1, seen)) + } + + /// Whether `/A` or `/AA` here holds an action. An editor can leave an empty dictionary (or + /// array) behind, and that is no action: a document with none must not report JavaScript. An + /// `/A` is an action when it carries `/S`; an `/AA` is one when an event maps to such an action. + fn carries_action(&self, d: &Dictionary) -> bool { + let action = |o: &Object| matches!(self.resolve(o), Object::Dictionary(a) if a.get(b"S").is_ok()); + [&b"A"[..], &b"AA"[..]].iter().any(|k| match d.get(k) { + Ok(o) => match self.resolve(o) { + Object::Dictionary(x) => x.get(b"S").is_ok() || x.iter().any(|(_, v)| action(v)), + Object::Array(a) => a.iter().any(action), + _ => false, + }, + Err(_) => false, + }) + } + pub(super) fn field( &self, o: &'a Object, @@ -510,19 +583,35 @@ impl<'a> Inspector<'a> { } let Some(d) = self.dict(o) else { return }; let (parent_name, parent_ft, parent_ff) = parent.unwrap_or(("", None, None)); - let partial = self.text(d, b"T"); - let full = match (&partial, parent_name.is_empty()) { - (Some(t), true) => t.clone(), - (Some(t), false) => format!("{parent_name}.{t}"), - (None, _) => parent_name.to_string(), - }; - let ft = self.name(d, b"FT").or(parent_ft); - let ff = d.get(b"Ff").ok().and_then(|x| x.as_i64().ok()).or(parent_ff); // Non-terminal: has kids that are fields (they carry /T). Widgets-only kids are terminal. let mut kids: Vec = match d.get(b"Kids").map(|k| self.resolve(k)) { Ok(Object::Array(k)) => k.clone(), _ => Vec::new(), }; + // A field left unlisted for want of text allowance still has its actions counted, through + // its whole subtree: an action can sit on a descendant widget, not only on the field. + let unlisted = || { + if self.has_action(o, 0) { + self.field_actions.set(true); + } + }; + // A /T that does not fit the allowance lists no field (not one under its parent's name). + let Ok(partial) = self.field_text_within(d, b"T") else { + unlisted(); + return; + }; + let full = match (&partial, parent_name.is_empty()) { + (Some(t), true) => t.clone(), + (Some(t), false) => format!("{parent_name}.{t}"), + (None, _) => parent_name.to_string(), + }; + // Every field copies its parent's full name: charge it, and list no more once it is spent. + if !self.field_charge(full.len()) { + unlisted(); + return; + } + let ft = self.name(d, b"FT").or(parent_ft); + let ff = d.get(b"Ff").ok().and_then(|x| x.as_i64().ok()).or(parent_ff); let (field_kids, widget_kids): (Vec, Vec) = kids.iter().cloned().partition(|k| self.dict(k).is_some_and(|kd| kd.get(b"T").is_ok())); let field_kids: Vec<_> = field_kids.iter().filter(|k| !self.not_a_child(walk, o, k)).collect(); @@ -570,17 +659,37 @@ impl<'a> Inspector<'a> { _ => FieldKind::Unknown, }; let value = match d.get(b"V").map(|v| self.resolve(v)) { - Ok(Object::Name(n)) => Some(String::from_utf8_lossy(n).into_owned()), - Ok(Object::String(bytes, _)) => Some(text_string(bytes)), - Ok(Object::Array(a)) => Some( - a.iter() - .filter_map(|x| match self.resolve(x) { - Object::String(bytes, _) => Some(text_string(bytes)), - _ => None, - }) - .collect::>() - .join(", "), - ), + Ok(Object::Name(n)) => self.field_spend(n, |n| String::from_utf8_lossy(n).into_owned()), + Ok(Object::String(bytes, _)) => self.field_spend(bytes, text_string), + Ok(Object::Array(a)) => { + let mut shown = Vec::new(); + let mut dropped = false; + for x in a { + let text = match self.resolve(x) { + Object::String(bytes, _) => self.field_spend(bytes, text_string), + // A name entry (`/V [/Opt1 (b)]`) is a selection too, as the forms reader + // takes it; leaving it out reported a partial selection as complete. + Object::Name(n) => self.field_spend(n, |n| String::from_utf8_lossy(n).into_owned()), + _ => continue, + }; + // The ", " between selections is charged to the allowance like the text. + match text { + Some(text) if shown.is_empty() || self.field_charge(2) => shown.push(text), + _ => dropped = true, + } + } + // A selected value the allowance could not hold shows as an omission rather than + // as part of a complete selection (its few bytes are charged, fit or not). + let mut value = shown.join(", "); + if dropped { + self.field_charge(5); + if !value.is_empty() { + value.push_str(", "); + } + value.push('…'); + } + Some(value) + } Ok(Object::Dictionary(_)) if kind == FieldKind::Signature => Some("signed".into()), _ => None, }; @@ -602,9 +711,11 @@ impl<'a> Inspector<'a> { .and_then(|p| self.page_of(p)) .or_else(|| self.find_widget_page(o, &kids)) .or_else(|| named.and_then(|id| self.annotation_pages.get_or_init(|| self.annotation_pages()).get(&id).copied())); - let has_actions = d.get(b"AA").is_ok() || widget.is_some_and(|w| w.get(b"AA").is_ok() || w.get(b"A").is_ok()); + let has_actions = self.carries_action(d) + || widget.is_some_and(|w| self.carries_action(w)) + || kids.iter().any(|k| self.dict(k).is_some_and(|w| self.carries_action(w))); let rect = widget.and_then(|w| w.get(b"Rect").ok()).map(|r| rect4(self.resolve(r))); - out.push(Field { name: full, kind, value, page, tooltip: self.text(d, b"TU"), has_actions, rect }); + out.push(Field { name: full, kind, value, page, tooltip: self.field_text(d, b"TU"), has_actions, rect }); } /// Whether `k`, listed in the `/Kids` of field `o`, is not a child of `o` after all: `o` itself diff --git a/crates/render/src/inspect/core.rs b/crates/render/src/inspect/core.rs index 88dc13c34a..adf3edffbd 100644 --- a/crates/render/src/inspect/core.rs +++ b/crates/render/src/inspect/core.rs @@ -155,16 +155,34 @@ pub(super) struct Inspector<'a> { /// has no `/P`. Searching every page's `/Annots` for each such field was O(fields × /// annotations): a second on a form of 8,000 fields. pub(super) annotation_pages: OnceCell>, + /// What is left of [`MAX_FIELD_TEXT_BYTES`]: names, values and tooltips of form fields and + /// layer names, which many entries can share one string of. + pub(super) field_text: std::cell::Cell, + /// Set when a field with an action went unlisted because the text allowance was spent, so the + /// document still reports JavaScript. + pub(super) field_actions: std::cell::Cell, /// The first page widget naming each field as its `/Parent`, built when a field has no widget /// among its `/Kids`. pub(super) parent_widgets: OnceCell>, } +/// Bytes of field and layer text one inspection reads and keeps together. +pub(super) const MAX_FIELD_TEXT_BYTES: usize = 64 << 20; + impl<'a> Inspector<'a> { pub(super) fn new(doc: &'a dyn Structure) -> Self { let page_ids = doc.page_ids(); let page_index = page_ids.iter().copied().enumerate().map(|(index, id)| (id, index)).collect(); - let mut me = Self { doc, page_ids, page_index, named: HashMap::new(), annotation_pages: OnceCell::new(), parent_widgets: OnceCell::new() }; + let mut me = Self { + doc, + page_ids, + page_index, + named: HashMap::new(), + annotation_pages: OnceCell::new(), + field_text: MAX_FIELD_TEXT_BYTES.into(), + field_actions: false.into(), + parent_widgets: OnceCell::new(), + }; let mut named = HashMap::new(); if let Some(tree) = doc.catalog().ok().and_then(|c| c.get(b"Names").ok()).and_then(|o| me.dict(o)).and_then(|n| n.get(b"Dests").ok()).and_then(|o| me.dict(o)) @@ -260,7 +278,7 @@ impl<'a> Inspector<'a> { } } } - info.has_javascript |= info.fields.iter().any(|f| f.has_actions); + info.has_javascript |= info.fields.iter().any(|f| f.has_actions) || self.field_actions.get(); let needs_rendering = catalog.get(b"NeedsRendering").ok().and_then(|o| self.resolve(o).as_bool().ok()).unwrap_or(false); if let Some(xfa) = catalog.get(b"AcroForm").ok().and_then(|o| self.dict(o)).and_then(|form| form.get(b"XFA").ok()) { // A form with no fields is taken to be laid out from its template, unless its @@ -351,11 +369,49 @@ impl<'a> Inspector<'a> { /// Whitespace-trimmed metadata, spent from `budget` as [`spend`] does, so text that trims /// short or to nothing still spends its raw length. + /// [`Self::budgeted_text`] against the shared field and layer text allowance. + pub(super) fn field_text(&self, d: &Dictionary, key: &[u8]) -> Option { + let mut left = self.field_text.get(); + let text = self.budgeted_text(d, key, &mut left); + self.field_text.set(left); + text + } + + /// Converts `bytes` against the field and layer text allowance, as [`spend`] does. + pub(super) fn field_spend(&self, bytes: &[u8], convert: impl FnOnce(&[u8]) -> String) -> Option { + let mut left = self.field_text.get(); + let text = spend(bytes, &mut left, convert); + self.field_text.set(left); + text + } + + /// Spends `n` bytes of the field and layer text allowance; `false` when too few are left. + pub(super) fn field_charge(&self, n: usize) -> bool { + let left = self.field_text.get(); + self.field_text.set(left.saturating_sub(n)); + n <= left + } + pub(super) fn budgeted_text(&self, d: &Dictionary, key: &[u8], budget: &mut usize) -> Option { - let o = self.resolve(d.get(key).ok()?); - let (Object::String(bytes, _) | Object::Name(bytes)) = o else { return None }; - let text = spend(bytes, budget, |_| trimmed_text(o).unwrap_or_default())?; - (!text.is_empty()).then_some(text) + self.budgeted_text_within(d, key, budget).ok().flatten() + } + + /// [`Self::budgeted_text`], but `Err` when the text is there and does not fit `budget` + /// (against `Ok(None)`: no text, or none that trims to anything). + pub(super) fn budgeted_text_within(&self, d: &Dictionary, key: &[u8], budget: &mut usize) -> Result, ()> { + let Ok(o) = d.get(key) else { return Ok(None) }; + let o = self.resolve(o); + let (Object::String(bytes, _) | Object::Name(bytes)) = o else { return Ok(None) }; + let text = spend(bytes, budget, |_| trimmed_text(o).unwrap_or_default()).ok_or(())?; + Ok((!text.is_empty()).then_some(text)) + } + + /// [`Self::field_text`], but `Err` when the text does not fit the allowance. + pub(super) fn field_text_within(&self, d: &Dictionary, key: &[u8]) -> Result, ()> { + let mut left = self.field_text.get(); + let text = self.budgeted_text_within(d, key, &mut left); + self.field_text.set(left); + text } pub(super) fn name(&self, d: &Dictionary, key: &[u8]) -> Option { diff --git a/crates/render/src/inspect/navigation.rs b/crates/render/src/inspect/navigation.rs index c5f2a1be86..f680b46246 100644 --- a/crates/render/src/inspect/navigation.rs +++ b/crates/render/src/inspect/navigation.rs @@ -250,7 +250,7 @@ impl<'a> Inspector<'a> { None => {} } let open = d.get(b"Count").ok().and_then(|c| c.as_i64().ok()).is_some_and(|c| c > 0); - let title = self.text(d, b"Title").unwrap_or_default(); + let title = self.field_text(d, b"Title").unwrap_or_default(); let color = match d.get(b"C").map(|o| self.resolve(o)) { Ok(Object::Array(c)) => outline_color(c.iter().map(|v| self.resolve(v).as_float().ok())), _ => None, diff --git a/crates/render/src/inspect/resources.rs b/crates/render/src/inspect/resources.rs index 3dbc1c665f..6d90f715e1 100644 --- a/crates/render/src/inspect/resources.rs +++ b/crates/render/src/inspect/resources.rs @@ -121,9 +121,14 @@ impl<'a> Inspector<'a> { .unwrap_or_default(); let auto = config.map(|c| self.view_auto_state(c)).unwrap_or_default(); let Ok(Object::Array(ocgs)) = props.get(b"OCGs").map(|o| self.resolve(o)) else { return }; + let mut seen = HashSet::new(); for g in ocgs { let Some(d) = self.dict(g) else { continue }; let id = g.as_reference().ok(); + // A group listed twice is one layer (and its name is read once). + if id.is_some_and(|id| !seen.insert(id)) { + continue; + } // The View auto state, when it manages the group, then the configuration. let visible = match id { Some(id) if let Some(&state) = auto.get(&id) => state, @@ -132,7 +137,7 @@ impl<'a> Inspector<'a> { _ => !base_off, }; let Some(id) = id else { continue }; - out.push(Layer { id, name: self.text(d, b"Name").unwrap_or_else(|| "Layer".into()), visible, initial: visible }); + out.push(Layer { id, name: self.field_text(d, b"Name").unwrap_or_else(|| "Layer".into()), visible, initial: visible }); } } diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 68486d8097..c908b241cf 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1161,6 +1161,50 @@ fn repeated_annotations_are_listed_once_within_a_byte_budget() { assert!(notes.iter().map(retained).sum::() <= MAX_ANNOTATION_BYTES); } +/// Blank text is not kept, so only reading it is charged: many comments sharing one large blank +/// string stop being built at the budget instead of decoding it once each. +#[test] +fn comments_sharing_a_blank_string_stop_at_the_read_budget() { + let small = "<< /Subtype /Text /Rect [0 0 1 1] /Contents 7 0 R >> ".repeat(1200); + let pdf = format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Annots [{small}] >> endobj\n\ + 7 0 obj ({}) endobj\ntrailer << /Root 1 0 R >>\n%%EOF", + " ".repeat(64 << 10), + ); + let doc = Document::load_mem(pdf.as_bytes()).expect("loads"); + let page = PageInfo { width: 200.0, height: 200.0, crop: [0.0, 0.0, 200.0, 200.0], rotation: 0, label: String::new() }; + let mut info = DocInfo { pages: vec![page], ..Default::default() }; + Inspector::new(&doc).fill(&mut info); + assert!(!info.annotations.is_empty() && info.annotations.len() < 1200, "{}", info.annotations.len()); +} + +/// Many fields sharing one large tooltip, and one layer listed many times, are read within a byte +/// allowance rather than once each. +#[test] +fn fields_sharing_a_tooltip_and_repeated_layers_stop_at_the_text_budget() { + let fields = (0..1300).map(|i| format!("<< /FT /Tx /T (f{i}) /TU 7 0 R /V 7 0 R >> ")).collect::(); + let pdf = format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [{fields}] >> \ + /OCProperties << /OCGs [{}] >> >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] >> endobj\n\ + 6 0 obj << /Type /OCG /Name (Ink) >> endobj\n\ + 7 0 obj ({}) endobj\ntrailer << /Root 1 0 R >>\n%%EOF", + "6 0 R ".repeat(1000), + "x".repeat(32 << 10), + ); + let doc = Document::load_mem(pdf.as_bytes()).expect("loads"); + let page = PageInfo { width: 200.0, height: 200.0, crop: [0.0, 0.0, 200.0, 200.0], rotation: 0, label: String::new() }; + let mut info = DocInfo { pages: vec![page], ..Default::default() }; + Inspector::new(&doc).fill(&mut info); + let kept: usize = info.fields.iter().map(|f| f.tooltip.as_ref().map_or(0, String::len) + f.value.as_ref().map_or(0, String::len)).sum(); + assert!(kept > 0 && kept <= super::core::MAX_FIELD_TEXT_BYTES, "{kept}"); + assert!(info.fields.iter().any(|f| f.tooltip.is_none()), "text past the allowance is left out"); + assert_eq!(info.layers.len(), 1); +} + /// A link reached through an indirect object that holds only a reference to it is the same /// annotation as the link itself, and so is a page array reached the same way: each is listed once. #[test] @@ -2686,6 +2730,42 @@ fn inspection_keeps_no_private_page_data() { assert_eq!(report(with), report(without), "what inspection reports does not change"); } +/// A field with several widgets reports an action on any of them, not only the first, and so does +/// the document's JavaScript flag. +#[test] +fn a_field_reports_an_action_on_a_later_widget() { + let pdf = "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] >> >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] >> endobj\n\ + 4 0 obj << /FT /Tx /T (f) /Kids [5 0 R 6 0 R] >> endobj\n\ + 5 0 obj << /Subtype /Widget /Rect [0 0 1 1] >> endobj\n\ + 6 0 obj << /Subtype /Widget /Rect [0 0 1 1] /A << /S /JavaScript /JS (app.alert(1)) >> >> endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF"; + let doc = Document::load_mem(pdf.as_bytes()).expect("loads"); + let page = PageInfo { width: 200.0, height: 200.0, crop: [0.0, 0.0, 200.0, 200.0], rotation: 0, label: String::new() }; + let mut info = DocInfo { pages: vec![page], ..Default::default() }; + Inspector::new(&doc).fill(&mut info); + assert!(info.fields.iter().any(|f| f.name == "f" && f.has_actions), "{:?}", info.fields); + assert!(info.has_javascript); +} + +/// A shared descendant first reached at the depth limit (its action-bearing child cut) is read +/// again from a shallower path, so the action is still found. +#[test] +fn an_action_below_a_shared_field_first_met_deep_is_found_from_a_shallower_path() { + // 1 is the root, 2..=32 a chain of 31 fields, 33 the shared field, 34 holds the action, + // 35 a short path to 33. + let mut bodies = vec!["<< /Kids [2 0 R 35 0 R] >>".to_string()]; + for i in 2..=32 { + bodies.push(format!("<< /Kids [{} 0 R] >>", i + 1)); + } + bodies.push("<< /Kids [34 0 R] >>".into()); + bodies.push("<< /A << /S /JavaScript /JS (x) >> >>".into()); + bodies.push("<< /Kids [33 0 R] >>".into()); + let doc = Document::load_mem(&pdf_of(&bodies)).expect("loads"); + assert!(Inspector::new(&doc).has_action(&Object::Reference((1, 0)), 0)); +} + /// A `/Kids` entry that loops back, or names a root field that doesn't name this one as its /// `/Parent`, is not a child (ISO 32000-2 §12.7.3, Table 224; §12.7.4.1, Table 226): the field /// keeps its other kids and is listed with them. diff --git a/crates/sign/src/pdf/listing.rs b/crates/sign/src/pdf/listing.rs index c2a7183db1..2d41477127 100644 --- a/crates/sign/src/pdf/listing.rs +++ b/crates/sign/src/pdf/listing.rs @@ -431,8 +431,12 @@ pub fn list(doc: &Document, bytes: &[u8], trust: &TrustStore) -> Vec Vec { let pages = annot_pages(doc); - let mut out = Vec::new(); + let mut out: Vec = Vec::new(); let mut field_values: HashSet = HashSet::new(); + // Fields that name one signature value are judged once: validation does not depend on the + // field, and repeating it per field would repeat the digest, chain and evidence work. + // The row of `out` that holds each value's verdict: a later field copies it, not a map of copies. + let mut judged: HashMap = HashMap::new(); let (fields, whole) = walk_sig_fields(doc); for f in fields { if let Some(v_ref) = f.dict.get(b"V").and_then(Object::as_ref) { @@ -445,6 +449,13 @@ pub fn list_cached(doc: &Document, bytes: &[u8], trust: &TrustStore, cache: &Dig .filter(|r| r.len() == 4) .map(|r| [r[0].min(r[2]), r[1].min(r[3]), r[0].max(r[2]), r[1].max(r[3])]); let visible = widget.is_some_and(|(_, w)| widget_shown(doc, w)); + // A repeat of a judged value reads nothing of it: the signature dictionary is not copied. + let shared = f.dict.get(b"V").and_then(Object::as_ref).filter(|_| f.dict.get(b"V").is_some_and(|v| doc.resolve(v).as_dict().is_some())); + if let Some(first) = shared.and_then(|r| judged.get(&r)).and_then(|&i| out.get(i)) { + let own = SignatureInfo { field: f.name.clone(), signed: true, page, rect, visible, ..first.clone() }; + out.push(own); + continue; + } let v = f.signature(doc); let mut info = SignatureInfo { field: f.name.clone(), @@ -477,6 +488,9 @@ pub fn list_cached(doc: &Document, bytes: &[u8], trust: &TrustStore, cache: &Dig if let Some(v) = v { validate_into(doc, bytes, trust, &v, &mut info, cache); } + if let Some(r) = shared { + judged.insert(r, out.len()); + } out.push(info); } // Standalone document timestamps (ISO 32000-2 §12.8.2.2) live outside AcroForm fields. diff --git a/crates/sign/src/pdf/validate.rs b/crates/sign/src/pdf/validate.rs index 52bf79351f..d3ad411a62 100644 --- a/crates/sign/src/pdf/validate.rs +++ b/crates/sign/src/pdf/validate.rs @@ -371,7 +371,11 @@ pub(crate) const MAX_DSS_BYTES: usize = 64 << 20; pub(crate) struct DssEvidence<'a> { doc: &'a Document, items: std::vec::IntoIter, - seen: HashSet, + /// Object numbers read: `Document::get` resolves a reference by number, so `5 0 R` and `5 1 R` + /// are one stream. + seen: HashSet, + /// Full references already tried, resolved or not. + tried: HashSet, taken: usize, left: usize, /// A cap stopped the array being read in full. @@ -381,7 +385,7 @@ pub(crate) struct DssEvidence<'a> { impl<'a> DssEvidence<'a> { pub(crate) fn new(doc: &'a Document, dss: &Dict, key: &[u8]) -> Self { let items = dss.get(key).map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default(); - DssEvidence { doc, items: items.into_iter(), seen: HashSet::new(), taken: 0, left: MAX_DSS_BYTES, cut: false } + DssEvidence { doc, items: items.into_iter(), seen: HashSet::new(), tried: HashSet::new(), taken: 0, left: MAX_DSS_BYTES, cut: false } } } @@ -395,11 +399,21 @@ impl Iterator for DssEvidence<'_> { // Repeats are skipped before the caps count anything, so repeating one stream cannot use // them up; the scan of the array itself is bounded by the parser's cap on array length. for item in self.items.by_ref() { - if item.as_ref().is_some_and(|r| !self.seen.insert(r)) { + // Resolved first: a reference to a generation the slot doesn't hold reads as null and + // must not make a later, valid reference to the same object number a repeat. + // A repeat is skipped before anything is resolved: a long chain of repeats costs no + // lookups. The number is recorded only once the reference gave a stream. + // Each full reference is tried once, so one that never reaches a stream (a cycle) costs + // its lookups once, however often it is listed; `seen` keeps object numbers apart so a + // dead generation can't hide a live one. + if item.as_ref().is_some_and(|r| self.seen.contains(&r.num) || !self.tried.insert(r)) { continue; } let o = self.doc.resolve(&item); let Object::Stream(s) = o.as_ref() else { continue }; + if let Some(r) = item.as_ref() { + self.seen.insert(r.num); + } if self.taken >= MAX_DSS_ENTRIES { self.cut = true; return None; diff --git a/crates/sign/src/pdf_discovery_tests.rs b/crates/sign/src/pdf_discovery_tests.rs index 29f115cb07..d3250d62ed 100644 --- a/crates/sign/src/pdf_discovery_tests.rs +++ b/crates/sign/src/pdf_discovery_tests.rs @@ -373,6 +373,19 @@ fn only_grew_compares_huge_arrays_without_quadratic_work() { assert!(start.elapsed() < std::time::Duration::from_secs(10), "{:?}", start.elapsed()); } +/// A reference to a generation the object doesn't have reads as null: it is no repeat of the valid +/// reference to the same object number that follows it. +#[test] +fn dss_evidence_skips_a_dead_reference_without_hiding_the_live_one() { + let mut doc = fixture(&[]); + let live = doc.add(Object::Stream(Stream::flate(Dict::new(), b"evidence"))); + let dead = ObjRef::new(live.num, live.generation + 1); + let mut dss = Dict::new(); + dss.set(b"CRLs".to_vec(), Object::Array(vec![Object::Ref(dead), Object::Ref(live)])); + let read: Vec> = DssEvidence::new(&doc, &dss, b"CRLs").collect(); + assert_eq!(read, vec![b"evidence".to_vec()]); +} + #[test] fn dss_evidence_decodes_each_stream_once_within_its_caps() { // One small stream listed 1000 times, then streams that inflate past the evidence cap. diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index e765941c9a..5d24d05dce 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -325,6 +325,21 @@ fn an_indirect_annots_array_is_judged_by_the_annotations_it_gains() { assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::OTHER]))); } +/// A signature widget taken off the page's inline /Annots array is covered by no permission (the +/// indirect-array case is the test above), and stays a change whatever level the signature has. +#[test] +fn removing_a_widget_from_an_inline_annots_array_is_not_a_comment_change() { + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let page = pdfcraft_cos::ObjRef { num: 3, generation: 0 }; + let signed = pdfcraft_sign::sign(&open(&fixture()), &id, &opts()).unwrap(); + let inline = open(&signed).get(page).as_dict().and_then(|d| d.get(b"Annots").cloned()).unwrap(); + assert!(matches!(inline, Object::Array(ref a) if !a.is_empty()), "the page keeps its widgets inline"); + let stripped = edit_after(&signed, |doc| doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Array(vec![]))).unwrap()); + let s = signatures(&open(&stripped), &stripped, &trust).into_iter().find(|s| s.signed).unwrap(); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::OTHER]))); +} + /// Issue #1328: a signature field whose /V is null, directly or through a reference, is unsigned: /// it is listed as unsigned and can be signed by name. #[test] @@ -3354,6 +3369,38 @@ fn signing_a_field_with_a_null_value_after_another_signature_keeps_the_first_val ); } +/// Fields that name one signature value are listed each, under their own name, with the verdict +/// the value earned once. +#[test] +fn fields_sharing_a_signature_value_are_listed_with_one_verdict() { + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let signed = pdfcraft_sign::sign(&open(&fixture()), &id, &SignOptions { field: Some("Approval".into()), ..opts() }).unwrap(); + let v = open(&signed).get(pdfcraft_cos::ObjRef { num: 6, generation: 0 }).as_dict().and_then(|d| d.get(b"V").cloned()).unwrap(); + let shared = edit_after(&signed, |doc| { + let mut w = pdfcraft_cos::Dict::new(); + w.set(b"Type".to_vec(), Object::name("Annot")); + w.set(b"Subtype".to_vec(), Object::name("Widget")); + w.set(b"FT".to_vec(), Object::name("Sig")); + w.set(b"T".to_vec(), PdfString::text("Twin")); + w.set(b"V".to_vec(), v.clone()); + w.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(0); 4])); + let w = doc.add(Object::Dict(w)); + let root = doc.root().unwrap(); + let af = doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").cloned()).unwrap(); + let af = doc.resolve(&af).as_dict().cloned().unwrap(); + let mut fields = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap(); + fields.push(Object::Ref(w)); + let mut af = af; + af.set(b"Fields".to_vec(), Object::Array(fields)); + doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(af))).unwrap(); + }); + let list = signatures(&open(&shared), &shared, &trust); + let (a, b) = (list.iter().find(|s| s.field == "Approval").unwrap(), list.iter().find(|s| s.field == "Twin").unwrap()); + assert!(a.signed && b.signed); + assert_eq!((a.status, &a.modification, &a.signer, a.signed_len), (b.status, &b.modification, &b.signer, b.signed_len)); +} + #[test] fn the_catalogs_perms_do_not_change_under_no_changes() { // /Perms names the certification signature (DocMDP). Under P=1 nothing may change, so a later diff --git a/crates/ui-egui/tests/flatpak_updates.rs b/crates/ui-egui/tests/flatpak_updates.rs new file mode 100644 index 0000000000..536613f08a --- /dev/null +++ b/crates/ui-egui/tests/flatpak_updates.rs @@ -0,0 +1,27 @@ +//! Release and source Flatpaks must allow release checks and host update helpers. +fn finish_args(manifest: &str) -> Vec<&str> { + manifest + .split("finish-args:") + .nth(1) + .unwrap_or_default() + .lines() + .take_while(|line| line.is_empty() || line.starts_with(' ') || line.starts_with('#')) + .filter_map(|line| line.trim().strip_prefix("- ")) + .map(|arg| arg.trim_matches(['\'', '"'])) + .collect() +} + +#[test] +fn flatpak_update_permissions() { + let manifests = [ + include_str!("../../../packaging/linux/flatpak/ai.storyteller.pdfcraft.bundle.yml"), + include_str!("../../../packaging/linux/flatpak/ai.storyteller.pdfcraft.yml"), + ]; + let args = manifests.map(finish_args); + assert_eq!(args[0], args[1], "source and release permissions must agree"); + for permissions in args { + for required in ["--share=network", "--talk-name=org.freedesktop.Flatpak"] { + assert!(permissions.contains(&required), "missing update permission: {required}"); + } + } +} diff --git a/crates/xfa/src/data.rs b/crates/xfa/src/data.rs index 34be028d89..4e7de4af74 100644 --- a/crates/xfa/src/data.rs +++ b/crates/xfa/src/data.rs @@ -148,13 +148,21 @@ pub enum DataOp { impl DataNode { /// The node at `path`, made (with empty siblings before it) where missing; `None` past /// the index cap. - fn ensure(&mut self, path: &[(String, usize)]) -> Option<&mut DataNode> { + fn ensure(&mut self, path: &[(String, usize)], budget: &mut usize) -> Option<&mut DataNode> { let mut node = self; for (name, idx) in path { if *idx >= MAX_INDEX { return None; } let have = node.children.iter().filter(|c| &c.name == name).count(); + // Every skipped sibling owns a copy of the name: bounded as `Build::at` bounds a write. + // The budget is the whole path's, not each level's: a deep path that is affordable at + // every level taken alone would otherwise allocate its way past the bound. + let need = (*idx + 1).saturating_sub(have).saturating_mul(name.len()); + if need > *budget { + return None; + } + *budget -= need; for _ in have..=*idx { node.children.push(DataNode { name: name.clone(), ..Default::default() }); } @@ -166,9 +174,11 @@ impl DataNode { /// Apply `op` to this tree (the root is the `xfa:data` element), as [`write_data_ops`] /// will to the packet, so scripts that run before it is written see the change. pub fn apply(&mut self, op: &DataOp) { + // One budget for the op's whole path, as `Build::at` bounds a write. + let mut budget = build::MAX_NEW_NAME_BYTES; match op { DataOp::Value { path, text } => { - if let Some(n) = self.ensure(path) { + if let Some(n) = self.ensure(path, &mut budget) { // A multi-select list's picks, one `value` each (see `value_at`), told apart // as the packet writer does: `value` leaves only, or a group holding none. let picks = if n.children.is_empty() { n.group } else { n.children.iter().all(|c| c.name == "value" && c.children.is_empty()) }; @@ -189,7 +199,7 @@ impl DataNode { if let Some(&last) = count.checked_sub(1).as_ref() { let mut path = parent.clone(); path.push((name.clone(), last.min(MAX_INDEX.saturating_sub(1)))); - let _ = self.ensure(&path); + let _ = self.ensure(&path, &mut budget); } } DataOp::Remove(path) => { diff --git a/crates/xfa/src/data/build.rs b/crates/xfa/src/data/build.rs index 98e34f0f49..e90904d60c 100644 --- a/crates/xfa/src/data/build.rs +++ b/crates/xfa/src/data/build.rs @@ -38,7 +38,7 @@ pub(super) const MAX_INDEX: usize = 10_000; /// Most bytes of names the nodes one write creates hold together: every sibling an index /// skips over gets its own copy of the name. -const MAX_NEW_NAME_BYTES: usize = 4 << 20; +pub(super) const MAX_NEW_NAME_BYTES: usize = 4 << 20; /// What one write may still create: nodes and bytes of their names. pub(super) struct NewNodes { diff --git a/crates/xfa/src/data/passwords.rs b/crates/xfa/src/data/passwords.rs index 0f98a3f2b4..896d513f2c 100644 --- a/crates/xfa/src/data/passwords.rs +++ b/crates/xfa/src/data/passwords.rs @@ -1,26 +1,38 @@ use super::import::datasets_packet; use super::merge::Op; -use super::path::{data_path, parse_datasets}; +use super::path::{data_path, parse_datasets_packet}; use super::values::{Want, path_of, text_of, with_template}; use super::write::{datasets_stream_ref, rewrite}; use super::{DataNode, DataPath, Document, MAX_DEPTH, Node, ObjRef, Object, SomIndices, Template, Ui, XfaError}; +/// How deep the AcroForm field tree is walked for password fields: as deep as the form's own +/// field walk (`pdfcraft_forms::fields`), which is what editing and dataset synchronization see. +const MAX_ACRO_DEPTH: usize = 64; + /// The SOM path and default value of every AcroForm text field with the Password flag (bit 14, /// `1 << 13`) in `doc`, for static XFA forms whose password fields set the flag in the AcroForm -/// dictionary. -fn acro_password_fields(doc: &Document) -> Vec<(String, String)> { +/// dictionary. `None` when the walk was cut short at [`MAX_ACRO_DEPTH`]: which branch is cut +/// depends on the order the tree is visited in, so a field the form's own walk reaches may have +/// been missed, and the caller can't tell that no password field is left. +fn acro_password_fields(doc: &Document) -> Option> { const PASSWORD_FLAG: i64 = 1 << 13; - let Some(root) = doc.root() else { return Vec::new() }; + let Some(root) = doc.root() else { return Some(Vec::new()) }; let catalog = doc.get(root); - let Some(acro) = catalog.as_dict().and_then(|c| c.get(b"AcroForm")).map(|a| doc.resolve(a)) else { return Vec::new() }; + let Some(acro) = catalog.as_dict().and_then(|c| c.get(b"AcroForm")).map(|a| doc.resolve(a)) else { return Some(Vec::new()) }; let Some(top) = acro.as_dict().and_then(|a| a.get(b"Fields")).map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()) else { - return Vec::new(); + return Some(Vec::new()); }; struct AcroWalk<'a> { doc: &'a Document, - seen: std::collections::HashSet, + /// Each field reached, with what it was reached with: its parent's name, type, flags and + /// default, which decide what it classifies as. + seen: std::collections::HashMap, + /// Fields past the depth limit, with what they were reached with. One reached again from + /// another root in the same way was classified there; any other is not fully read. + cut: Vec<(ObjRef, Reach)>, out: Vec<(String, String)>, } + type Reach = (String, Option>, i64, Option); impl AcroWalk<'_> { fn walk( &mut self, @@ -31,12 +43,19 @@ fn acro_password_fields(doc: &Document) -> Vec<(String, String)> { inherited_dv: Option<&str>, depth: usize, ) { - if depth > 32 { + if depth > MAX_ACRO_DEPTH { + let reach: Reach = (parent_name.to_string(), inherited_ft.map(<[u8]>::to_vec), inherited_ff, inherited_dv.map(str::to_string)); + self.cut.extend(items.iter().filter_map(Object::as_ref).map(|r| (r, reach.clone()))); return; } for item in items { let Some(r) = item.as_ref() else { continue }; - if !self.seen.insert(r) { + let reach: Reach = (parent_name.to_string(), inherited_ft.map(<[u8]>::to_vec), inherited_ff, inherited_dv.map(str::to_string)); + // The reach a field was classified with stays: a later visit that skips it classified + // nothing, so it must not stand in for the cut-off one. + if let std::collections::hash_map::Entry::Vacant(slot) = self.seen.entry(r) { + slot.insert(reach); + } else { continue; } let obj = self.doc.get(r); @@ -65,20 +84,35 @@ fn acro_password_fields(doc: &Document) -> Vec<(String, String)> { } } } - let mut w = AcroWalk { doc, seen: std::collections::HashSet::new(), out: Vec::new() }; + let mut w = AcroWalk { doc, seen: std::collections::HashMap::new(), cut: Vec::new(), out: Vec::new() }; w.walk(&top, "", None, 0, None, 0); - w.out + // Decided once every root has been visited: a branch cut deep down may be reached shallower + // from a later root, and is then read in full. + let incomplete = w.cut.iter().any(|(r, reach)| w.seen.get(r) != Some(reach)); + (!incomplete).then_some(w.out) } /// The `Want` entries needed to restore password fields in `doc`'s datasets packet to their /// defaults (or empty when they have no default). -fn password_dataset_wants(doc: &Document) -> Vec { +/// +/// The packet is read as the datasets writer reads it ([`parse_datasets_packet`]): a packet the +/// writer can't read holds no value it wrote, so there is nothing typed in it to clear. An error +/// when the password fields can't all be found (a field tree deeper than the field walk reaches): +/// the save is refused rather than written with a typed password left in the data. +fn password_dataset_wants(doc: &Document) -> Result, XfaError> { if datasets_stream_ref(doc).is_none() { - return Vec::new(); + return Ok(Vec::new()); } - let Ok(Some(packet)) = datasets_packet(doc) else { return Vec::new() }; - let Some(data) = parse_datasets(&packet) else { return Vec::new() }; - with_template(doc, None, |tpl| { + // Checked before the packet is read: a packet that can't be parsed leaves nothing to clear, but + // a form whose fields can't all be found is refused whatever the packet holds. + let Some(acro_fields) = acro_password_fields(doc) else { + return Err(XfaError::Malformed( + "the form's fields nest too deeply to find every password field, so typed passwords could not be removed from its XFA data".into(), + )); + }; + let Ok(Some(packet)) = datasets_packet(doc) else { return Ok(Vec::new()) }; + let Some(data) = parse_datasets_packet(&packet) else { return Ok(Vec::new()) }; + Ok(with_template(doc, None, |tpl| { let mut candidates: Vec<(String, DataPath, String)> = Vec::new(); let mut seen = std::collections::HashSet::new(); if let Some(t) = tpl { @@ -153,7 +187,7 @@ fn password_dataset_wants(doc: &Document) -> Vec { } } } - for (som, default) in acro_password_fields(doc) { + for (som, default) in acro_fields { let path = path_of(tpl, &som); if !path.is_empty() && seen.insert(path.clone()) { candidates.push((som, path, default)); @@ -166,27 +200,29 @@ fn password_dataset_wants(doc: &Document) -> Vec { (!cur.is_empty() && cur != default).then_some(Want { som, path, text: default, ensure: false }) }) .collect() - }) + })) } /// Whether any XFA password field (`` or an AcroForm text field with the Password -/// flag) holds a value in `doc`'s datasets packet beyond its default (#3944). +/// flag) holds a value in `doc`'s datasets packet beyond its default (#3944). A form whose +/// password fields can't all be found counts as holding some, so that clearing them +/// ([`clear_password_values`]) reports why the document can't be written without them. pub fn has_password_values(doc: &Document) -> bool { - !password_dataset_wants(doc).is_empty() + !password_dataset_wants(doc).is_ok_and(|w| w.is_empty()) } /// Clear typed password values from `doc`'s XFA datasets packet (restoring a template default /// where the field has one, else emptying the element), replacing the datasets stream in place /// (#3944). Returns how many password fields were cleared in the datasets. pub fn clear_password_values(doc: &mut Document) -> Result { - let wants = password_dataset_wants(doc); + let wants = password_dataset_wants(doc)?; if wants.is_empty() { return Ok(0); } let count = wants.len(); let reuse = datasets_stream_ref(doc); let report = rewrite(doc, &[Op::Values(&wants)], reuse)?; - if !report.written || !password_dataset_wants(doc).is_empty() { + if !report.written || !password_dataset_wants(doc)?.is_empty() { let reason = report.warnings.first().map_or("the XFA datasets password values could not be cleared", String::as_str); return Err(XfaError::Malformed(reason.to_string())); } diff --git a/crates/xfa/src/data/path.rs b/crates/xfa/src/data/path.rs index 23c5e5ba5d..7fdba77071 100644 --- a/crates/xfa/src/data/path.rs +++ b/crates/xfa/src/data/path.rs @@ -24,6 +24,33 @@ pub fn parse_datasets(xdp: &str) -> Option { Some(to_node(data, 0)) } +/// The `xfa:data` element of a datasets packet as the datasets writer reads it before changing it: +/// a packet of an array may use the `xfa` and `xdp` prefixes its XDP envelope declares, so it is +/// read inside an element declaring them when it doesn't parse alone, and the datasets element in +/// the XFA data namespace is preferred over another with the same local name. Reading the packet +/// any other way could miss the values the writer put in it. +pub(super) fn parse_datasets_packet(text: &str) -> Option { + if crate::xml_nesting_depth(text) > crate::MAX_XML_DEPTH { + return None; + } + let opts = || roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: MAX_NODES, ..Default::default() }; + let wrapped; + let doc = match roxmltree::Document::parse_with_options(text, opts()) { + Ok(d) => d, + Err(_) => { + wrapped = format!("{}{text}{}", super::merge::WRAP_OPEN, super::merge::WRAP_CLOSE); + roxmltree::Document::parse_with_options(&wrapped, opts()).ok()? + } + }; + let is = |n: &roxmltree::Node, name: &str| n.is_element() && n.tag_name().name() == name; + let datasets = doc + .descendants() + .find(|n| is(n, "datasets") && n.tag_name().namespace() == Some(super::XFA_DATA_NS)) + .or_else(|| doc.descendants().find(|n| is(n, "datasets")))?; + let data = datasets.children().find(|n| is(n, "data"))?; + Some(to_node(data, 0)) +} + /// The data path a SOM expression names: `form[0].#subform[0].name[1]` → `[(form, 0), (name, 1)]`. /// Unnamed containers (`#subform`) have no data node. pub fn som_to_path(som: &str) -> DataPath { diff --git a/crates/xfa/src/data/write.rs b/crates/xfa/src/data/write.rs index 4667036fe5..554685e1ad 100644 --- a/crates/xfa/src/data/write.rs +++ b/crates/xfa/src/data/write.rs @@ -82,6 +82,11 @@ pub fn remove_data_instance(doc: &mut Document, path: &DataPath) -> Result) let mut run: Vec = Vec::new(); // Instances queued in this run, by parent and name: asking for more only queues the rest. let mut queued: HashMap<(&DataPath, &str), usize> = HashMap::new(); - let (mut ensured, mut capped) = (0usize, false); + let (mut ensured, mut ensured_bytes, mut capped, mut capped_bytes) = (0usize, 0usize, false, false); let flush = |run: &mut Vec, steps: &mut Vec>, removes: &mut Vec>| { if !run.is_empty() { let mut seen = std::collections::HashSet::new(); @@ -117,12 +122,24 @@ pub fn write_data_ops(doc: &mut Document, ops: &[DataOp], reuse: Option) } DataOp::Instances { parent, name, count } => { let done = queued.entry((parent, name.as_str())).or_insert(0); + // The path (its segments' buffers and their text) is held by the queued step, which a + // clone then `push` can leave with up to twice the room, and cloned again into + // `flush`'s dedup set, beside the label and the step itself. + let segment = std::mem::size_of::<(String, usize)>(); + let path_bytes = + parent.iter().fold(segment.saturating_add(name.len()), |n, (p, _)| n.saturating_add(segment).saturating_add(p.len())); + let per_instance = path_bytes.saturating_mul(3).saturating_add(name.len()).saturating_add(16 + std::mem::size_of::()); while *done < (*count).min(MAX_INDEX) { if ensured >= MAX_ENSURED { capped = true; break; } + if ensured_bytes.saturating_add(per_instance) > MAX_ENSURED_BYTES { + capped_bytes = true; + break; + } ensured += 1; + ensured_bytes += per_instance; #[cfg(test)] ENSURED.with(|n| n.set(n.get() + 1)); let k = *done; @@ -154,6 +171,11 @@ pub fn write_data_ops(doc: &mut Document, ops: &[DataOp], reuse: Option) if capped { written.warnings.push(format!("XFA data instances not written past {MAX_ENSURED}: the rows were added and removed too often")); } + if capped_bytes { + written + .warnings + .push(format!("XFA data instances not written past {MAX_ENSURED_BYTES} bytes of names: the rows were added and removed too often")); + } Ok(written) } diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 682887e673..c1abcce190 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -7114,6 +7114,99 @@ fn a_container_is_as_wide_as_a_turned_draw_stands() { } } +/// The static fixture with its text field (`form1[0].page1[0].name[0]`) flagged Password and +/// `levels` unnamed fields between `form1[0]` and `page1[0]`, which add no SOM segment. +fn static_password_doc(levels: usize, data: &str) -> Document { + let mut doc = Document::open(Arc::new(static_shell(data))).unwrap(); + doc.update_dict(pdfcraft_cos::ObjRef::new(8, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1 << 13))).unwrap(); + let mut next = pdfcraft_cos::ObjRef::new(7, 0); + for _ in 0..levels { + let mut d = Dict::new(); + d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(next)])); + next = doc.add(Object::Dict(d)); + } + doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(next)]))).unwrap(); + doc +} + +/// #3944: a Password-flagged AcroForm field of a static XFA form nested deeper than 32 levels (as +/// deep as the form's own field walk reaches) has its typed value cleared from the datasets. +#[test] +fn a_deeply_nested_acroform_password_field_is_cleared_from_the_datasets() { + let data = "hunter2"; + let mut doc = static_password_doc(40, data); + assert!(has_password_values(&doc)); + assert_eq!(clear_password_values(&mut doc).unwrap(), 1); + assert!(!has_password_values(&doc)); + assert!(!String::from_utf8_lossy(&xfa_bytes(&doc)).contains("hunter2")); +} + +/// A field tree deeper than the walk reaches can't be shown to hold no password field: clearing +/// refuses (so the save does) rather than leave a typed password in the datasets. +#[test] +fn a_field_tree_too_deep_to_check_for_passwords_refuses_to_clear() { + let mut doc = static_password_doc(80, "hunter2"); + assert!(has_password_values(&doc)); + assert!(clear_password_values(&mut doc).is_err()); + // A shallow form without a typed value has nothing to clear. + let mut doc = static_password_doc(0, ""); + assert!(!has_password_values(&doc)); + assert_eq!(clear_password_values(&mut doc).unwrap(), 0); +} + +/// A datasets packet of an XFA array may use the `xfa` prefix its preamble declares: the typed +/// password in it is still found and cleared, as the datasets writer reads that packet too. +#[test] +fn a_password_in_a_datasets_packet_using_the_preamble_prefix_is_cleared() { + let mut doc = static_password_doc(0, ""); + let pre = r#""#; + let tpl = r#""#; + let ds = "hunter2"; + let parts: Vec = [("preamble", pre), ("template", tpl), ("datasets", ds), ("postamble", "")] + .into_iter() + .flat_map(|(name, text)| { + let r = doc.add(Object::Stream(pdfcraft_cos::Stream::flate(Dict::new(), text.as_bytes()))); + [Object::String(pdfcraft_cos::PdfString::literal(name.as_bytes().to_vec())), Object::Ref(r)] + }) + .collect(); + let root = doc.root().unwrap(); + doc.update_dict(root, |c| { + let mut acro = c.get(b"AcroForm").and_then(Object::as_dict).cloned().unwrap(); + acro.set(b"XFA".to_vec(), Object::Array(parts)); + c.set(b"AcroForm".to_vec(), Object::Dict(acro)); + }) + .unwrap(); + assert!(crate::data::parse_datasets(ds).is_none(), "the packet doesn't parse alone"); + assert!(has_password_values(&doc)); + assert_eq!(clear_password_values(&mut doc).unwrap(), 1); + assert!(!has_password_values(&doc)); +} + +/// Every sibling a SOM index skips owns a copy of the element name: the in-memory builder refuses +/// a path whose copies would pass the name-byte bound the writer holds itself to, and creates +/// nothing for it. +#[test] +fn the_in_memory_data_builder_bounds_copied_name_bytes() { + let mut root = DataNode::default(); + // 1 KiB x 9,999 skipped siblings is about 10 MB, over the 4 MiB bound. + let long = "n".repeat(1 << 10); + root.apply(&DataOp::Value { path: vec![(long.clone(), 9_999)], text: "x".into() }); + assert!(root.children.is_empty()); + // Within the bound, the same shape still builds (and keeps indexes). + root.apply(&DataOp::Value { path: vec![(long, 100)], text: "x".into() }); + assert_eq!(root.children.len(), 101); +} + +/// Each queued instance owns a copy of its parent path: a long ancestor stops the queue at the +/// byte allowance, reported as capped, instead of being multiplied by the instance count. +#[test] +fn queued_instances_are_bounded_by_the_path_bytes_they_copy() { + let mut doc = Document::new_empty(); + let parent: DataPath = vec![("a".repeat(16 << 20), 0), ("b".repeat(16 << 20), 0)]; + let written = crate::data::write_data_ops(&mut doc, &[DataOp::Instances { parent, name: "row".into(), count: 100 }], None).unwrap(); + assert!(written.warnings.iter().any(|w| w.contains("not written past")), "{:?}", written.warnings); +} + /// A page break splits an empty multi-line field that grows with its text (XFA 3.3 pp. 291–292: /// text splits between lines, and a field's `keep intact` is `none`): it starts where it is, its /// widget runs to the bottom of the content area, and the rest of its box, closed on every side, diff --git a/packaging/linux/flatpak/ai.storyteller.pdfcraft.bundle.yml b/packaging/linux/flatpak/ai.storyteller.pdfcraft.bundle.yml index d781ad6c7c..8d2fb49192 100644 --- a/packaging/linux/flatpak/ai.storyteller.pdfcraft.bundle.yml +++ b/packaging/linux/flatpak/ai.storyteller.pdfcraft.bundle.yml @@ -17,6 +17,8 @@ runtime-version: '25.08' sdk: org.freedesktop.Sdk command: pdfcraft finish-args: + # The shared updater inspects and updates this installation via flatpak-spawn --host. + - --talk-name=org.freedesktop.Flatpak - --filesystem=xdg-data/ArtCraft/versions:ro # Display: Wayland first; X11 only when no Wayland compositor is running. - --socket=wayland diff --git a/packaging/linux/flatpak/ai.storyteller.pdfcraft.yml b/packaging/linux/flatpak/ai.storyteller.pdfcraft.yml index fba62fe382..2c5d6d88c6 100644 --- a/packaging/linux/flatpak/ai.storyteller.pdfcraft.yml +++ b/packaging/linux/flatpak/ai.storyteller.pdfcraft.yml @@ -18,6 +18,8 @@ sdk-extensions: - org.freedesktop.Sdk.Extension.rust-stable command: pdfcraft finish-args: + # The shared updater inspects and updates this installation via flatpak-spawn --host. + - --talk-name=org.freedesktop.Flatpak - --filesystem=xdg-data/ArtCraft/versions:ro # Display: Wayland first; X11 only when no Wayland compositor is running. - --socket=wayland