fix(render): charge inspection text (fields, tooltips, layers, bookmarks, blank comments) against shared allowances (#6015)

* fix(render): charge field, tooltip, layer, bookmark and blank-comment text against shared allowances

* address review: charge comment text once, keep JS and field-action flags past the budgets

* review: count an unlisted field's actions through its whole subtree

* review: mark multi-select values the allowance dropped

* review: count only real actions, walk each field once, keep name selections

* address review: revisit shared field descendants from shallower paths, charge value separators, check every widget for actions

* merge main, keep both
This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-11 15:50:02 -05:00
1 parent daec898d7c
commit 77078864f4
5 files changed
+289 -37

No files matched your search

+140 -29
View File
@@ -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<ObjectId>) {
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<ObjectId, u32>) -> 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<Object> = 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<Object>, Vec<Object>) =
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::<Vec<_>>()
.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
+62 -6
View File
@@ -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<HashMap<ObjectId, usize>>,
/// 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<usize>,
/// 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<bool>,
/// 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<HashMap<ObjectId, ObjectId>>,
}
/// 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<String> {
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<String> {
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<String> {
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<Option<String>, ()> {
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<Option<String>, ()> {
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<String> {
+1 -1
View File
@@ -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,
+6 -1
View File
@@ -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 });
}
}
+80
View File
@@ -1161,6 +1161,50 @@ fn repeated_annotations_are_listed_once_within_a_byte_budget() {
assert!(notes.iter().map(retained).sum::<usize>() <= 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::<String>();
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.