mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-10 13:35:21 +00:00
M8: bound the removed text kept for the metadata check per document (#2448)
The run and byte limits were per page, and a glyph's /ToUnicode text can be 256 UTF-16 units, so a short content stream could keep gigabytes of removed text across pages. The document now keeps at most 16 MiB; past that the rest is not kept and the metadata check fails closed, as for any other text it couldn't keep (CodeRabbit's retained concern on #2539). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pq7HyG234xeYkJfr41E6iP
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
4bfea3ade6
commit
dcef0aebff
@@ -49,6 +49,11 @@ const _: () = assert!(MAX_DEPTH >= pdfcraft_edit::text::MAX_FORM_DEPTH);
|
||||
/// Bounds on the removed text collected for the metadata check (#2448).
|
||||
const MAX_RUNS: usize = 100_000;
|
||||
const MAX_RUN_BYTES: usize = 64 * 1024;
|
||||
/// The removed text kept for the whole document, in bytes. A glyph's `/ToUnicode` text has no
|
||||
/// length limit, so a short content stream can make many runs of nearly [`MAX_RUN_BYTES`] each,
|
||||
/// on every page; past this the rest is not kept and the metadata check fails closed, as for
|
||||
/// any other text it couldn't keep (review on #2539).
|
||||
pub(crate) const MAX_REMOVED_BYTES: usize = 16 * 1024 * 1024;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum Mode {
|
||||
@@ -132,6 +137,9 @@ pub(crate) struct Scope<'a> {
|
||||
/// Some removed text was not kept (too many runs, or a run too long): the metadata check
|
||||
/// can't search for it and must fail closed.
|
||||
pub removed_cut: bool,
|
||||
/// Bytes of removed text this scope may still keep: what is left of the document's
|
||||
/// [`MAX_REMOVED_BYTES`] (see [`Scope::with_removed_room`]).
|
||||
pub removed_room: usize,
|
||||
/// The run being collected, and where its last glyph ended (device space) with its height.
|
||||
run: String,
|
||||
run_end: Option<((f64, f64), f64)>,
|
||||
@@ -161,6 +169,7 @@ impl<'a> Scope<'a> {
|
||||
layer_blocks: 0,
|
||||
removed_text: Vec::new(),
|
||||
removed_cut: false,
|
||||
removed_room: MAX_REMOVED_BYTES,
|
||||
run: String::new(),
|
||||
run_end: None,
|
||||
fonts: HashMap::new(),
|
||||
@@ -170,12 +179,20 @@ impl<'a> Scope<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Keep at most `room` bytes of removed text: what earlier pages left of the document's
|
||||
/// [`MAX_REMOVED_BYTES`].
|
||||
pub fn with_removed_room(mut self, room: usize) -> Self {
|
||||
self.removed_room = room;
|
||||
self
|
||||
}
|
||||
|
||||
/// Close the run of removed text being collected.
|
||||
pub fn flush(&mut self) {
|
||||
self.run_end = None;
|
||||
let run = std::mem::take(&mut self.run);
|
||||
if !run.trim().is_empty() {
|
||||
if self.removed_text.len() < MAX_RUNS {
|
||||
if self.removed_text.len() < MAX_RUNS && run.len() <= self.removed_room {
|
||||
self.removed_room -= run.len();
|
||||
self.removed_text.push(run);
|
||||
} else {
|
||||
self.removed_cut = true;
|
||||
|
||||
@@ -438,6 +438,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result<Report, Redac
|
||||
widget_owner,
|
||||
removed_text: Vec::new(),
|
||||
removed_cut: false,
|
||||
removed_room: interp::MAX_REMOVED_BYTES,
|
||||
removed_everywhere: HashSet::new(),
|
||||
};
|
||||
for &(pi, page_marks) in &groups {
|
||||
@@ -572,6 +573,9 @@ struct Applying {
|
||||
removed_text: Vec<String>,
|
||||
/// Some removed text was too long to keep whole for that check.
|
||||
removed_cut: bool,
|
||||
/// Bytes of removed text later pages may still keep (`interp::MAX_REMOVED_BYTES` for the
|
||||
/// document).
|
||||
removed_room: usize,
|
||||
}
|
||||
|
||||
/// Step 1 for one page: rewrites its content so nothing readable stays under a mark, and cleans the tags it no longer shows.
|
||||
@@ -582,17 +586,18 @@ fn redact_page_content(
|
||||
page: &pdfcraft_model::Page,
|
||||
rects: &[[f64; 4]],
|
||||
) -> Result<(), RedactError> {
|
||||
let Applying { report, doomed_xobjects, retired_properties, removed_text, removed_cut, .. } = applying;
|
||||
let Applying { report, doomed_xobjects, retired_properties, removed_text, removed_cut, removed_room, .. } = applying;
|
||||
// 1. Content.
|
||||
let mut budget = pdfcraft_cos::DecodeBudget::page();
|
||||
let (list, data) = page_streams_within(doc, &page.dict, pi, &mut budget)?;
|
||||
let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default();
|
||||
let out = {
|
||||
let mut scope = Scope::new(rects, Mode::Apply, report, budget);
|
||||
let mut scope = Scope::new(rects, Mode::Apply, report, budget).with_removed_room(*removed_room);
|
||||
let out = process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY);
|
||||
scope.flush();
|
||||
removed_text.append(&mut scope.removed_text);
|
||||
*removed_cut |= scope.removed_cut;
|
||||
*removed_room = scope.removed_room;
|
||||
out
|
||||
};
|
||||
// Fail closed: the interpreter lays vertical-writing text out horizontally, so it cannot
|
||||
|
||||
@@ -289,6 +289,55 @@ fn metadata_is_cleared_when_the_removed_text_was_cut_short() {
|
||||
assert!(!has_xmp(&doc));
|
||||
}
|
||||
|
||||
/// #2539 review: the removed text kept for the metadata check is bounded for the whole document,
|
||||
/// not per page. One glyph code can stand for 256 UTF-16 units of `/ToUnicode` text (768 bytes of
|
||||
/// UTF-8), so here each page makes 150 runs of 80 such glyphs (9.2 MB, under the page's run and
|
||||
/// byte limits); two pages pass `MAX_REMOVED_BYTES`, so the rest isn't kept and the check fails
|
||||
/// closed.
|
||||
#[test]
|
||||
fn removed_text_kept_for_the_metadata_check_is_bounded_per_document() {
|
||||
let long = "4E00".repeat(256);
|
||||
let to_unicode = format!("/CIDInit /ProcSet findresource begin 1 beginbfchar <41> <{long}> endbfchar end");
|
||||
let run = "A".repeat(80);
|
||||
let mut content = b"BT /F1 1 Tf 10 100 Td".to_vec();
|
||||
for _ in 0..75 {
|
||||
content.extend_from_slice(format!(" ({run}) Tj 30 0 Td ({run}) Tj -30 0 Td").as_bytes());
|
||||
}
|
||||
content.extend_from_slice(b" ET");
|
||||
let page = "/Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >>";
|
||||
let objs: Vec<Vec<u8>> = vec![
|
||||
b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(),
|
||||
b"<< /Type /Pages /Kids [3 0 R 8 0 R] /Count 2 >>".to_vec(),
|
||||
format!("<< {page} >>").into_bytes(),
|
||||
stream("", &content),
|
||||
FONT.replace("95 0 R", "6 0 R").replace(" /Widths", " /ToUnicode 7 0 R /Widths").into_bytes(),
|
||||
widths(),
|
||||
stream("", to_unicode.as_bytes()),
|
||||
format!("<< {page} >>").into_bytes(),
|
||||
];
|
||||
let mut doc = pdf(objs);
|
||||
let mut applying = Applying {
|
||||
report: Report::default(),
|
||||
doomed_fields: Vec::new(),
|
||||
doomed_xobjects: Vec::new(),
|
||||
retired_properties: Vec::new(),
|
||||
widget_owner: Vec::new(),
|
||||
removed_text: Vec::new(),
|
||||
removed_cut: false,
|
||||
removed_room: interp::MAX_REMOVED_BYTES,
|
||||
removed_everywhere: HashSet::new(),
|
||||
};
|
||||
let rects = [[5.0, 99.0, 200.0, 102.0]];
|
||||
let kept = |a: &Applying| a.removed_text.iter().map(String::len).sum::<usize>();
|
||||
let first = pdfcraft_model::pages(&doc).swap_remove(0);
|
||||
redact_page_content(&mut doc, &mut applying, 0, &first, &rects).unwrap();
|
||||
assert_eq!((kept(&applying), applying.removed_cut), (9_216_000, false), "one page fits");
|
||||
let second = pdfcraft_model::pages(&doc).swap_remove(1);
|
||||
redact_page_content(&mut doc, &mut applying, 1, &second, &rects).unwrap();
|
||||
assert!(applying.removed_cut, "the second page passes the document's limit");
|
||||
assert!(kept(&applying) <= interp::MAX_REMOVED_BYTES, "kept {} bytes", kept(&applying));
|
||||
}
|
||||
|
||||
/// #2539 review: the searching a scrub may do is one budget for all of it, not a check per
|
||||
/// value: very many small `/Info` values each fit, but together they don't, and past the budget
|
||||
/// the rest counts as matching (fail-closed). With room to search, nothing that doesn't mention
|
||||
|
||||
Reference in New Issue
Block a user