diff --git a/crates/doc/src/attrs.rs b/crates/doc/src/attrs.rs index 2389fea94..f5c7feac9 100644 --- a/crates/doc/src/attrs.rs +++ b/crates/doc/src/attrs.rs @@ -563,6 +563,9 @@ pub enum TabAlign { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct TabStop { + /// A file that held a non-finite position (written as `null`) reads it back as NaN, which + /// [`TabStop::sanitized_list`] drops. + #[serde(deserialize_with = "position_or_nan")] pub position: f64, #[serde(default)] pub align: TabAlign, @@ -609,26 +612,93 @@ impl TabStop { /// A tab-stop list as the Tabs commands keep it: at most [`MAX_TAB_STOPS`] stops, positions /// clamped, leaders and align-on characters checked, a char stop aligning on `.` unless it /// names a character, sorted by position, and one stop per position (the later one wins). - pub fn checked_list(tabs: Vec) -> Result, String> { + pub fn checked_list(mut tabs: Vec) -> Result, String> { if tabs.len() > MAX_TAB_STOPS { return Err(format!("a paragraph holds at most {MAX_TAB_STOPS} tab stops")); } - let mut out: Vec = Vec::with_capacity(tabs.len()); - for mut t in tabs { + for t in &mut tabs { t.position = Self::clamp_position(t.position)?; Self::check_leader(&t.leader)?; Self::check_align_on(&t.align_on)?; + } + Ok(Self::normalized(tabs)) + } + + /// A tab-stop list read from a file, repaired to what [`checked_list`](Self::checked_list) + /// accepts: stops with a non-finite position dropped, positions clamped, control characters + /// removed from leaders and align-on characters and both cut to their limits, sorted, one stop + /// per position (the later one wins), and the first [`MAX_TAB_STOPS`] by position kept. + pub fn sanitized_list(tabs: Vec) -> Vec { + let tabs = tabs + .into_iter() + .filter_map(|mut t| { + t.position = Self::clamp_position(t.position).ok()?; + if Self::check_leader(&t.leader).is_err() { + t.leader = t.leader.chars().filter(|c| !c.is_control()).take(MAX_TAB_LEADER).collect(); + } + if Self::check_align_on(&t.align_on).is_err() { + t.align_on = t.align_on.chars().filter(|c| !c.is_control()).take(1).collect(); + } + Some(t) + }) + .collect(); + let mut out = Self::normalized(tabs); + out.truncate(MAX_TAB_STOPS); + out + } + + /// Finite, clamped stops sorted by position, one per position: of stops closer than + /// [`SAME_TAB_POSITION`], the later one in `tabs` wins. A char stop with no character aligns + /// on `.`. + fn normalized(tabs: Vec) -> Vec { + let mut stops: Vec<(usize, TabStop)> = tabs.into_iter().enumerate().collect(); + stops.sort_by(|a, b| a.1.position.total_cmp(&b.1.position)); + let mut out: Vec<(usize, TabStop)> = Vec::with_capacity(stops.len()); + for (i, mut t) in stops { if t.align == TabAlign::Char && t.align_on.is_empty() { t.align_on = ".".into(); } - out.retain(|o| (o.position - t.position).abs() >= SAME_TAB_POSITION); - out.push(t); + match out.last_mut() { + Some(last) if t.position - last.1.position < SAME_TAB_POSITION => { + if i > last.0 { + *last = (i, t); + } + } + _ => out.push((i, t)), + } } - out.sort_by(|a, b| a.position.total_cmp(&b.position)); - Ok(out) + out.into_iter().map(|(_, t)| t).collect() } } +/// A stop's position; `null` (how JSON writes a non-finite number) reads as NaN. +fn position_or_nan<'de, D: serde::Deserializer<'de>>(d: D) -> Result { + Ok(Option::::deserialize(d)?.unwrap_or(f64::NAN)) +} + +/// A paragraph's tab-stop list in either attribute form, as read from a file. +trait FileTabs: Sized { + fn sanitized(self) -> Self; +} + +impl FileTabs for Vec { + fn sanitized(self) -> Self { + TabStop::sanitized_list(self) + } +} + +impl FileTabs for Option> { + fn sanitized(self) -> Self { + self.map(TabStop::sanitized_list) + } +} + +/// Deserializes a `tabs` field through [`TabStop::sanitized_list`]: a saved document's tab lists +/// get the limits the Tabs commands keep. +fn sanitized_tabs<'de, D: serde::Deserializer<'de>, T: Deserialize<'de> + FileTabs>(d: D) -> Result { + T::deserialize(d).map(FileTabs::sanitized) +} + /// A paragraph rule (Rule Above / Rule Below). #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(default, rename_all = "camelCase")] @@ -1110,6 +1180,7 @@ attr_set! { split_outside_gutter: f64 = 0.0, rule_above: Rule = Rule::default(), rule_below: Rule = Rule::default(), + #[serde(deserialize_with = "sanitized_tabs")] tabs: Vec = Vec::new(), list_type: ListType = ListType::None, /// Named list (Define Lists) the numbers belong to ("" = the story's own list). @@ -1479,6 +1550,36 @@ mod tests { assert_eq!(list.iter().map(|t| t.position).collect::>(), [10.0, 50.001], "sorted, one stop per position"); } + /// A file's tab list is repaired, never refused; a list the commands accept comes through + /// unchanged. + #[test] + fn tab_lists_from_files_are_sanitized() { + let stop = |position: f64| TabStop { position, align: TabAlign::Left, leader: String::new(), align_on: String::new() }; + let mut tabs: Vec = (0..500).rev().map(|i| stop(i as f64 * 10.0)).collect(); + tabs.push(stop(f64::NAN)); + tabs.push(stop(f64::NEG_INFINITY)); + tabs.push(TabStop { leader: "\u{7}0123456789".into(), align: TabAlign::Char, align_on: "\n,;".into(), ..stop(-3.0) }); + let list = TabStop::sanitized_list(tabs); + assert_eq!(list.len(), MAX_TAB_STOPS); + assert!(list.windows(2).all(|w| w[1].position - w[0].position >= SAME_TAB_POSITION), "sorted, one stop per position"); + let first = &list[0]; + assert_eq!((first.position, first.leader.as_str(), first.align_on.as_str()), (0.0, "01234567", ","), "the later stop at 0 wins"); + assert_eq!(list.last().map(|t| t.position), Some(990.0), "the first stops by position are kept"); + + let ok = TabStop::checked_list(vec![stop(72.0), TabStop { align: TabAlign::Char, ..stop(10.0) }]).unwrap(); + assert_eq!(TabStop::sanitized_list(ok.clone()), ok); + + // `null` is how JSON writes a non-finite position; that stop is dropped, the others kept. + let json = r#"{"tabs":[{"position":null},{"position":20000,"leader":"0123456789"},{"position":5}]}"#; + let a: ParaAttrs = serde_json::from_str(json).unwrap(); + let tabs = a.tabs.unwrap(); + assert_eq!(tabs.iter().map(|t| t.position).collect::>(), [5.0, MAX_TAB_POSITION]); + assert_eq!(tabs[1].leader, "01234567"); + let p: ParaProps = serde_json::from_str(json).unwrap(); + assert_eq!(p.tabs, tabs); + assert!(serde_json::from_str::("{}").is_err(), "a stop names its position"); + } + #[test] fn set_json_by_name() { let mut a = ParaAttrs::default(); diff --git a/crates/engine/src/cmd/tabs.rs b/crates/engine/src/cmd/tabs.rs index 2d69c656e..ceb9422c2 100644 --- a/crates/engine/src/cmd/tabs.rs +++ b/crates/engine/src/cmd/tabs.rs @@ -245,7 +245,8 @@ struct Measure { width: f64, } -/// The stops and indents of the first selected paragraph, or of style `style`. +/// The stops ([`TabStop::sanitized_list`]) and indents of the first selected paragraph, or of +/// style `style`. fn current(s: &Session, style: Option<&str>, c: &str) -> Result<(Vec, Measure)> { let st = s.doc()?; let props = match style { @@ -263,8 +264,8 @@ fn current(s: &Session, style: Option<&str>, c: &str) -> Result<(Vec, M } }; let width = if style.is_none() { column_of(s).map(|(_, r)| r.width()) } else { None }.unwrap_or_else(|| page_column_width(s)); - let mut tabs = props.tabs; - sort(&mut tabs); + // A list that never went through the checks (one set by code) reads as a file's would. + let tabs = TabStop::sanitized_list(props.tabs); Ok((tabs, Measure { left_indent: props.left_indent, first_line_indent: props.first_line_indent, right_indent: props.right_indent, width })) } @@ -312,6 +313,7 @@ fn get(s: &mut Session, p: &Value) -> Result { } /// Apply `op` to the current stops and write the list to the selected paragraphs or the style. +/// Returns the list as stored ([`TabStop::checked_list`]) and the index of `op`'s stop in it. fn edit( s: &mut Session, p: &Value, @@ -321,6 +323,10 @@ fn edit( let style = str_param(p, "style").map(str::to_string); let (mut tabs, m) = current(s, style.as_deref(), c)?; let index = op(&mut tabs, m).map_err(|e| bad(c, e))?; + let at = index.and_then(|i| tabs.get(i)).map(|t| t.position); + let tabs = TabStop::checked_list(tabs).map_err(|e| bad(c, e))?; + // Checking can drop stops that share a position, which moves the indices after them. + let index = at.and_then(|x| tabs.iter().position(|t| (t.position - x).abs() < SAME)); let list = serde_json::to_value(&tabs).map_err(|e| bad(c, e.to_string()))?; match style { Some(name) => s.edit(|d, _| { @@ -447,6 +453,26 @@ mod tests { assert_eq!(at, [(50.0, TabAlign::Char, "."), (72.001, TabAlign::Right, ""), (120.0, TabAlign::Left, "")]); } + /// A command returns the list as it stores it, though the list it started from was never + /// checked. + #[test] + fn tab_commands_return_the_stored_list() { + let (mut s, _) = session("x"); + s.execute("style.paragraph.create", &json!({"name": "Old"})).unwrap(); + let stop = |position, align| TabStop { position, align, leader: String::new(), align_on: String::new() }; + s.edit(|d, _| { + let st = d.styles_mut().para_mut("Old").unwrap(); + st.para.tabs = Some(vec![stop(50.0, TabAlign::Char), stop(72.0, TabAlign::Left), stop(72.001, TabAlign::Right)]); + Ok(()) + }) + .unwrap(); + let out = s.execute("type.tabs.add", &json!({"style": "Old", "position": 120})).unwrap(); + let d = s.doc().unwrap().doc.clone(); + let stored = d.styles.para("Old").unwrap().para.tabs.clone().unwrap(); + assert_eq!(out["tabs"], serde_json::to_value(&stored).unwrap()); + assert_eq!(out["index"], 2, "the added stop, in the stored list"); + } + /// `type.para` and the paragraph style commands take a whole `tabs` list; it gets the checks /// the Tabs commands apply. #[test] diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 073e1b4ed..7175c4376 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -452,6 +452,27 @@ mod tests { assert!(load(&serde_json::to_vec(&v).unwrap()).unwrap().settings.glyph_fallback); } + /// A saved tab list over the limits loads repaired, as an imported one does: a non-finite + /// position (saved as `null`) dropped, positions clamped, leaders cut, the count capped. + #[test] + fn saved_tab_lists_load_sanitized() { + use designcraft_doc::{MAX_TAB_LEADER, MAX_TAB_POSITION, MAX_TAB_STOPS, TabAlign, TabStop}; + let mut d = Document::new(&NewDocument::default()); + let lid = d.default_layer(); + let (_, sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(72.0, 72.0, 300.0, 300.0), lid, "a\tb", ParaFormat::default()).unwrap(); + let stop = |position: f64| TabStop { position, align: TabAlign::Right, leader: "0123456789".into(), align_on: String::new() }; + let mut tabs: Vec = (0..500).rev().map(|i| stop(i as f64 * 40.0)).collect(); + tabs.push(stop(f64::NAN)); + d.story_mut(sid).unwrap().paras[0].para.tabs = Some(tabs); + let bytes = save(&d).unwrap(); + let back = load(&bytes).unwrap(); + let tabs = back.stories[&sid].paras[0].para.tabs.clone().unwrap(); + assert_eq!(tabs.len(), MAX_TAB_STOPS); + assert!(tabs.windows(2).all(|w| w[0].position < w[1].position), "sorted"); + assert!(tabs.iter().all(|t| t.position.is_finite() && t.position <= MAX_TAB_POSITION && t.leader.chars().count() <= MAX_TAB_LEADER)); + assert_eq!(tabs.last().map(|t| t.position), Some(3960.0), "the first stops by position"); + } + #[test] fn rejects_garbage_and_newer_versions() { assert!(load(b"nope").is_err()); diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 88f668558..4c78c5a0e 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -1593,7 +1593,7 @@ impl<'r> Importer<'r> { align_on: get("AlignmentCharacter").filter(|c| c != ".").unwrap_or_default(), }); } - a.tabs = Some(tabs); + a.tabs = Some(TabStop::sanitized_list(tabs)); } a.list_type = e.prop("BulletsAndNumberingListType").map(|v| match v.trim() { "BulletList" => ListType::Bullets, diff --git a/crates/idml/src/tests.rs b/crates/idml/src/tests.rs index 36e3c5203..06e0f7237 100644 --- a/crates/idml/src/tests.rs +++ b/crates/idml/src/tests.rs @@ -2606,3 +2606,30 @@ fn ruby_settings_import_from_idml_and_round_trip() { ruby_overprint_stroke ); } + +/// A file's tab list is sanitized as the Tabs commands check theirs: a hostile one imports +/// within the limits instead of failing the import. +#[test] +fn hostile_tab_lists_import_within_the_limits() { + let item = |position: &str, leader: &str| { + format!( + r#"LeftAlign.{leader}{position}"# + ) + }; + let mut items: String = (0..500).rev().map(|i| item(&(i * 3).to_string(), "")).collect(); + items.push_str(&item("NaN", "")); + items.push_str(&item("1e300", "0123456789")); + let story = format!( + r#" + + {items} + Many tabs + "# + ); + let doc = import_idml(&fixture_with_story(&story)).unwrap(); + let tabs = doc.stories.values().find(|s| s.text.contains("Many tabs")).unwrap().paras[0].para.tabs.clone().unwrap(); + assert_eq!(tabs.len(), designcraft_doc::MAX_TAB_STOPS); + assert!(tabs.iter().all(|t| t.position.is_finite() && t.leader.chars().count() <= designcraft_doc::MAX_TAB_LEADER)); + assert!(tabs.windows(2).all(|w| w[0].position < w[1].position), "sorted"); + assert_eq!((tabs[0].position, tabs[99].position), (0.0, 297.0), "the first stops by position"); +}