M6: a dotted name checks the place it goes: an empty parent's name, a parent's value (#2696)

Review on #2957. The field list holds only terminal fields, so a parent
left empty by a move wasn't seen and a second field of its name could be
made beside it, by adding or by renaming. And a new field joining a parent
that holds /V, /DV or /MaxLen inherited them, with no neutral value of its
own to turn them off: such a parent now takes no field that doesn't hold
its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mtlcl410
2026-10-23 10:45:14 +08:00
co-authored by Claude Opus 5.5
parent f41abf921f
commit 224a64e3d0
2 changed files with 103 additions and 12 deletions
+44 -8
View File
@@ -530,22 +530,51 @@ fn link(doc: &mut Document, af: ObjRef, parent: Option<ObjRef>, field: ObjRef) -
Ok(())
}
/// The node named `name` (its own `/T`) directly under `parent`, or at the top of the form.
fn child_named(doc: &Document, af: ObjRef, parent: Option<ObjRef>, name: &str) -> Option<ObjRef> {
let list = match parent {
Some(p) => list_of(doc, p, b"Kids"),
None => list_of(doc, af, b"Fields"),
};
list.iter()
.filter_map(|o| o.as_ref())
.find(|r| doc.get(*r).as_dict().and_then(|d| d.get(b"T").cloned()).and_then(|t| crate::text_of(&doc.resolve(&t))).as_deref() == Some(name))
}
/// The values a parent field passes to the fields under it that can't be turned off by giving
/// the field a neutral one of its own, as `/Ff` and `/Q` can.
const PASSED_VALUES: [&[u8]; 3] = [b"V", b"DV", b"MaxLen"];
/// Put `field` in the field tree under the full name `parts`: its own `/T` is the last part,
/// and the parts before it name parent fields, reused where the tree has them and made where
/// not (Acrobat makes `a.b` a field `b` under a field `a`).
fn attach(doc: &mut Document, af: ObjRef, field: ObjRef, parts: &[&str]) -> Result<(), FormError> {
let Some((own, parents)) = parts.split_last() else { return invalid("field names can't be empty") };
// First check the place, changing nothing. The field list holds only terminal fields, so a
// parent left empty by a move isn't in it: a node of the same name there is found here.
let owned = doc.get(field).as_dict().cloned().unwrap_or_default();
let mut at: Option<ObjRef> = None;
let mut whole_path = true;
for part in parents {
let Some(r) = child_named(doc, af, at, part) else {
whole_path = false;
break;
};
// A value, reset value or length limit held by a parent would pass to this field.
let held = doc.get(r).as_dict().is_some_and(|d| PASSED_VALUES.iter().any(|k| d.contains(k) && !owned.contains(k)));
if held {
return invalid(format!("{part:?} holds a value for the fields under it, so {:?} can't be put there", parts.join(".")));
}
at = Some(r);
}
// `field` is new or already out of the tree, so a node found is another.
if whole_path && child_named(doc, af, at, own).is_some() {
return invalid(format!("a field named {:?} already exists", parts.join(".")));
}
let mut parent: Option<ObjRef> = None;
let mut inherits = false;
for part in parents {
let list = match parent {
Some(p) => list_of(doc, p, b"Kids"),
None => list_of(doc, af, b"Fields"),
};
let found = list.iter().filter_map(|o| o.as_ref()).find(|r| {
doc.get(*r).as_dict().and_then(|d| d.get(b"T").cloned()).and_then(|t| crate::text_of(&doc.resolve(&t))).as_deref() == Some(*part)
});
let next = match found {
let next = match child_named(doc, af, parent, part) {
Some(r) => {
inherits |= doc.get(r).as_dict().is_some_and(|d| d.contains(b"Ff") || d.contains(b"Q"));
r
@@ -1093,6 +1122,13 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S
if let Some(why) = all.iter().filter(|x| x.obj != f.obj).find_map(|x| clash(&new_name, &x.name)) {
return invalid(why);
}
// A node of that name beside it that isn't a terminal field, such as a parent
// field left empty by a move.
let af = ensure_form(doc)?;
let parent = doc.get(f.obj).as_dict().and_then(|d| d.reference(b"Parent"));
if child_named(doc, af, parent, n).is_some_and(|r| r != f.obj) {
return invalid(format!("a field named {new_name:?} already exists"));
}
doc.update_dict(f.obj, |d| d.set(b"T".to_vec(), PdfString::text(n)))?;
}
// Calculations read the field by its new name, unless another field keeps the old one.
+59 -4
View File
@@ -3103,11 +3103,66 @@ fn dotted_field_names_are_made_with_parent_fields() {
let city = field(&fields(&doc), "address.city").clone();
assert!(city.read_only() && city.value == ["Springfield"]);
set_props(&mut doc, "address.city", &FieldProps { name: Some("billing.city".into()), ..FieldProps::default() }).unwrap();
add_field(&mut doc, 0, [300.0, 300.0, 450.0, 320.0], &text, Some("address.street")).unwrap();
let all = fields(&doc);
let moved = field(&all, "billing.city");
let moved = field(&fields(&doc), "billing.city").clone();
assert!(moved.read_only() && moved.value == ["Springfield"], "the moved field keeps what it inherited");
assert!(!field(&all, "address.street").read_only(), "a new field doesn't take its parent's flags");
// A parent's value, reset value or length limit would pass to a new field under it (review
// on #2957), and a field can't turn them off: such a parent takes no new field.
let street = |doc: &mut Document| add_field(doc, 0, [300.0, 300.0, 450.0, 320.0], &text, Some("address.street"));
for (key, value) in [
(&b"V"[..], Object::String(PdfString::literal(b"x".to_vec()))),
(b"DV", Object::String(PdfString::literal(b"x".to_vec()))),
(b"MaxLen", Object::Int(3)),
] {
doc.update_dict(address, |d| {
for k in [&b"V"[..], b"DV", b"MaxLen"] {
d.remove(k);
}
d.set(key.to_vec(), value);
})
.unwrap();
let mut copy = doc.clone();
assert!(street(&mut copy).is_err(), "address holds /{}", String::from_utf8_lossy(key));
}
doc.update_dict(address, |d| {
d.remove(b"MaxLen");
})
.unwrap();
street(&mut doc).unwrap();
let new = field(&fields(&doc), "address.street").clone();
assert!(!new.read_only(), "a new field doesn't take its parent's flags");
assert!(new.value.iter().all(|v| v.is_empty()) && new.max_len.is_none(), "nor a value or length limit: {:?} {:?}", new.value, new.max_len);
// A parent left empty by a move isn't a terminal field, but its name is still taken.
add_field(&mut doc, 0, [300.0, 340.0, 450.0, 360.0], &text, Some("box.only")).unwrap();
set_props(&mut doc, "box.only", &FieldProps { name: Some("elsewhere.only".into()), ..FieldProps::default() }).unwrap();
assert!(fields(&doc).iter().all(|f| !f.name.starts_with("box")), "box holds no field now");
let mut copy = doc.clone();
assert!(add_field(&mut copy, 0, [300.0, 380.0, 450.0, 400.0], &text, Some("box")).is_err(), "a new field named box");
let mut copy = doc.clone();
assert!(set_props(&mut copy, "name", &FieldProps { name: Some("box".into()), ..FieldProps::default() }).is_err(), "a rename to box");
// Under it, the name is free: the empty parent takes a field again.
add_field(&mut doc, 0, [300.0, 380.0, 450.0, 400.0], &text, Some("box.again")).unwrap();
// Only a whole path that exists can be taken: under box.sub, made new, "again" is free.
add_field(&mut doc, 0, [300.0, 420.0, 450.0, 440.0], &text, Some("box.sub.again")).unwrap();
// A field holding its own value may join a parent holding one; and the parents a new path
// makes are new, even where an existing field has the name of a later part.
doc.update_dict(address, |d| d.set(b"V".to_vec(), Object::String(PdfString::literal(b"x".to_vec())))).unwrap();
set_props(&mut doc, "billing.city", &FieldProps { name: Some("address.city".into()), ..FieldProps::default() }).unwrap();
assert_eq!(field(&fields(&doc), "address.city").value, ["Springfield"]);
add_field(&mut doc, 0, [300.0, 460.0, 450.0, 480.0], &text, Some("nope.address.x")).unwrap();
// Renaming the last part: to its own name is no change; to an empty parent's name beside it
// is taken.
assert_eq!(set_props(&mut doc, "address.city", &FieldProps { name: Some("city".into()), ..FieldProps::default() }).unwrap(), "address.city");
doc.update_dict(address, |d| {
d.remove(b"V");
})
.unwrap();
add_field(&mut doc, 0, [300.0, 500.0, 450.0, 520.0], &text, Some("address.grp.x")).unwrap();
set_props(&mut doc, "address.grp.x", &FieldProps { name: Some("elsewhere.x".into()), ..FieldProps::default() }).unwrap();
assert!(
set_props(&mut doc.clone(), "address.city", &FieldProps { name: Some("grp".into()), ..FieldProps::default() }).is_err(),
"address.grp is taken"
);
for bad in ["MASTER", "MASTER.street1.x", "MASTER..x", ".x", "x.", "MASTER. "] {
assert!(add_field(&mut doc, 0, [300.0, 200.0, 450.0, 220.0], &text, Some(bad)).is_err(), "{bad:?}");