diff --git a/crates/compose/src/grep.rs b/crates/compose/src/grep.rs index b387384f9..5a56920dc 100644 --- a/crates/compose/src/grep.rs +++ b/crates/compose/src/grep.rs @@ -432,15 +432,36 @@ impl Grep { Ok((out, more)) } - /// Matches in `text` and their replacements, `template` expanded per match (`$1`…; see - /// [`translate_replacement`]). Stops after the first match when `first_only`. - pub fn replacements(&self, text: &str, template: &str, first_only: bool) -> Result, String)>, GrepError> { + /// Matches in `text` that `keep` accepts and their replacements, `template` expanded per match + /// (`$1`…; see [`translate_replacement`]). Stops after the first kept match when `first_only`, + /// and with an error once the expanded replacements pass `max_bytes` in all (before any more + /// are made). + pub fn replacements( + &self, + text: &str, + template: &str, + first_only: bool, + max_bytes: usize, + keep: impl Fn(&Range) -> bool, + ) -> Result, String)>, GrepError> { let exp = fancy_regex::Expander::default(); let mut out = Vec::new(); + let mut bytes = 0usize; for caps in self.re.captures_iter(text) { let caps = caps.map_err(run_err)?; let Some(m) = caps.get(0) else { continue }; - out.push((m.range(), exp.expansion(template, &caps))); + if !keep(&m.range()) { + continue; + } + // Written into a buffer that refuses bytes past the budget, so an oversized + // expansion is rejected before it is allocated. + let mut w = BoundedBuf { buf: Vec::new(), left: max_bytes.saturating_sub(bytes) }; + if exp.write_expansion(&mut w, template, &caps).is_err() { + return Err(err(format!("the replacements would put more than {max_bytes} bytes in"))); + } + bytes += w.buf.len(); + let expanded = String::from_utf8(w.buf).map_err(|_| err("the replacement is not valid UTF-8".to_string()))?; + out.push((m.range(), expanded)); if first_only { break; } @@ -449,6 +470,27 @@ impl Grep { } } +/// A byte sink that fails a write that would pass its remaining budget (nothing is copied then). +struct BoundedBuf { + buf: Vec, + left: usize, +} + +impl std::io::Write for BoundedBuf { + fn write(&mut self, data: &[u8]) -> std::io::Result { + if data.len() > self.left { + return Err(std::io::ErrorKind::OutOfMemory.into()); + } + self.left -= data.len(); + self.buf.extend_from_slice(data); + Ok(data.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + fn run_err(e: fancy_regex::Error) -> GrepError { err(format!("GREP search stopped: {e}")) } @@ -520,7 +562,13 @@ mod tests { assert_eq!(g.find_all("axb A.B a.b", 1).unwrap().first(), Some(&(4..7))); let g = grep(r"(\d+)\h(\l+)"); let tpl = translate_replacement(r"$2~S$1x\r\$$"); - assert_eq!(g.replacements("5 kg", &tpl, false).unwrap(), [(0..4, "kg\u{A0}5x\n$$".to_string())]); + assert_eq!(g.replacements("5 kg", &tpl, false, usize::MAX, |_| true).unwrap(), [(0..4, "kg\u{A0}5x\n$$".to_string())]); + // The total expanded size is bounded while the replacements are made, over the matches kept. + let g = grep("a"); + assert!(g.replacements("aaaa", "xxxx", false, 15, |_| true).is_err()); + assert_eq!(g.replacements("aaaa", "xxxx", false, 16, |_| true).unwrap().len(), 4); + assert_eq!(g.replacements("aaaa", "xxxx", false, 8, |m| m.start < 2).unwrap().len(), 2); + assert_eq!(g.replacements("aaaa", "xxxx", true, 4, |m| m.start >= 2).unwrap(), [(2..3, "xxxx".to_string())]); } #[test] diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index d9ee6d3e9..4ebcd665b 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -251,6 +251,52 @@ impl Document { pub fn conditions_hide(&self, conds: &[String]) -> bool { !conds.is_empty() && conds.iter().all(|c| self.conditions.iter().any(|x| x.name == *c && !x.visible)) } + + /// Is text with character format `f`, in a paragraph whose base character attributes are + /// `base` (`Styles::resolve_para`), left off the page: hidden by its effective conditions + /// (the paragraph's and character style's included) or marked deleted? Exports leave it out + /// as composition does. + pub fn hides_run(&self, base: &CharProps, f: &CharFormat) -> bool { + let props = self.styles.resolve_char(base, f); + props.change == ChangeMark::Deleted || self.conditions_hide(&props.conditions) + } + + /// The byte ranges of `st` that show, in order: the runs [`Self::hides_run`] hides left out. + /// Paragraphs and runs are walked together, once; take it once per story. + pub fn visible_spans(&self, st: &Story) -> Vec> { + let mut out: Vec> = Vec::new(); + let runs: Vec<(std::ops::Range, &CharFormat)> = st.runs().collect(); + let mut k = 0; + for (pi, pr) in st.para_ranges().iter().enumerate() { + let base = st.paras.get(pi).map(|pf| self.styles.resolve_para(pf).1).unwrap_or_default(); + while runs.get(k).is_some_and(|(rr, _)| rr.end <= pr.start) { + k += 1; + } + for (rr, f) in runs.iter().skip(k).take_while(|(rr, _)| rr.start < pr.end) { + let (a, b) = (rr.start.max(pr.start), rr.end.min(pr.end)); + if a >= b || self.hides_run(&base, f) { + continue; + } + match out.last_mut() { + Some(last) if last.end == a => last.end = b, + _ => out.push(a..b), + } + } + } + out + } + + /// The text of `st` within `range` that lies in `spans` ([`Self::visible_spans`] of `st`). + pub fn visible_text(st: &Story, spans: &[std::ops::Range], range: std::ops::Range) -> String { + let first = spans.partition_point(|s| s.end <= range.start); + let mut out = String::new(); + for s in spans.iter().skip(first).take_while(|s| s.start < range.end) { + if let Some(t) = st.text.get(s.start.max(range.start)..s.end.min(range.end)) { + out.push_str(t); + } + } + out + } } /// Ink Manager settings for output, by swatch name. Output applies them through a diff --git a/crates/engine/src/cmd/book.rs b/crates/engine/src/cmd/book.rs index a4217f825..29f95741d 100644 --- a/crates/engine/src/cmd/book.rs +++ b/crates/engine/src/cmd/book.rs @@ -1,6 +1,8 @@ //! Books (File › New / Open Book): an ordered list of documents with continuous page numbers, //! styles synchronised from a style source, and one PDF for the whole book. Saved as `.dcbook`. +use std::collections::{HashMap, HashSet}; + use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; @@ -177,6 +179,15 @@ fn paginate(s: &Session) -> Result { Ok(json!({"pages": next - 1, "chapters": chapters})) } +/// `list` with only the last entry of each name, in the order of those last entries. +#[cfg(not(target_arch = "wasm32"))] +fn keep_last(list: &[T], name: impl Fn(&T) -> &str) -> Vec<&T> { + let mut seen = HashSet::new(); + let mut out: Vec<&T> = list.iter().rev().filter(|x| seen.insert(name(x))).collect(); + out.reverse(); + out +} + #[cfg(not(target_arch = "wasm32"))] fn sync_styles(s: &Session) -> Result { let b = s.book.as_ref().ok_or_else(no_book)?; @@ -187,6 +198,11 @@ fn sync_styles(s: &Session) -> Result { let styles = src.styles.clone(); let swatches = src.swatches.clone(); drop(src); + // A name the source defines twice keeps its last definition, at its last position. + let paragraph = keep_last(&styles.paragraph, |x| x.name.as_str()); + let character = keep_last(&styles.character, |x| x.name.as_str()); + let (paragraph_names, character_names): (HashSet<&str>, HashSet<&str>) = + (paragraph.iter().map(|x| x.name.as_str()).collect(), character.iter().map(|x| x.name.as_str()).collect()); let mut n = 0; for (i, path) in paths.iter().enumerate() { if i == b.style_source { @@ -195,21 +211,27 @@ fn sync_styles(s: &Session) -> Result { writable_chapter(s, path, "book.syncStyles")?; let mut d = load(path)?; { + // The source's styles replace the chapter's of the same names and go last, in the + // source's order (one pass over each list, not one per style). let st = d.styles_mut(); - for ps in &styles.paragraph { - st.paragraph.retain(|x| x.name != ps.name); - st.paragraph.push(ps.clone()); - } - for cs in &styles.character { - st.character.retain(|x| x.name != cs.name); - st.character.push(cs.clone()); - } + st.paragraph.retain(|x| !paragraph_names.contains(x.name.as_str())); + st.paragraph.extend(paragraph.iter().map(|x| (*x).clone())); + st.character.retain(|x| !character_names.contains(x.name.as_str())); + st.character.extend(character.iter().map(|x| (*x).clone())); + } + // First index per name, as Document::swatch resolves colours. + let mut at: HashMap = HashMap::new(); + for (i, w) in d.swatches.iter().enumerate() { + at.entry(w.name.clone()).or_insert(i); } for sw in &swatches { - match d.swatches.iter_mut().find(|w| w.name == sw.name) { + match at.get(&sw.name).and_then(|i| d.swatches.get_mut(*i)) { Some(w) if !w.locked => *w = sw.clone(), Some(_) => {} - None => d.swatches.push(sw.clone()), + None => { + at.insert(sw.name.clone(), d.swatches.len()); + d.swatches.push(sw.clone()); + } } } batch.stage(path, &d)?; @@ -431,4 +453,13 @@ mod tests { assert_eq!(t.execute("book.open", &json!({"path": p("book.dcbook")})).unwrap()["documents"], 2); std::fs::remove_dir_all(&dir).ok(); } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn a_source_style_defined_twice_keeps_its_last_definition() { + let list = [("a", 1), ("b", 2), ("a", 3), ("c", 4)]; + let kept: Vec<_> = super::keep_last(&list, |x| x.0).into_iter().copied().collect(); + // As the one-style-at-a-time loop left them: each name once, at its last position. + assert_eq!(kept, [("b", 2), ("a", 3), ("c", 4)]); + } } diff --git a/crates/engine/src/cmd/datamerge.rs b/crates/engine/src/cmd/datamerge.rs index 411fd545c..213b3d1cb 100644 --- a/crates/engine/src/cmd/datamerge.rs +++ b/crates/engine/src/cmd/datamerge.rs @@ -58,7 +58,7 @@ pub fn specs() -> Vec { "Insert Field", [], None, - "{field, role?: text|image|qr|hyperlink, story?, at?, end?, item?} → {id}; an image field at a text insertion point merges as an inline image", + "{field, role?: text|image|qr|hyperlink, story?, at?, end?, item?} → {id}; an image field at a text insertion point merges as an inline image; with the insertion point in a table cell or a note → {cell: true, marker} (a typed marker, not an anchored placeholder)", has_doc, placeholder_add ), @@ -470,7 +470,7 @@ fn overset_count(doc: &Document) -> usize { .count() } -fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> Result { +fn add_placeholder(d: &mut Document, sel: &mut Selection, p: &Value, name: &str) -> Result { let (source_id, field) = { let source = d.data_merge.sources.first().ok_or_else(|| bad("data.placeholder.add", "select a data source"))?; let field = @@ -527,6 +527,42 @@ fn add_placeholder(d: &mut Document, sel: &Selection, p: &Value, name: &str) -> if role == PlaceholderRole::Qr { return Err(bad("data.placeholder.add", "qr placeholders bind to a frame")); } + // The insertion point is in a table cell or a footnote: the field goes into that text as + // its marker, which the merge fills as it fills a typed one. Such a text is a story of its + // own that a placeholder can't name, and the selection's offsets are its own, so they used + // to put the marker in the table's story instead (#2315). + if !explicit_story + && let Some(t) = sel.text + && let Some(pressed) = t.cell + { + if role != PlaceholderRole::Text { + return Err(bad("data.placeholder.add", "a hyperlink field can't go in a table cell or a note; insert it as text")); + } + // The merge finds a marker by its name, so a name that can't be read back out of one + // would stay a marker: say so rather than insert it. + if name.is_empty() || name.contains(['<', '>', '\n']) { + return Err(bad( + "data.placeholder.add", + format!("the field \"{name}\" can't be inserted in a table cell or a note: its name holds < > or a line break"), + )); + } + // A position a merged cell covers is the merged cell's: the field goes where it shows. + let cell = d.story(t.story).and_then(|st| st.tables.get(&pressed.table)).map_or(pressed, |tab| { + let (row, col) = tab.owner(pressed.row, pressed.col); + designcraft_doc::CellAddr { row, col, ..pressed } + }); + let marker = format!("<<{name}>>"); + let st = d.text_story_mut(t.story, Some(cell)).ok_or_else(|| bad("data.placeholder.add", "no table cell there"))?; + let at = if cell == pressed { t.range().start.min(st.text.len()) } else { st.text.len() }; + let at = designcraft_doc::floor_char_boundary(&st.text, at); + st.replace(at..at, &marker); + // The caret goes after the field, in that text, as after typing; a cell selection + // would have the next keystroke replace the whole cell, field and all. + let end = at + marker.len(); + sel.cells = None; + sel.text = Some(designcraft_doc::TextSel { anchor: end, focus: end, cell: Some(cell), ..t }); + return Ok(json!({"cell": true, "marker": marker})); + } let (story, at, end) = text_target(p, sel).ok_or_else(|| bad("data.placeholder.add", "select text or a frame"))?; if d.story(story).is_none() { return Err(bad("data.placeholder.add", "no story")); diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index ebe950fb6..9d94da7d9 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -245,6 +245,7 @@ pub fn fill_row( reps.push(Rep { story, start, end, text, object }); } apply_reps(doc, &reps); + fill_nested_markers(doc, fields, row, maps); for (anchor, dest, field) in links { let anchor = match anchor { PlaceholderAnchor::Text { story, start, end } => { @@ -354,6 +355,75 @@ fn typed_reps( } } +/// How deep the merge follows texts inside texts (a table in a cell of a table, a note in a cell). +/// Documents don't come anywhere near it; a file that goes deeper is filled down to here instead of +/// exhausting the stack. +pub(super) const MAX_NESTED_DEPTH: usize = 32; + +/// The `<>` markers in the table cells, footnotes and endnotes of the stories being filled. +/// Those texts are stories inside their story, which no placeholder can name and `typed_reps` +/// doesn't read, so a field in a table was left as its marker (#2315). A story, table or note is +/// copied only when something inside it holds a marker of a known field. +fn fill_nested_markers(doc: &mut designcraft_doc::Document, fields: &[DataField], row: &[String], maps: &Maps) { + let mut mapped: Vec = maps.stories.values().copied().collect(); + mapped.sort(); + mapped.dedup(); + for sid in mapped { + if !doc.story(sid).is_some_and(|st| inner_has(st, fields, 0)) { + continue; + } + let Some(st) = doc.story_mut(sid) else { continue }; + fill_inner(st, fields, row, 0); + } +} + +/// Whether `st`'s own text holds a marker of a known field. +fn own_has(st: &Story, fields: &[DataField]) -> bool { + find_markers(&st.text).iter().any(|(_, _, name)| fields.iter().any(|f| f.name == *name)) +} + +/// Whether a text inside `st` (a table cell, footnote or endnote, or one inside those), not `st`'s +/// own text, holds a marker of a known field. +fn inner_has(st: &Story, fields: &[DataField], depth: usize) -> bool { + if depth >= MAX_NESTED_DEPTH { + return false; + } + let holds = |t: &Story| own_has(t, fields) || inner_has(t, fields, depth + 1); + st.tables.values().any(|t| t.cells.iter().any(|c| holds(&c.text))) || st.notes.iter().chain(&st.endnotes).any(|n| holds(&n.text)) +} + +/// Fill the known fields' markers in the texts inside `st`, not in its own text. +fn fill_inner(st: &mut Story, fields: &[DataField], row: &[String], depth: usize) { + if depth >= MAX_NESTED_DEPTH { + return; + } + let holds = |t: &Story| own_has(t, fields) || inner_has(t, fields, depth + 1); + for t in st.tables.values_mut() { + if t.cells.iter().any(|c| holds(&c.text)) { + for c in &mut Arc::make_mut(t).cells { + fill_own(&mut c.text, fields, row); + fill_inner(&mut c.text, fields, row, depth + 1); + } + } + } + for n in st.notes.iter_mut().chain(st.endnotes.iter_mut()) { + if holds(&n.text) { + let n = Arc::make_mut(n); + fill_own(&mut n.text, fields, row); + fill_inner(&mut n.text, fields, row, depth + 1); + } + } +} + +/// Replace the markers of known fields in `st`'s own text with the record's values. +fn fill_own(st: &mut Story, fields: &[DataField], row: &[String]) { + let found: Vec<(usize, usize, String)> = + find_markers(&st.text).into_iter().filter(|(_, _, name)| fields.iter().any(|f| f.name == *name)).collect(); + for (start, end, name) in found.into_iter().rev() { + st.replace(start..end, cell_of(fields, row, &name)); + } +} + fn find_markers(text: &str) -> Vec<(usize, usize, String)> { let mut out = Vec::new(); let mut i = 0; diff --git a/crates/engine/src/cmd/datamerge/parse.rs b/crates/engine/src/cmd/datamerge/parse.rs index 5b8717baa..ca3a61b27 100644 --- a/crates/engine/src/cmd/datamerge/parse.rs +++ b/crates/engine/src/cmd/datamerge/parse.rs @@ -159,7 +159,11 @@ pub fn select_records(count: usize, records: &str, one: u32, range: &str, limit: let i = record_index(one, count)?; idx.push(i); } - "range" => select_range(range, count, &mut idx, &mut seen)?, + "range" => { + // Past this many records nothing more can be kept, so later ranges are only validated. + let keep = limit.filter(|l| *l > 0).map_or(count, |l| count.min(l as usize)); + select_range(range, count, keep, &mut idx, &mut seen)? + } other => return Err(format!("unknown records value \"{other}\"")), } if let Some(limit) = limit { @@ -175,7 +179,9 @@ pub fn select_records(count: usize, records: &str, one: u32, range: &str, limit: } /// Expand a comma-separated list of record numbers and `a-b` ranges into `idx`, skipping repeats. -fn select_range(range: &str, count: usize, idx: &mut Vec, seen: &mut std::collections::HashSet) -> Result<(), String> { +/// Every token is still checked, but once `idx` holds `keep` records no more are expanded, so +/// repeated or overlapping ranges cost nothing past that point. +fn select_range(range: &str, count: usize, keep: usize, idx: &mut Vec, seen: &mut std::collections::HashSet) -> Result<(), String> { if range.trim().is_empty() { return Err("no records".into()); } @@ -194,8 +200,11 @@ fn select_range(range: &str, count: usize, idx: &mut Vec, seen: &mut std: if a > b { return Err(format!("range {a}-{b} is reversed")); } + // Both ends in range means every record between them is. + record_index(a, count)?; + record_index(b, count)?; let mut n = a; - while n <= b { + while n <= b && idx.len() < keep { let i = record_index(n, count)?; if seen.insert(i) { idx.push(i); diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 5480a5686..e5cf28497 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -1497,3 +1497,178 @@ fn merged_records_give_anchored_objects_their_own_ids() { doc.check().unwrap(); designcraft_format::load(&designcraft_format::save(doc).unwrap()).unwrap(); } + +/// The cell texts of the first table in the active document's first story that has one. +fn cell_texts(s: &Session) -> Vec { + let doc = &s.documents()[s.active_index().unwrap()].doc; + let t = doc.stories.values().find_map(|st| st.tables.values().next().cloned()).expect("a table"); + t.cells.iter().map(|c| c.text.text.clone()).collect() +} + +/// A field inserted with the insertion point in a table cell goes into that cell, and the +/// preview and the merge fill it. The cell's offsets used to put the marker in the table's own +/// story, outside the table, and the merge never read a cell's text (#2315). +#[test] +fn a_field_inserted_in_a_table_cell_goes_there_and_is_filled() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City\nAda,London\nGrace,Arlington\n"); + let made = text_frame(&mut s, [36.0, 36.0, 400.0, 200.0], "Who\tWhere", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 9})); + exec(&mut s, "table.convertFromText", json!({"columnSeparator": "tab"})); + let story_text = |s: &Session| s.documents()[s.active_index().unwrap()].doc.stories.values().next().unwrap().text.clone(); + let before = story_text(&s); + exec(&mut s, "table.select", json!({"story": made["story"], "rows": [0, 0], "cols": [1, 1]})); + let r = exec(&mut s, "data.placeholder.add", json!({"field": "City"})); + assert_eq!(r["marker"], "<>", "{r}"); + assert_eq!(cell_texts(&s), ["Who", "<>Where"], "the marker is in the cell with the caret"); + assert_eq!(story_text(&s), before, "and not in the table's story"); + + exec(&mut s, "data.preview", json!({"record": 2})); + assert_eq!(cell_texts(&s), ["Who", "ArlingtonWhere"], "the preview fills the cell"); + exec(&mut s, "data.preview.stop", json!({})); + assert_eq!(cell_texts(&s), ["Who", "<>Where"], "stopping it puts the marker back"); + exec(&mut s, "data.merge", json!({})); + let merged = &s.documents()[s.active_index().unwrap()].doc; + let mut cities: Vec = + merged.stories.values().flat_map(|st| st.tables.values().map(|t| t.cells[1].text.text.clone()).collect::>()).collect(); + cities.sort(); + assert_eq!(cities, ["ArlingtonWhere", "LondonWhere"], "one table per record, filled"); +} + +/// A field typed (or inserted) in text that is then converted to a table is filled in its cell. +#[test] +fn a_field_converted_into_a_table_is_filled_in_its_cell() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City\nAda,London\n"); + let made = text_frame(&mut s, [36.0, 36.0, 400.0, 200.0], "Name\t<>", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 13})); + exec(&mut s, "table.convertFromText", json!({"columnSeparator": "tab"})); + assert_eq!(cell_texts(&s), ["Name", "<>"]); + exec(&mut s, "data.preview", json!({"record": 1})); + assert_eq!(cell_texts(&s), ["Name", "Ada"], "the field is filled in its cell"); +} + +/// A table `Who⇥Where` with the caret in its second cell, and a source with `Name` and `City`. +fn table_with_caret_in_cell(s: &mut Session, csv: &str) -> Value { + new_doc(s, 1); + select_csv(s, csv); + let made = text_frame(s, [36.0, 36.0, 400.0, 200.0], "Who\tWhere", None); + exec(s, "text.select", json!({"story": made["story"], "anchor": 0, "focus": 9})); + exec(s, "table.convertFromText", json!({"columnSeparator": "tab"})); + exec(s, "table.select", json!({"story": made["story"], "rows": [0, 0], "cols": [1, 1]})); + made +} + +/// After a field goes into a cell the caret is after it, in that cell, and the cell selection is +/// gone: typing goes on after the field instead of replacing the cell, field and all. +#[test] +fn typing_after_a_field_inserted_in_a_cell_goes_on_after_it() { + let mut s = Session::new(); + table_with_caret_in_cell(&mut s, "Name,City\nAda,London\n"); + exec(&mut s, "data.placeholder.add", json!({"field": "City"})); + let sel = &s.documents()[s.active_index().unwrap()].selection; + assert!(sel.cells.is_none(), "no cell selection left"); + exec(&mut s, "text.insert", json!({"text": ", "})); + assert_eq!(cell_texts(&s), ["Who", "<>, Where"]); +} + +/// A position that a merged cell covers is the merged cell's: the field goes into the merged cell, +/// where it shows, not into the hidden one under it. +#[test] +fn a_field_inserted_in_a_merged_cell_goes_where_it_shows() { + let mut s = Session::new(); + let made = table_with_caret_in_cell(&mut s, "Name,City\nAda,London\n"); + exec(&mut s, "table.merge", json!({"story": made["story"], "rows": [0, 0], "cols": [0, 1]})); + exec(&mut s, "table.select", json!({"story": made["story"], "rows": [0, 0], "cols": [1, 1]})); + exec(&mut s, "data.placeholder.add", json!({"field": "City"})); + let cells = cell_texts(&s); + assert!(cells[0].contains("<>"), "in the merged cell: {cells:?}"); + assert!(!cells[1].contains("<>"), "not in the covered one: {cells:?}"); + exec(&mut s, "data.preview", json!({"record": 1})); + assert!(cell_texts(&s)[0].contains("London"), "and filled: {:?}", cell_texts(&s)); +} + +/// A field whose name a marker can't hold (it has `<` or `>`) can't be filled from a cell, so it +/// isn't inserted there; in body text it still becomes an anchored placeholder. +#[test] +fn a_field_named_with_angle_brackets_is_refused_in_a_cell() { + let mut s = Session::new(); + let made = table_with_caret_in_cell(&mut s, "Customer ,City\n7,London\n"); + let e = s.execute("data.placeholder.add", &json!({"field": "Customer "})).expect_err("refused").to_string(); + assert!(e.contains("can't be inserted in a table cell"), "{e}"); + assert_eq!(cell_texts(&s), ["Who", "Where"], "nothing inserted"); + let other = text_frame(&mut s, [36.0, 300.0, 400.0, 400.0], "No. ", None); + exec(&mut s, "text.select", json!({"story": other["story"], "anchor": 4, "focus": 4})); + assert!(exec(&mut s, "data.placeholder.add", json!({"field": "Customer "}))["id"].is_u64(), "body text still takes it"); + let _ = made; +} + +/// Endnote text is filled too: it is a text inside its story, like a footnote's. +#[test] +fn a_field_in_an_endnote_is_filled() { + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City\nAda,London\n"); + let made = text_frame(&mut s, [36.0, 36.0, 400.0, 200.0], "See the note", None); + exec(&mut s, "text.select", json!({"story": made["story"], "anchor": 12, "focus": 12})); + exec(&mut s, "endnote.insert", json!({"text": "From <>"})); + let endnotes = |s: &Session| -> Vec { + s.documents()[s.active_index().unwrap()].doc.stories.values().flat_map(|st| st.endnotes.iter().map(|e| e.text.text.clone())).collect() + }; + assert_eq!(endnotes(&s), ["From <>"]); + exec(&mut s, "data.preview", json!({"record": 1})); + assert_eq!(endnotes(&s), ["From London"]); +} + +/// Tables nested in cells far deeper than any document goes are filled down to the merge's depth +/// limit, and the preview returns instead of exhausting the stack. +#[test] +fn deeply_nested_tables_are_filled_down_to_the_limit_without_overflowing() { + use designcraft_doc::{Cell, ParaFormat, Table}; + let mut s = Session::new(); + new_doc(&mut s, 1); + select_csv(&mut s, "Name,City\nAda,London\n"); + let made = text_frame(&mut s, [36.0, 36.0, 400.0, 200.0], "x", None); + let sid = StoryId(made["story"].as_u64().unwrap()); + // `depth` tables, each in the one cell of the next, each cell's own text `<>`. + let nest = |depth: usize| { + let mut inner: Option = None; + for i in 0..depth { + let mut t = Table::new(1000 + i as u64, 1, 1, 0, 0, 100.0); + t.cells[0] = Cell::with_text("<>", ParaFormat::default()); + if let Some(child) = inner.take() { + t.cells[0].text.insert_table(0, child); + } + inner = Some(t); + } + inner.unwrap() + }; + s.edit(|d, _| { + d.story_mut(sid).unwrap().insert_table(0, nest(500)); + Ok(()) + }) + .unwrap(); + exec(&mut s, "data.preview", json!({"record": 1})); + let doc = &s.documents()[s.active_index().unwrap()].doc; + let mut t = doc.story(sid).unwrap().tables.values().next().unwrap().clone(); + let mut filled = Vec::new(); + for _ in 0..40 { + filled.push(t.cells[0].text.text.contains("Ada")); + let Some(next) = t.cells[0].text.tables.values().next().cloned() else { break }; + t = next; + } + assert!(filled[..super::fill::MAX_NESTED_DEPTH].iter().all(|f| *f), "filled down to the limit: {filled:?}"); + assert!(!filled[super::fill::MAX_NESTED_DEPTH], "and not past it"); +} + +#[test] +fn select_records_stops_expanding_once_the_limit_is_met_but_still_checks_every_range() { + // 4 billion records in a range would not finish if it were expanded; the limit stops it. + let idx = parse::select_records(u32::MAX as usize, "range", 1, "1-3,1-4294967295,2-4294967295", Some(5)).unwrap(); + assert_eq!(idx, vec![0, 1, 2, 3, 4]); + // A later token that is out of range is still an error. + assert!(parse::select_records(10, "range", 1, "1-10,5-11", Some(2)).is_err()); + assert!(parse::select_records(10, "range", 1, "1-2,0", Some(2)).is_err()); +} diff --git a/crates/engine/src/cmd/export/formats.rs b/crates/engine/src/cmd/export/formats.rs index a25553d69..4474d8ae7 100644 --- a/crates/engine/src/cmd/export/formats.rs +++ b/crates/engine/src/cmd/export/formats.rs @@ -142,7 +142,7 @@ pub(super) fn export_text(s: &mut Session, p: &Value) -> Result { let text = match format { "rtf" => designcraft_textimport::export::rtf(doc, story), "tagged" => designcraft_textimport::tagged::tagged_text(doc, story), - _ => designcraft_textimport::export::plain_text(story), + _ => designcraft_textimport::export::plain_text(doc, story), }; match path { Some(path) => { @@ -159,13 +159,30 @@ fn page_text(d: &designcraft_doc::Document, cache: &designcraft_compose::Cache, let mut out = String::new(); for st in d.stories.values() { let cs = cache.get(d, st.id, None); + let spans = d.visible_spans(st); for ft in &cs.frames { if d.page_of_item(ft.frame) != Some(abs) { continue; } + // Pages render printing-only: a hidden or nonprinting frame or group around it, or a top-level item on a hidden or non-printable layer (members draw on its layer), isn't on the rendered page. + // Inside a multi-state object only the active state is drawn. + if d.find(ft.frame).is_some_and(|loc| { + (1..=loc.path.len()).any(|n| { + d.item_at(&designcraft_doc::ItemLoc { spread: loc.spread, path: loc.path[..n].to_vec() }).is_some_and(|it| { + it.hidden + || it.nonprinting + || n == 1 && d.layers.iter().any(|l| (!l.visible || !l.printable) && l.id == it.layer) + || n < loc.path.len() && !it.states.is_empty() && loc.path[n] != it.active_state + }) + }) + }) { + continue; + } for l in &ft.lines { let r = l.range.start.min(st.text.len())..l.range.end.min(st.text.len()); - let t: String = st.text[r].chars().filter(|c| !('\u{E000}'..='\u{F8FF}').contains(c)).collect(); + // The line's text as it shows: hidden conditional text and deletions left out. + let t: String = + designcraft_doc::Document::visible_text(st, &spans, r).chars().filter(|c| !('\u{E000}'..='\u{F8FF}').contains(c)).collect(); if !t.trim().is_empty() { out.push_str(t.trim_end_matches('\n')); out.push('\n'); diff --git a/crates/engine/src/cmd/export/text_tests.rs b/crates/engine/src/cmd/export/text_tests.rs index b1b791d3b..48e255ef7 100644 --- a/crates/engine/src/cmd/export/text_tests.rs +++ b/crates/engine/src/cmd/export/text_tests.rs @@ -92,6 +92,25 @@ fn epub_cover_and_fixed_layout() { assert!(p1.contains("width=612, height=792") && p1.contains("Page one words"), "{p1}"); } +/// Fixed-layout page text leaves out frames in a multi-state object's inactive states. +#[test] +fn fixed_epub_text_skips_inactive_states() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let a = s.execute("frame.create", &json!({"rect": [72, 72, 200, 100], "content": "text", "text": "Active words", "caret": false})).unwrap()["id"] + .as_u64() + .unwrap(); + let b = + s.execute("frame.create", &json!({"rect": [72, 300, 200, 100], "content": "text", "text": "Inactive words", "caret": false})).unwrap()["id"] + .as_u64() + .unwrap(); + s.execute("states.create", &json!({"ids": [a, b]})).unwrap(); + let r = s.execute("file.exportFixedEpub", &json!({})).unwrap(); + let mut z = unzip(r["base64"].as_str().unwrap()); + let p1 = read(&mut z, "OEBPS/page1.xhtml"); + assert!(p1.contains("Active words") && !p1.contains("Inactive words"), "{p1}"); +} + /// #775: the fallback follows the font's class, and embedFonts packages the stylesheet's fonts /// (and reports those it can't). #[test] diff --git a/crates/engine/src/cmd/find.rs b/crates/engine/src/cmd/find.rs index b9ab42550..63a1debb9 100644 --- a/crates/engine/src/cmd/find.rs +++ b/crates/engine/src/cmd/find.rs @@ -316,19 +316,27 @@ impl Search { ) -> Result, String)>> { let limit = if first_only { 1 } else { usize::MAX }; match (&self.re, self.spans(d, sid, cell, r)) { - (Some(re), None) if grep => re.replacements(text, rep, first_only).map_err(|e| bad("find.change", e)), - (Some(re), Some(spans)) if grep => Ok(re - .replacements(text, rep, false) - .map_err(|e| bad("find.change", e))? - .into_iter() - .filter(|(m, _)| within(&spans, m)) - .take(limit) - .collect()), - _ => Ok(self.find_all(d, sid, cell, r, text, limit)?.into_iter().map(|m| (m, rep.to_string())).collect()), + (Some(re), spans) if grep => re + .replacements(text, rep, first_only, MAX_CHANGE_BYTES, |m| spans.as_ref().is_none_or(|s| within(s, m))) + .map_err(|e| bad("find.change", e)), + _ => { + let found = self.find_all(d, sid, cell, r, text, limit)?; + // Every match gets its own copy of the change: bounded before any is made. + if found.len().saturating_mul(rep.len()) > MAX_CHANGE_BYTES { + return Err(bad( + "find.change", + format!("changing {} matches to {} bytes each would put more than {MAX_CHANGE_BYTES} bytes in", found.len(), rep.len()), + )); + } + Ok(found.into_iter().map(|m| (m, rep.to_string())).collect()) + } } } } +/// The most text one Change All may put into a story: the matches times the change's length. +const MAX_CHANGE_BYTES: usize = 64 << 20; + #[derive(Clone, Debug, PartialEq, Eq)] enum Domain { Document, diff --git a/crates/engine/src/cmd/find/tests.rs b/crates/engine/src/cmd/find/tests.rs index 348acd294..edea872ad 100644 --- a/crates/engine/src/cmd/find/tests.rs +++ b/crates/engine/src/cmd/find/tests.rs @@ -959,3 +959,21 @@ fn change_takes_any_selection_find_next_made() { assert_eq!(s.doc().unwrap().doc.story(sid).unwrap().text, if grep { "a" } else { "ab" }); } } + +/// Change All with a long change and many matches is refused before any copy of the change is +/// made, not after the memory for all of them has gone. +#[test] +fn change_all_is_bounded_by_what_it_would_put_in() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let text = "x ".repeat(2048); + s.execute("frame.create", &json!({"rect": [36, 36, 300, 300], "content": "text", "text": text})).unwrap(); + let huge = "y".repeat(40_000); + let e = s.execute("find.change", &json!({"find": "x", "change": huge})).unwrap_err().to_string(); + assert!(e.contains("would put more than"), "{e}"); + // GREP replacements are held to the same total while they are made. + let e = s.execute("find.change", &json!({"find": "x", "change": huge, "grep": true})).unwrap_err().to_string(); + assert!(e.contains("would put more than"), "{e}"); + let r = s.execute("find.change", &json!({"find": "x", "change": "yy"})).unwrap(); + assert_eq!(r["count"], 2048); +} diff --git a/crates/engine/src/cmd/interchange.rs b/crates/engine/src/cmd/interchange.rs index 47b36b776..e84c28103 100644 --- a/crates/engine/src/cmd/interchange.rs +++ b/crates/engine/src/cmd/interchange.rs @@ -17,7 +17,7 @@ use crate::{DocState, EngineError, Result, Session}; pub fn specs() -> Vec { vec![ cmd!(query "file.exportIdml", "Export IDML…", [], None, - "{path?, embedImages?: true} — writes an IDML package to `path`, or returns {base64} without a path", + "{path?, embedImages?: true} — writes an IDML package to `path`, or returns {base64} without a path; `warnings` names each image written as a link because the embedded-image limit was reached", has_doc, export_idml), cmd!(noundo "file.openIdml", "Open IDML", [], None, "{path | base64, name?} — opens an IDML package, or an InDesign document or template (.indd, .indt: converted to IDML first; its conversion warnings join `warnings`), as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings: conversion warnings, package parts missing, font files skipped, GREP styles whose pattern can't compile}", @@ -29,15 +29,19 @@ fn export_idml(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; let opts = designcraft_idml::ExportOptions { embed_images: p.get("embedImages").and_then(Value::as_bool).unwrap_or(true) }; let layer_doc = with_pdf_layer_variants(&st.doc, opts.embed_images)?; - let bytes = designcraft_idml::export_idml_with(layer_doc.as_ref().unwrap_or(&st.doc), &opts); - match str_param(p, "path") { + let (bytes, warnings) = designcraft_idml::export_idml_report(layer_doc.as_ref().unwrap_or(&st.doc), &opts); + let mut out = match str_param(p, "path") { Some(path) => { #[cfg(not(target_arch = "wasm32"))] crate::io::write_file(path, &bytes).map_err(EngineError::Other)?; - Ok(json!({"path": path, "bytes": bytes.len()})) + json!({"path": path, "bytes": bytes.len()}) } - None => Ok(json!({"base64": base64_encode(&bytes), "bytes": bytes.len()})), + None => json!({"base64": base64_encode(&bytes), "bytes": bytes.len()}), + }; + if !warnings.is_empty() { + out["warnings"] = json!(warnings); } + Ok(out) } /// Whether a placement holds an Object Layer Options choice an export can still apply: a graphic diff --git a/crates/engine/src/cmd/paste_svg.rs b/crates/engine/src/cmd/paste_svg.rs index a5d738cef..fa3c1cffc 100644 --- a/crates/engine/src/cmd/paste_svg.rs +++ b/crates/engine/src/cmd/paste_svg.rs @@ -3,6 +3,8 @@ //! clipboard as SVG markup; each path becomes a DesignCraft path with its fill and stroke, groups //! stay groups, and text comes in as outlines. +use std::collections::{HashMap, HashSet}; + use designcraft_doc::{Content, Fill, Item, ItemId, Selection, Shape, Stroke, StrokeType}; use designcraft_geom::{Affine, BezPath, PathData, Point, Rect}; use designcraft_images::usvg; @@ -201,16 +203,46 @@ fn bounds(art: &[Art]) -> Option { .reduce(|a, b| a.union(b)) } +/// The document's RGB colour swatches by colour and its swatch names, looked up once per paste +/// (a pasted SVG can hold tens of thousands of colours). +struct PasteSwatches { + by_rgb: HashMap<[u32; 3], String>, + names: HashSet, +} + +impl PasteSwatches { + /// An RGB colour as a key: equal colours, equal keys (`+ 0.0` makes -0 and 0 one key). + fn key(r: f32, g: f32, b: f32) -> [u32; 3] { + [(r + 0.0).to_bits(), (g + 0.0).to_bits(), (b + 0.0).to_bits()] + } + + fn new(d: &designcraft_doc::Document) -> Self { + use designcraft_color::swatch::SwatchValue; + let mut by_rgb = HashMap::new(); + for w in &d.swatches { + if let SwatchValue::Color { color: designcraft_color::Color::Rgb { r, g, b }, .. } = &w.value { + // The first swatch of a colour is the one, as a search from the start finds it. + by_rgb.entry(Self::key(*r, *g, *b)).or_insert_with(|| w.name.clone()); + } + } + Self { by_rgb, names: d.swatches.iter().map(|w| w.name.clone()).collect() } + } +} + /// The swatch holding exactly `rgb`: an existing process colour, else a new unnamed one (hidden /// from the Swatches panel until Add to Swatches), as InDesign does for pasted colours. -fn swatch_for(d: &mut designcraft_doc::Document, rgb: [u8; 3]) -> String { +fn swatch_for(d: &mut designcraft_doc::Document, known: &mut PasteSwatches, rgb: [u8; 3]) -> String { use designcraft_color::swatch::{Swatch, SwatchValue}; let color = designcraft_color::Color::rgb8(rgb[0], rgb[1], rgb[2]); - if let Some(w) = d.swatches.iter().find(|w| matches!(&w.value, SwatchValue::Color { color: c, .. } if *c == color)) { - return w.name.clone(); + let designcraft_color::Color::Rgb { r, g, b } = color else { return String::new() }; + let key = PasteSwatches::key(r, g, b); + if let Some(name) = known.by_rgb.get(&key) { + return name.clone(); } let base = format!("R={} G={} B={}", rgb[0], rgb[1], rgb[2]); - let name = designcraft_doc::Styles::unique_name(|n| d.swatch(n).is_some(), &base); + let name = designcraft_doc::Styles::unique_name(|n| known.names.contains(n), &base); + known.names.insert(name.clone()); + known.by_rgb.insert(key, name.clone()); d.swatches.push(Swatch { name: name.clone(), value: SwatchValue::Color { color, color_type: designcraft_color::ColorType::Process }, @@ -221,7 +253,7 @@ fn swatch_for(d: &mut designcraft_doc::Document, rgb: [u8; 3]) -> String { name } -fn build(d: &mut designcraft_doc::Document, art: Vec, layer: designcraft_doc::LayerId, shift: Affine) -> Vec { +fn build(d: &mut designcraft_doc::Document, known: &mut PasteSwatches, art: Vec, layer: designcraft_doc::LayerId, shift: Affine) -> Vec { let mut out = Vec::new(); for a in art { match a { @@ -239,12 +271,12 @@ fn build(d: &mut designcraft_doc::Document, art: Vec, layer: designcraft_do let paint = fill.map(|f| f.opacity).or(stroke.as_ref().map(|(c, _)| c.opacity)).unwrap_or(1.0); let mut it = Item::new(ItemId(d.alloc()), layer, if line { Shape::GraphicLine } else { Shape::Path }, data); it.fill = match fill { - Some(f) => Fill::swatch(&swatch_for(d, f.rgb)), + Some(f) => Fill::swatch(&swatch_for(d, known, f.rgb)), None => Fill::none(), }; it.stroke = match stroke { Some((c, s)) => Stroke { - swatch: swatch_for(d, c.rgb), + swatch: swatch_for(d, known, c.rgb), weight: s.width, cap: s.cap, join: s.join, @@ -258,7 +290,7 @@ fn build(d: &mut designcraft_doc::Document, art: Vec, layer: designcraft_do out.push(it); } Art::Group(items, opacity) => { - let children = build(d, items, layer, shift); + let children = build(d, known, items, layer, shift); if children.is_empty() { continue; } @@ -312,7 +344,8 @@ fn paste_svg(s: &mut Session, p: &Value) -> Result { warnings.push(format!("{} embedded image(s) left out (place images with File › Place)", notes.images)); } s.edit(|d, sel| { - let items = build(d, art, layer, shift); + let mut known = PasteSwatches::new(d); + let items = build(d, &mut known, art, layer, shift); let mut ids = Vec::new(); for it in items { ids.push(it.id); diff --git a/crates/engine/src/cmd/paths.rs b/crates/engine/src/cmd/paths.rs index 7800b3189..946da4215 100644 --- a/crates/engine/src/cmd/paths.rs +++ b/crates/engine/src/cmd/paths.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use designcraft_doc::{Content, Document, Fill, Item, ItemId, Selection, Shape, SpreadRef, Stroke}; -use designcraft_geom::{Affine, BezPath, PathData, SubPath}; +use designcraft_geom::{Affine, BezPath, PathData, Shape as _, SubPath}; use serde_json::{Value, json}; use super::{CommandSpec, bad, cmd, has_selection, point_param, targets}; @@ -75,20 +75,40 @@ fn spread_xf(d: &Document, id: ItemId) -> Option<(SpreadRef, Affine)> { Some((loc.spread, d.parent_xf(&loc) * d.item_at(&loc)?.xf)) } +/// Containment tests `orient_holes` makes at most (after the bounding-box filter). A compound path +/// of thousands of nested contours would otherwise take quadratic time; past this the command is +/// refused rather than leave later subpaths with the wrong direction. +const MAX_HOLE_TESTS: usize = 4_000_000; + /// Orient subpaths so those nested an odd number of times run against the others: with the /// non-zero rule they become holes. -fn orient_holes(path: &mut PathData) { +fn orient_holes(path: &mut PathData, command: &str) -> Result<()> { + orient_holes_within(path, command, MAX_HOLE_TESTS) +} + +fn orient_holes_within(path: &mut PathData, command: &str, max_tests: usize) -> Result<()> { let beziers: Vec = path.subpaths.iter().map(|s| PathData::single(s.clone()).to_bezpath()).collect(); + // A point outside a contour's box is outside the contour: most pairs are skipped without a winding count. + let boxes: Vec<_> = beziers.iter().map(BezPath::bounding_box).collect(); let n = path.subpaths.len(); + let mut tests = 0usize; for i in 0..n { let Some(p) = path.subpaths[i].anchors.first().map(|a| a.p) else { continue }; - let depth = (0..n).filter(|&j| j != i && designcraft_geom::hit::fill_contains(&beziers[j], designcraft_geom::FillRule::NonZero, p)).count(); + let mut depth = 0; + for j in (0..n).filter(|&j| j != i && boxes[j].inflate(1e-6, 1e-6).contains(p)) { + tests += 1; + if tests > max_tests { + return Err(bad(command, "too many nested contours to tell holes from fills")); + } + depth += usize::from(designcraft_geom::hit::fill_contains(&beziers[j], designcraft_geom::FillRule::NonZero, p)); + } let sp = &mut path.subpaths[i]; let cw = sp.area() > 0.0; if cw == (depth % 2 == 1) { sp.reverse(); } } + Ok(()) } fn make_compound(s: &mut Session, p: &Value) -> Result { @@ -115,7 +135,7 @@ fn make_compound(s: &mut Session, p: &Value) -> Result { } let it = d.item_mut(base_id).ok_or(designcraft_doc::DocError::NoItem(base_id))?; it.path = PathData::new(subpaths); - orient_holes(&mut it.path); + orient_holes(&mut it.path, "object.makeCompoundPath")?; it.shape = Shape::Path; *sel = Selection::items(vec![base_id]); Ok(json!({"id": base_id.0})) @@ -159,7 +179,7 @@ fn pathfinder(s: &mut Session, p: &Value) -> Result { } let it = d.item_mut(keep).ok_or(designcraft_doc::DocError::NoItem(keep))?; it.path = result.transformed(kxf.inverse()); - orient_holes(&mut it.path); + orient_holes(&mut it.path, "object.pathfinder")?; it.shape = Shape::Path; it.corners = Default::default(); *sel = Selection::items(vec![keep]); @@ -409,6 +429,38 @@ mod tests { s.doc().unwrap().doc.check().unwrap(); } + #[test] + fn orient_holes_alternates_nested_contours_and_skips_distant_ones() { + use designcraft_geom::{Rect, shapes::rectangle}; + let mut subs: Vec = Vec::new(); + for k in 0..3 { + let o = 10.0 * f64::from(k); + subs.extend(rectangle(Rect::new(o, o, 100.0 - o, 100.0 - o)).subpaths); + } + // Thousands of separate contours cost no winding counts (their boxes miss each other). + for k in 0..3000 { + let x = 200.0 + 20.0 * f64::from(k); + subs.extend(rectangle(Rect::new(x, 0.0, x + 10.0, 10.0)).subpaths); + } + let mut path = PathData::new(subs); + orient_holes(&mut path, "test").unwrap(); + let cw: Vec = path.subpaths.iter().map(|s| s.area() > 0.0).collect(); + assert_eq!(cw[..3], [true, false, true]); + assert!(cw[3..].iter().all(|c| *c)); + } + + #[test] + fn orient_holes_refuses_past_its_budget_instead_of_half_orienting() { + use designcraft_geom::{Rect, shapes::rectangle}; + let mut subs: Vec = Vec::new(); + for k in 0..4 { + let o = 10.0 * f64::from(k); + subs.extend(rectangle(Rect::new(o, o, 100.0 - o, 100.0 - o)).subpaths); + } + assert!(orient_holes_within(&mut PathData::new(subs.clone()), "test", 3).is_err()); + assert!(orient_holes_within(&mut PathData::new(subs), "test", 100).is_ok()); + } + #[test] fn pathfinder_subtracts_front_from_back() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 0f77d5372..e351fe837 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -133,7 +133,8 @@ fn convert_shape(s: &mut Session, p: &Value) -> Result { const ID: &str = "object.convertShape"; let to = str_param(p, "to").ok_or_else(|| bad(ID, "`to` required"))?.to_string(); let ids = targets(s, p)?; - let sides = s.prefs.polygon_sides; + // The preference as object.polygon takes it: 3–100 sides, whatever prefs.set was given. + let sides = s.prefs.polygon_sides.clamp(3, 100); let inset = s.prefs.star_inset; s.edit(|d, _| { let mut changed = 0; diff --git a/crates/engine/src/cmd/spelling.rs b/crates/engine/src/cmd/spelling.rs index 4a43ab752..8ff9ac94d 100644 --- a/crates/engine/src/cmd/spelling.rs +++ b/crates/engine/src/cmd/spelling.rs @@ -161,6 +161,9 @@ const COMMON: &[&str] = &[ "yay", ]; +/// Longest word [`suggest`] makes candidates for. +pub const MAX_SUGGEST_CHARS: usize = 40; + /// Is `w` (lowercase) a known word, allowing common inflections of dictionary stems? pub fn known(dict: &Dictionary, user: &[String], w: &str) -> bool { // A typographer's apostrophe (don’t, we’re) is the word list's straight one. @@ -207,6 +210,11 @@ pub fn known(dict: &Dictionary, user: &[String], w: &str) -> bool { /// Up to 6 suggestions at edit distance 1, ranked: transpositions, replacements, deletions, insertions. pub fn suggest(dict: &Dictionary, w: &str) -> Vec { + // Each candidate is a copy of the word: 53 per position, quadratic in its length. No word of + // a dictionary is this long, so no edit of such a "word" is one either. Checked before collecting. + if w.chars().take(MAX_SUGGEST_CHARS + 1).count() > MAX_SUGGEST_CHARS { + return Vec::new(); + } let chars: Vec = w.chars().collect(); let mut out: Vec = Vec::new(); let mut push = |c: Vec| { @@ -325,6 +333,18 @@ mod tests { assert!(known(d, &["designcraft".into()], "designcraft")); } + /// A "word" longer than any in the dictionary gets no candidates: there would be more than + /// fifty copies of it per character. + #[test] + fn very_long_words_get_no_suggestions() { + // The dictionary holds the longest allowed word, so deleting one letter of the 41-letter + // input would be a hit if the length cap were not applied. + let d = Dictionary::from_entries(vec![("a".repeat(MAX_SUGGEST_CHARS), 0)]); + assert_eq!(suggest(&d, &"a".repeat(MAX_SUGGEST_CHARS)), Vec::::new()); + assert_eq!(suggest(&d, &"a".repeat(MAX_SUGGEST_CHARS - 1)), vec!["a".repeat(MAX_SUGGEST_CHARS)]); + assert!(suggest(&d, &"a".repeat(MAX_SUGGEST_CHARS + 1)).is_empty()); + } + #[test] fn user_dictionary_replaces_and_counts() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/table/styles.rs b/crates/engine/src/cmd/table/styles.rs index fa3717977..046904fef 100644 --- a/crates/engine/src/cmd/table/styles.rs +++ b/crates/engine/src/cmd/table/styles.rs @@ -133,6 +133,11 @@ fn reapply_cell_styles(d: &mut Document, names: &[String]) { /// Include descendants when editing an inherited style, with the resolver's depth limit. fn dependent_styles<'a>(name: &str, styles: impl Iterator)>) -> Vec { let styles: Vec<_> = styles.collect(); + // The first style of a name is the one, as the resolver's lookup finds it. + let mut based_on: std::collections::HashMap<&str, Option<&str>> = std::collections::HashMap::new(); + for (n, base) in &styles { + based_on.entry(n).or_insert(*base); + } styles .iter() .filter(|(candidate, _)| { @@ -142,7 +147,7 @@ fn dependent_styles<'a>(name: &str, styles: impl Iterator Vec { has_text_or_frames, |s, p| format_paras(s, p.get("attrs").unwrap_or(p)) ), - cmd!("type.align", "Align", [], None, "{align: left|center|right|leftJustified|…}", has_text_or_frames, |s, p| { - format_paras(s, &json!({"align": p.get("align").cloned().unwrap_or(json!("left"))})) + cmd!("type.align", "Align", [], None, "{align: left|center|right|leftJustified|…|null} (required)", has_text_or_frames, |s, p| { + let align = p.get("align").cloned().ok_or_else(|| bad("type.align", "`align` required"))?; + format_paras(s, &json!({ "align": align })) }), cmd!("type.alignLeft", "Align Left", [], Some("Cmd+Shift+L"), "{}", has_text_or_frames, |s, _| format_paras(s, &json!({"align": "left"}))), cmd!("type.alignCenter", "Align Center", [], Some("Cmd+Shift+C"), "{}", has_text_or_frames, |s, _| format_paras( diff --git a/crates/engine/src/cmd/text/edit.rs b/crates/engine/src/cmd/text/edit.rs index e2e06ea40..b8efeaf01 100644 --- a/crates/engine/src/cmd/text/edit.rs +++ b/crates/engine/src/cmd/text/edit.rs @@ -487,12 +487,15 @@ pub(super) fn change_case(s: &mut Session, p: &Value) -> Result { let mut changes = Vec::new(); for seg in segs.iter().rev() { let old = st.slice(seg.clone()).to_string(); - let new: String = starts.iter().zip(&mapped).filter(|(b, _)| seg.contains(b)).map(|(_, m)| m.as_str()).collect(); + // The characters in this run: `starts` is sorted, so a search finds the slice. + let (from, to) = (starts.partition_point(|b| *b < seg.start), starts.partition_point(|b| *b < seg.end)); + let new: String = mapped.get(from..to).unwrap_or_default().iter().map(String::as_str).collect(); if new != old { delta += new.len() as isize - old.len() as isize; st.replace(seg.clone(), &new); // Per-character changes (back to front), so ranges inside the run keep their boundaries. - for (i, (b, m)) in starts.iter().zip(&mapped).enumerate().rev().filter(|(_, (b, _))| seg.contains(b)) { + for i in (from..to.min(mapped.len())).rev() { + let (b, m) = (&starts[i], &mapped[i]); let ce = starts.get(i + 1).copied().unwrap_or(r.end); if m.len() != ce - b { changes.push((*b..ce, m.len())); diff --git a/crates/engine/src/cmd/text/format.rs b/crates/engine/src/cmd/text/format.rs index d915da91d..64f3c17c7 100644 --- a/crates/engine/src/cmd/text/format.rs +++ b/crates/engine/src/cmd/text/format.rs @@ -1074,6 +1074,46 @@ mod pick_up_format_tests { } } +#[cfg(test)] +mod align_param_tests { + use serde_json::json; + + use crate::Session; + + /// `type.align` with empty params (`{}`) or a bare value (`"center"`) applied Align Left. + /// A misspelled key and an unknown value were already refused, and each case checks which + /// check refuses it. + #[test] + fn type_align_without_a_value_is_refused_and_changes_nothing() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 1})).unwrap(); + let f = s.execute("frame.create", &json!({"rect": [20, 20, 400, 300], "content": "text", "text": "Hello world"})).unwrap(); + s.execute("text.select", &json!({"story": f["story"].clone(), "anchor": 0, "focus": 5})).unwrap(); + s.execute("type.align", &json!({"align": "right"})).unwrap(); + let align = |s: &mut Session| s.execute("type.selectionAttrs", &json!({})).unwrap()["para"]["align"].clone(); + assert_eq!(align(&mut s), json!("right")); + let steps = s.doc().unwrap().history.undo.len(); + // `{}` and `"center"` reach the command without `align`. The parameter check refuses + // `{"alignment": …}` and the value parser refuses `"middle"`. + for (params, refusal) in [ + (json!({}), "`align` required"), + (json!("center"), "`align` required"), + (json!({"alignment": "center"}), "unknown parameter `alignment`"), + (json!({"align": "middle"}), "middle"), + ] { + let err = s.execute("type.align", ¶ms).unwrap_err().to_string(); + assert!(err.contains(refusal), "{params}: {err}"); + assert_eq!(align(&mut s), json!("right"), "{params}"); + assert_eq!(s.doc().unwrap().history.undo.len(), steps, "{params}: no undo step"); + } + // A named value still applies, and null removes the override. + s.execute("type.align", &json!({"align": "center"})).unwrap(); + assert_eq!(align(&mut s), json!("center")); + s.execute("type.align", &json!({"align": null})).unwrap(); + assert_eq!(align(&mut s), json!("left")); + } +} + #[cfg(test)] mod size_bounds_tests { use serde_json::json; diff --git a/crates/engine/src/cmd/xml.rs b/crates/engine/src/cmd/xml.rs index 81b57b5b3..949d96436 100644 --- a/crates/engine/src/cmd/xml.rs +++ b/crates/engine/src/cmd/xml.rs @@ -60,10 +60,13 @@ fn plain(t: &str) -> String { } /// A paragraph's text with inline-tagged runs as elements. -fn para_xml(st: &designcraft_doc::Story, r: std::ops::Range) -> String { +/// `runs` are the story's runs in order (collected once per story): only those reaching the +/// paragraph are visited, found by binary search. +fn para_xml(st: &designcraft_doc::Story, runs: &[(std::ops::Range, &designcraft_doc::CharFormat)], r: std::ops::Range) -> String { let mut out = String::new(); let mut open: Option = None; - for (rr, f) in st.runs() { + let first = runs.partition_point(|(rr, _)| rr.end <= r.start); + for (rr, f) in runs.iter().skip(first).take_while(|(rr, _)| rr.start < r.end) { let (a, b) = (rr.start.max(r.start), rr.end.min(r.end)); if a >= b { continue; @@ -97,8 +100,10 @@ pub fn export_xml(d: &Document) -> String { let Some(st) = d.story(tf.story) else { continue }; let mapped = |style: &str| d.xml.style_map.iter().find(|(s, _)| s == style).map(|(_, t)| t.clone()); // Paragraph text with inline tags already escaped. - let paras: Vec<(Option, String)> = - st.para_ranges().into_iter().enumerate().map(|(i, r)| (mapped(&st.paras[i].style), para_xml(st, r))).collect(); + let paras: Vec<(Option, String)> = { + let runs: Vec<_> = st.runs().collect(); + st.para_ranges().into_iter().enumerate().map(|(i, r)| (mapped(&st.paras[i].style), para_xml(st, &runs, r))).collect() + }; if paras.iter().any(|(m, _)| m.is_some()) { out.push_str(&format!(" <{tag}>\n")); for (m, text) in paras { @@ -341,8 +346,10 @@ pub fn specs() -> Vec { let dtd = crate::dtd::parse(&text).map_err(|e| bad("xml.loadDtd", e))?; let names: Vec = dtd.element_names().map(str::to_string).collect(); s.edit(|d, _| { + // The tags there are, then those added: one set, not a search per element. + let mut known: std::collections::HashSet = d.xml.tags.iter().map(|t| t.name.clone()).collect(); for n in &names { - if valid_name(n) && !d.xml.tags.iter().any(|t| t.name == *n) { + if valid_name(n) && known.insert(n.clone()) { let color = COLORS[d.xml.tags.len() % COLORS.len()]; d.xml.tags.push(XmlTag { name: n.clone(), color }); } diff --git a/crates/engine/src/dtd.rs b/crates/engine/src/dtd.rs index 6b48ba3ab..c0d45d820 100644 --- a/crates/engine/src/dtd.rs +++ b/crates/engine/src/dtd.rs @@ -29,6 +29,8 @@ enum Model { #[derive(Clone, Debug, Default)] pub struct Dtd { elements: Vec<(String, Model)>, + /// Each declared name's first declaration (which wins), by name. + index: HashMap, /// Element → its `#REQUIRED` attributes. required: HashMap>, } @@ -47,7 +49,7 @@ impl Dtd { } fn model(&self, name: &str) -> Option<&Model> { - self.elements.iter().find(|(n, _)| n == name).map(|(_, m)| m) + self.elements.get(*self.index.get(name)?).map(|(_, m)| m) } } @@ -252,6 +254,7 @@ pub fn parse(src: &str) -> Result { let rest = rest.trim_start(); let name: String = rest.chars().take_while(|c| !c.is_whitespace()).collect(); let model = parse_model(&rest[name.len()..]).map_err(|e| format!(": {e}"))?; + dtd.index.entry(name.clone()).or_insert(dtd.elements.len()); dtd.elements.push((name, model)); } else if let Some(rest) = d.strip_prefix("ATTLIST") { // name (attr type default)*; only `#REQUIRED` matters here. @@ -309,7 +312,39 @@ pub fn parse(src: &str) -> Result { /// What `ends` has worked out for one sequence of children: the end positions of a group (by the /// address of its node, which lives as long as the model) from a position. -type EndsMemo = HashMap<(usize, usize), Vec>; +struct EndsMemo { + map: HashMap<(usize, usize), Vec>, + /// Matching steps left (shared by a whole validation); at zero `ends` stops and `over` is set. + left: usize, + /// End positions held in `map`; past [`MAX_STORED_ENDS`] results are no longer kept. + stored: usize, + over: bool, +} + +/// Matching steps one validation may take across all its elements (a step is a position handled or +/// compared). Valid documents with thousands of children per element use a small fraction. +const MAX_MATCH_STEPS: usize = 8 * 1024 * 1024; +/// End positions the memo may hold at once (8 bytes each). +const MAX_STORED_ENDS: usize = 4 * 1024 * 1024; +/// Element nesting depth: `check` recurses and keeps each level's full path. +const MAX_XML_DEPTH: usize = 256; +/// Elements in the document being validated. +const MAX_XML_NODES: usize = 4 * 1024 * 1024; +/// Problems reported before the list stops. +const MAX_PROBLEMS: usize = 10_000; + +impl EndsMemo { + /// Spend `n` steps; false once the budget is gone. + fn spend(&mut self, n: usize) -> bool { + if self.over || n > self.left { + self.left = 0; + self.over = true; + return false; + } + self.left -= n; + true + } +} /// End positions after `cp` matches a prefix of `seq[i..]`. /// @@ -321,8 +356,16 @@ fn ends(cp: &Cp, seq: &[&str], i: usize, memo: &mut EndsMemo) -> Vec { return if seq.get(i) == Some(&n.as_str()) { vec![i + 1] } else { vec![] }; } let key = (std::ptr::from_ref(cp) as usize, i); - if let Some(done) = memo.get(&key) { - return done.clone(); + if let Some(done) = memo.map.get(&key) { + // Charged before the clone, and a failed charge ends the match. + let len = done.len(); + if !memo.spend(len) { + return vec![]; + } + return memo.map.get(&key).cloned().unwrap_or_default(); + } + if !memo.spend(1) { + return vec![]; } let mut out: Vec = match cp { Cp::Name(_) => vec![], @@ -334,6 +377,9 @@ fn ends(cp: &Cp, seq: &[&str], i: usize, memo: &mut EndsMemo) -> Vec { for p in at { next.extend(ends(c, seq, p, memo)); } + if !memo.spend(next.len()) { + return vec![]; + } next.sort_unstable(); next.dedup(); at = next; @@ -349,16 +395,21 @@ fn ends(cp: &Cp, seq: &[&str], i: usize, memo: &mut EndsMemo) -> Vec { } Cp::Rep(c, r) => { let mut all = if *r == '+' { vec![] } else { vec![i] }; + // `all` as a set too: a list search per position made long repeats quadratic. + let mut have: std::collections::HashSet = all.iter().copied().collect(); let mut frontier = ends(c, seq, i, memo); if *r == '?' { all.extend(frontier); } else { while !frontier.is_empty() { - let new: Vec = frontier.iter().copied().filter(|p| !all.contains(p)).collect(); + if !memo.spend(frontier.len()) { + break; + } + let new: Vec = frontier.iter().copied().filter(|p| have.insert(*p)).collect(); all.extend(&new); let mut next = Vec::new(); for p in new.into_iter().filter(|&p| p > i) { - next.extend(ends(c, seq, p, memo).into_iter().filter(|p| !all.contains(p))); + next.extend(ends(c, seq, p, memo).into_iter().filter(|p| !have.contains(p))); } // Two paths to one position are one position: kept apart they multiply per round. next.sort_unstable(); @@ -371,7 +422,10 @@ fn ends(cp: &Cp, seq: &[&str], i: usize, memo: &mut EndsMemo) -> Vec { }; out.sort_unstable(); out.dedup(); - memo.insert(key, out.clone()); + if !memo.over && memo.stored + out.len() <= MAX_STORED_ENDS { + memo.stored += out.len(); + memo.map.insert(key, out.clone()); + } out } @@ -396,6 +450,7 @@ fn read_tree(xml: &str) -> Result { use quick_xml::events::Event; let mut r = quick_xml::Reader::from_str(xml); let mut stack: Vec = Vec::new(); + let mut count = 0usize; let node = |e: &quick_xml::events::BytesStart| Node { name: String::from_utf8_lossy(e.name().as_ref()).to_string(), attrs: e.attributes().flatten().map(|a| String::from_utf8_lossy(a.key.as_ref()).to_string()).collect(), @@ -403,8 +458,20 @@ fn read_tree(xml: &str) -> Result { text: false, }; loop { - match r.read_event().map_err(|e| e.to_string())? { - Event::Start(e) => stack.push(node(&e)), + let event = r.read_event().map_err(|e| e.to_string())?; + if matches!(event, Event::Start(_) | Event::Empty(_)) { + count += 1; + if count > MAX_XML_NODES { + return Err(format!("more than {MAX_XML_NODES} elements")); + } + } + match event { + Event::Start(e) => { + if stack.len() >= MAX_XML_DEPTH { + return Err(format!("elements nested more than {MAX_XML_DEPTH} deep")); + } + stack.push(node(&e)); + } Event::Empty(e) => { let n = node(&e); match stack.last_mut() { @@ -437,8 +504,15 @@ fn read_tree(xml: &str) -> Result { } } -fn check(dtd: &Dtd, n: &Node, path: &str, out: &mut Vec) { - let mut problem = |m: String| out.push(Problem { path: path.to_string(), message: m }); +fn check(dtd: &Dtd, n: &Node, path: &str, out: &mut Vec, steps: &mut usize) { + if out.len() >= MAX_PROBLEMS { + return; + } + let mut problem = |m: String| { + if out.len() < MAX_PROBLEMS { + out.push(Problem { path: path.to_string(), message: m }); + } + }; match dtd.model(&n.name) { None => problem(format!("`{}` isn't declared in the DTD", n.name)), Some(Model::Any) => {} @@ -459,7 +533,12 @@ fn check(dtd: &Dtd, n: &Node, path: &str, out: &mut Vec) { problem(format!("`{}` can't contain text", n.name)); } let seq: Vec<&str> = n.children.iter().map(|c| c.name.as_str()).collect(); - if !ends(cp, &seq, 0, &mut EndsMemo::new()).contains(&seq.len()) { + let mut memo = EndsMemo { map: HashMap::new(), left: *steps, stored: 0, over: false }; + let matched = ends(cp, &seq, 0, &mut memo).contains(&seq.len()); + *steps = memo.left; + if memo.over { + problem(format!("`{}` has too many children to check against its content model", n.name)); + } else if !matched { let got = if seq.is_empty() { "nothing".to_string() } else { seq.join(", ") }; problem(format!("`{}` contains {got}; the DTD asks for {}", n.name, show(cp))); } @@ -474,7 +553,7 @@ fn check(dtd: &Dtd, n: &Node, path: &str, out: &mut Vec) { for c in &n.children { let k = seen.entry(c.name.as_str()).or_default(); *k += 1; - check(dtd, c, &format!("{path}/{}[{k}]", c.name), out); + check(dtd, c, &format!("{path}/{}[{k}]", c.name), out, steps); } } @@ -484,7 +563,10 @@ pub fn validate(dtd: &Dtd, xml: &str) -> Vec { Ok(root) => { let mut out = Vec::new(); let path = root.name.clone(); - check(dtd, &root, &path, &mut out); + check(dtd, &root, &path, &mut out, &mut { MAX_MATCH_STEPS }); + if out.len() >= MAX_PROBLEMS { + out.push(Problem { path: String::new(), message: "further problems are not listed".into() }); + } out } Err(e) => vec![Problem { path: String::new(), message: format!("not well-formed XML: {e}") }], @@ -530,6 +612,15 @@ mod tests { assert!(parse("just text").is_err()); } + /// An element declared twice keeps its first model (the index points there). + #[test] + fn the_first_declaration_of_an_element_wins() { + let dtd = parse("\n\n").unwrap(); + assert_eq!(dtd.element_names().collect::>(), ["a", "b", "a"]); + assert_eq!(validate(&dtd, ""), vec![]); + assert!(!validate(&dtd, "").is_empty()); + } + /// A few hundred bytes of nested parameter entities used to inflate multiplicatively across /// the eight wholly-string-replacing passes (#713); expansion is capped. #[test] @@ -743,4 +834,29 @@ mod tests { assert_eq!(problems.len(), 1, "{problems:?}"); assert!(problems[0].message.starts_with("`r` contains a, a, a"), "{problems:?}"); } + + #[test] + fn nested_repeats_over_many_children_stop_at_the_step_budget() { + let dtd = parse("").unwrap(); + assert_eq!(validate(&dtd, &format!("{}", "".repeat(1000))), vec![]); + let problems = validate(&dtd, &format!("{}", "".repeat(20_000))); + assert_eq!(problems.len(), 1, "{problems:?}"); + assert!(problems[0].message.contains("too many children"), "{problems:?}"); + } + + #[test] + fn xml_nesting_is_bounded() { + let dtd = parse("").unwrap(); + let deep = |n: usize| format!("{}{}", "".repeat(n), "".repeat(n)); + assert_eq!(validate(&dtd, &deep(MAX_XML_DEPTH)), vec![]); + let problems = validate(&dtd, &deep(MAX_XML_DEPTH + 1)); + assert!(problems[0].message.contains("nested more than"), "{problems:?}"); + } + + #[test] + fn problems_are_capped() { + let dtd = parse("").unwrap(); + let problems = validate(&dtd, &format!("{}", "".repeat(MAX_PROBLEMS + 50))); + assert!(problems.len() <= MAX_PROBLEMS + 1, "{}", problems.len()); + } } diff --git a/crates/epub/src/chapters.rs b/crates/epub/src/chapters.rs index ede96f390..b9b6e6183 100644 --- a/crates/epub/src/chapters.rs +++ b/crates/epub/src/chapters.rs @@ -99,7 +99,8 @@ pub(crate) fn toc_entries(doc: &Document, order: &[Block]) -> Vec<(String, Strin continue; } let Some(st) = doc.story(*sid) else { continue }; - let first = label(st.text.split('\n').next().unwrap_or("")); + let spans = doc.visible_spans(st); + let first = st.para_ranges().into_iter().map(|r| label(&Document::visible_text(st, &spans, r))).find(|t| !t.is_empty()).unwrap_or_default(); if !first.is_empty() { firsts.push((format!("s{k}"), first, 1)); } @@ -107,7 +108,7 @@ pub(crate) fn toc_entries(doc: &Document, order: &[Block]) -> Vec<(String, Strin let Some(pf) = st.paras.get(pi) else { continue }; let (tag, _) = export_mapping(&doc.styles, &pf.style, false, &pc); let Some(level) = heading_level(tag).filter(|l| *l <= 3) else { continue }; - let text = label(st.text.get(r.clone()).unwrap_or("")); + let text = label(&Document::visible_text(st, &spans, r.clone())); if !text.is_empty() { headings.push((format!("s{k}-p{pi}"), text, level)); } diff --git a/crates/epub/src/html.rs b/crates/epub/src/html.rs index 6442d5045..233c2764e 100644 --- a/crates/epub/src/html.rs +++ b/crates/epub/src/html.rs @@ -14,11 +14,22 @@ pub enum Block { pub fn reading_order(doc: &Document) -> Vec { // Articles, when there are any to export, decide the order (and what's included). if doc.articles.iter().any(|a| a.export) { + let hidden_layers: Vec<_> = doc.layers.iter().filter(|l| !l.visible).map(|l| l.id).collect(); let mut out = Vec::new(); let mut seen = Vec::new(); for a in doc.articles.iter().filter(|a| a.export) { for id in &a.items { let Some(top) = doc.item(*id) else { continue }; + // A hidden item or group hides everything in it. The layer that counts is the + // top-level item's: pages draw a group's members on the group's layer. + if doc.find(*id).is_some_and(|loc| { + (1..=loc.path.len()).any(|n| { + doc.item_at(&designcraft_doc::ItemLoc { spread: loc.spread, path: loc.path[..n].to_vec() }) + .is_some_and(|g| g.hidden || n == 1 && hidden_layers.contains(&g.layer)) + }) + }) { + continue; + } let mut push = |it: &Item| match &it.content { Content::Text(tf) if !seen.contains(&tf.story) => { seen.push(tf.story); @@ -27,11 +38,21 @@ pub fn reading_order(doc: &Document) -> Vec { Content::Graphic(_) => out.push(Block::Image(it.id)), _ => {} }; - top.walk(&mut |i| push(i)); + // Hidden items (with everything in them) aren't on the page. + let mut stack = vec![top]; + while let Some(it) = stack.pop() { + if it.hidden { + continue; + } + push(it); + stack.extend(it.children().iter().rev().map(|c| &**c)); + } } } return out; } + let hidden_layers: Vec<_> = doc.layers.iter().filter(|l| !l.visible).map(|l| l.id).collect(); + let shown = shown_items(doc, &hidden_layers); let mut keyed: Vec<((usize, i64, i64), Block)> = Vec::new(); let mut seen_stories = Vec::new(); for (si, sp) in doc.spreads.iter().enumerate() { @@ -45,7 +66,8 @@ pub fn reading_order(doc: &Document) -> Vec { continue; }; let xf = *parent * it.xf; - if it.hidden { + // Hidden items, and top-level items on hidden layers (with everything in them), aren't on the page. + if it.hidden || stack.len() == 1 && hidden_layers.contains(&it.layer) { continue; } let children = it.children(); @@ -55,8 +77,13 @@ pub fn reading_order(doc: &Document) -> Vec { let block = match &it.content { Content::Text(tf) => { let Some(story) = doc.story(tf.story) else { continue }; - // A story is placed where its first frame is. - if story.frames.first() != Some(&it.id) || seen_stories.contains(&tf.story) || story.text.trim().is_empty() { + // A story is placed where its first frame on the page is: a story is left out + // only when every frame of it is hidden, and a hidden first frame does not hide + // the rest of the thread. + if story.frames.iter().find(|f| shown.contains(f)) != Some(&it.id) + || seen_stories.contains(&tf.story) + || story.text.trim().is_empty() + { continue; } seen_stories.push(tf.story); @@ -78,6 +105,22 @@ pub fn reading_order(doc: &Document) -> Vec { keyed.into_iter().map(|(_, b)| b).collect() } +/// The items the page shows: not hidden themselves, in a hidden group, or (top level) on a hidden layer. +fn shown_items(doc: &Document, hidden_layers: &[designcraft_doc::LayerId]) -> std::collections::HashSet { + let mut shown = std::collections::HashSet::new(); + for sp in &doc.spreads { + let mut stack: Vec<(&Item, bool)> = sp.items.iter().map(|i| (&**i, true)).collect(); + while let Some((it, top)) = stack.pop() { + if it.hidden || top && hidden_layers.contains(&it.layer) { + continue; + } + shown.insert(it.id); + stack.extend(it.children().iter().map(|c| (&**c, false))); + } + } + shown +} + /// Package path of an asset: `{dir}/{name}-{id}.{ext}`, the name from its original file name /// (lowercase ASCII, so the package stays readable) and the id keeping two assets of the same /// name apart. diff --git a/crates/epub/src/story_writer.rs b/crates/epub/src/story_writer.rs index d6fb05159..31ca17ebc 100644 --- a/crates/epub/src/story_writer.rs +++ b/crates/epub/src/story_writer.rs @@ -120,9 +120,15 @@ impl<'a, 'f> StoryWriter<'a, 'f> { let mut lead = lead; let mut list: Option<&str> = None; let mut pre: Option<(String, bool)> = None; + let runs: Vec<(std::ops::Range, &story::CharFormat)> = st.runs().collect(); for (pi, (r, pf)) in st.para_ranges().into_iter().zip(&st.paras).enumerate() { let Some(text) = st.text.get(r.clone()) else { continue }; - let table = pf.table.and_then(|id| st.tables.get(&id)).filter(|_| text.contains(story::TABLE_ANCHOR)); + let base = doc.styles.resolve_para(pf).1; + // A table whose anchor is hidden or deleted is off the page, so it stays out. + let table = pf + .table + .and_then(|id| st.tables.get(&id)) + .filter(|_| text.find(story::TABLE_ANCHOR).is_some_and(|at| !doc.hides_run(&base, st.char_format_of(r.start + at)))); let only_table = table.is_some() && text.chars().all(|c| c == story::TABLE_ANCHOR || c.is_whitespace()); if !only_table || !lead.is_empty() { // Export Tagging: the style's element and class. @@ -167,7 +173,7 @@ impl<'a, 'f> StoryWriter<'a, 'f> { lead = ""; let mut after = String::new(); if !only_table { - self.runs(st, r, depth, out, &mut after); + self.runs(st, &runs, r, &base, depth, out, &mut after); } if tag == "pre" { // Closed when the next paragraph isn't a line of the same block. @@ -224,17 +230,29 @@ impl<'a, 'f> StoryWriter<'a, 'f> { t } - /// The runs of paragraph `para` of `st`; footnotes go to `after`. - fn runs(&mut self, st: &Story, para: std::ops::Range, depth: usize, out: &mut String, after: &mut String) { + /// The runs of paragraph `para` of `st` (`runs`: the story's, collected once; `base`: the + /// paragraph's base character attributes); footnotes go to `after`. + #[allow(clippy::too_many_arguments)] + fn runs( + &mut self, + st: &Story, + runs: &[(std::ops::Range, &story::CharFormat)], + para: std::ops::Range, + base: &designcraft_doc::CharProps, + depth: usize, + out: &mut String, + after: &mut String, + ) { let doc = self.doc; - for (rr, f) in st.runs() { + let first = runs.partition_point(|(rr, _)| rr.end <= para.start); + for (rr, f) in runs.iter().skip(first).take_while(|(rr, _)| rr.start < para.end).map(|(rr, f)| (rr.clone(), *f)) { let a = rr.start.max(para.start); let b = rr.end.min(para.end); if a >= b { continue; } - // Hidden conditional text isn't exported. - if f.over.conditions.as_deref().is_some_and(|c| doc.conditions_hide(c)) || f.over.change == Some(designcraft_doc::ChangeMark::Deleted) { + // Hidden conditional text (the styles' conditions included) and deletions aren't exported. + if doc.hides_run(base, f) { continue; } let Some(raw) = st.text.get(a..b) else { continue }; diff --git a/crates/epub/src/tests.rs b/crates/epub/src/tests.rs index 62683f91a..62195aec0 100644 --- a/crates/epub/src/tests.rs +++ b/crates/epub/src/tests.rs @@ -1025,3 +1025,72 @@ fn hidden_group_children_are_not_exported() { assert!(html::reading_order(&grouped_text(true)).is_empty()); assert_eq!(html::reading_order(&grouped_text(false)).len(), 1); } + +/// Text hidden by a condition the paragraph style carries, and text marked deleted, are off the +/// page: neither the HTML body nor the navigation labels show them. A frame on a hidden layer +/// isn't exported at all. +#[test] +fn text_hidden_by_inherited_conditions_or_deletion_is_not_exported() { + use designcraft_doc::{ChangeMark, CharAttrs, Condition, ParagraphStyle}; + let mut d = Document::new(&NewDocument::default()); + d.conditions.push(Condition { name: "Secret".into(), color: [0, 0, 0], visible: false }); + d.styles_mut().paragraph.push(ParagraphStyle { + name: "Classified".into(), + based_on: None, + next_style: None, + para: Default::default(), + chars: CharAttrs { conditions: Some(vec!["Secret".into()]), ..Default::default() }, + shortcut: String::new(), + }); + let lid = d.default_layer(); + let (_, sid) = d + .add_text_frame( + SpreadRef::Doc(0), + Rect::new(36.0, 36.0, 300.0, 100.0), + lid, + "Public heading\nclassified line\nkept dropped", + ParaFormat::default(), + ) + .unwrap(); + let st = d.story_mut(sid).unwrap(); + let c = st.text.find("classified").unwrap(); + st.format_paras(c..c + 1, |p| p.style = "Classified".into()); + let g = st.text.find(" dropped").unwrap(); + st.format_chars(g..g + 8, |f| f.over = CharAttrs { change: Some(ChangeMark::Deleted), ..Default::default() }); + let hidden_layer = designcraft_doc::LayerId(d.alloc()); + d.layers.push(designcraft_doc::Layer { + id: hidden_layer, + name: "Hidden".into(), + color: [0, 0, 0], + visible: false, + locked: false, + printable: true, + show_guides: true, + suppress_wrap_when_hidden: false, + }); + d.add_text_frame(SpreadRef::Doc(0), Rect::new(36.0, 200.0, 300.0, 300.0), hidden_layer, "On a hidden layer", ParaFormat::default()).unwrap(); + let html = html::export_html(&d, &html::HtmlOptions::default()); + assert!(html.contains("Public heading") && html.contains("kept"), "{html}"); + for gone in ["classified line", "dropped", "On a hidden layer"] { + assert!(!html.contains(gone), "{gone} must not be exported: {html}"); + } + let labels: Vec = chapters::toc_entries(&d, &html::reading_order(&d)).into_iter().map(|(_, label, _)| label).collect(); + assert_eq!(labels, ["Public heading"]); +} + +/// A threaded story is left out only when every one of its frames is hidden: a hidden first +/// frame does not hide the frames after it. +#[test] +fn a_threaded_story_is_skipped_only_when_every_frame_is_hidden() { + let mut d = Document::new(&NewDocument::default()); + let lid = d.default_layer(); + let (a, sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(36.0, 36.0, 300.0, 100.0), lid, "first", ParaFormat::default()).unwrap(); + let (b, _) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(36.0, 200.0, 300.0, 300.0), lid, "second", ParaFormat::default()).unwrap(); + d.thread(a, b).unwrap(); + let story = |d: &Document| html::reading_order(d).iter().filter(|x| **x == html::Block::Story(sid)).count(); + assert_eq!(story(&d), 1); + d.item_mut(a).unwrap().hidden = true; + assert_eq!(story(&d), 1, "the visible second frame keeps the story"); + d.item_mut(b).unwrap().hidden = true; + assert_eq!(story(&d), 0); +} diff --git a/crates/epub/tests/it/grouped_reading_order.rs b/crates/epub/tests/it/grouped_reading_order.rs index c4689cf20..eb820883a 100644 --- a/crates/epub/tests/it/grouped_reading_order.rs +++ b/crates/epub/tests/it/grouped_reading_order.rs @@ -145,10 +145,13 @@ fn articles_keep_their_explicit_order_and_exclusions() { let (c, _) = text(&mut doc, 0, 36.0, 10.0, "Article third"); let (omitted, _) = text(&mut doc, 0, 36.0, 5.0, "Excluded story"); doc.articles.push(Article { name: "Unused".into(), items: vec![omitted], export: false }); - doc.articles.push(Article { name: "Chosen".into(), items: vec![grouped, c, grouped], export: true }); + let (hidden, _) = text(&mut doc, 0, 36.0, 1.0, "Hidden in article"); + doc.item_mut(hidden).unwrap().hidden = true; + doc.articles.push(Article { name: "Chosen".into(), items: vec![grouped, c, grouped, hidden], export: true }); assert_output_order(&doc, &["Article first", "Article second", "Article third"]); for output in outputs(&doc) { assert!(!output.contains("Excluded story")); + assert!(!output.contains("Hidden in article")); } } diff --git a/crates/fonts/src/fontdb/fallback.rs b/crates/fonts/src/fontdb/fallback.rs index 73a2e0a25..7291cb343 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/crates/fonts/src/fontdb/fallback.rs @@ -10,8 +10,17 @@ pub(super) struct SysFallback { pub(super) misses: std::collections::HashSet, /// CJK chain families already looked up in the catalog (loaded, or not installed). pub(super) chain_tried: std::collections::HashSet<&'static str>, + /// The code points each installed file covers (as ranges), by path, valid for the file's + /// stamp: one character after another is looked up without reading every file again. + pub(super) coverage: std::collections::HashMap>)>, + /// Ranges held in `coverage` (it stops growing at [`MAX_COVERAGE_RANGES`]). + pub(super) coverage_ranges: usize, } +/// Most coverage ranges kept across all files (8 bytes each). +#[cfg(not(target_arch = "wasm32"))] +pub(super) const MAX_COVERAGE_RANGES: usize = 4 * 1024 * 1024; + #[cfg(all(test, target_os = "linux"))] pub(super) struct FallbackSnapshotHook { pub(super) reached: std::sync::mpsc::SyncSender<()>, diff --git a/crates/fonts/src/fontdb/names.rs b/crates/fonts/src/fontdb/names.rs index af5f14cc3..6c2610824 100644 --- a/crates/fonts/src/fontdb/names.rs +++ b/crates/fonts/src/fontdb/names.rs @@ -3,16 +3,42 @@ use super::face::FontFace; use super::system::CatalogEntry; use super::{Arc, MetadataProvider, StringId}; +/// Characters kept of one decoded name: a name record can be 64 KB, and a font has many of them, +/// kept for every face (a real family or style name is far shorter). +const MAX_NAME_CHARS: usize = 256; + +fn capped(name: impl std::fmt::Display) -> String { + use std::fmt::Write as _; + let mut out = String::new(); + // Stops decoding at the cap: the name is never built in full. + let mut room = MAX_NAME_CHARS; + struct Cap<'a>(&'a mut String, &'a mut usize); + impl std::fmt::Write for Cap<'_> { + fn write_str(&mut self, s: &str) -> std::fmt::Result { + for c in s.chars() { + if *self.1 == 0 { + return Err(std::fmt::Error); + } + *self.1 -= 1; + self.0.push(c); + } + Ok(()) + } + } + let _ = write!(Cap(&mut out, &mut room), "{name}"); + out +} + pub(super) fn aliases(font: &skrifa::FontRef<'_>, ids: &[StringId]) -> Vec { let mut names: Vec = - ids.iter().flat_map(|id| font.localized_strings(*id)).take(256).map(|name| name.to_string()).filter(|name| !name.trim().is_empty()).collect(); + ids.iter().flat_map(|id| font.localized_strings(*id)).take(256).filter(|name| !name.chars().all(char::is_whitespace)).map(capped).collect(); names.sort(); names.dedup(); names } pub(super) fn name(font: &skrifa::FontRef<'_>, ids: &[StringId]) -> Option { - ids.iter().find_map(|id| font.localized_strings(*id).english_or_first().map(|s| s.to_string()).filter(|s| !s.is_empty())) + ids.iter().find_map(|id| font.localized_strings(*id).english_or_first().map(capped).filter(|s| !s.is_empty())) } /// A face's family and style names (the default instance's style for a variable font). @@ -116,3 +142,13 @@ pub(super) fn style_italic(style: &str) -> bool { let s = norm(style); s.contains("italic") || s.contains("oblique") || s == "it" } + +#[cfg(test)] +mod tests { + #[test] + fn decoded_names_are_capped() { + assert_eq!(super::capped("Arial"), "Arial"); + let long = "é".repeat(10_000); + assert_eq!(super::capped(&long).chars().count(), super::MAX_NAME_CHARS); + } +} diff --git a/crates/fonts/src/fontdb/outline.rs b/crates/fonts/src/fontdb/outline.rs index 30e328be4..0b07f3b3f 100644 --- a/crates/fonts/src/fontdb/outline.rs +++ b/crates/fonts/src/fontdb/outline.rs @@ -11,28 +11,48 @@ pub(super) fn has_serifs(face: &FontFace) -> Option { if bbox.height().is_nan() || bbox.height() <= 1.0 { return None; } + // Flattened segments kept: an "l" has a few dozen; a hostile outline has no such limit of its own. + const MAX_EDGES: usize = 1 << 20; let mut edges = Vec::new(); + let mut over = false; let mut last = None; let mut start = None; - kurbo::flatten(&path, 1.0, |el| match el { + // A tolerance of one unit is fine for outlines of normal size; a transform that blows one up + // gets a tolerance in proportion, so the subdivision stays bounded (4096 units stay at 1). + let tol = (bbox.width().max(bbox.height()) / 4096.0).max(1.0); + if !tol.is_finite() { + return None; + } + kurbo::flatten(&path, tol, |el| match el { kurbo::PathEl::MoveTo(p) => { last = Some(p); start = Some(p); } kurbo::PathEl::LineTo(p) => { if let Some(a) = last { - edges.push((a, p)); + if edges.len() < MAX_EDGES { + edges.push((a, p)); + } else { + over = true; + } } last = Some(p); } kurbo::PathEl::ClosePath => { if let (Some(a), Some(b)) = (last, start) { - edges.push((a, b)); + if edges.len() < MAX_EDGES { + edges.push((a, b)); + } else { + over = true; + } } last = start; } _ => {} }); + if over { + return None; + } // The outline is y-down: the foot is near bbox.y1 (the baseline). let width_at = |y: f64| { let xs: Vec = edges diff --git a/crates/fonts/src/fontdb/system.rs b/crates/fonts/src/fontdb/system.rs index 2a367b894..84916807e 100644 --- a/crates/fonts/src/fontdb/system.rs +++ b/crates/fonts/src/fontdb/system.rs @@ -38,6 +38,60 @@ fn file_stamp(path: &std::path::Path) -> Option { std::fs::metadata(path).ok().map(|m| (m.len(), m.modified().ok())) } +/// The code points any face of a font file maps, as sorted inclusive ranges. +#[cfg(not(target_arch = "wasm32"))] +fn coverage_ranges(data: &[u8]) -> Vec<(u32, u32)> { + // One bit per code point: memory stays fixed however many faces (or named instances of one face) map however much. + let mut bits = vec![0u64; 0x110000 / 64]; + let mut last = None; + for (i, _, _, _) in enumerate_faces(data) { + if last.replace(i) == Some(i) { + continue; + } + if let Ok(f) = skrifa::FontRef::from_index(data, i) { + for (cp, _) in f.charmap().mappings() { + if let Some(w) = bits.get_mut((cp / 64) as usize) { + *w |= 1u64 << (cp % 64); + } + } + } + } + let mut out: Vec<(u32, u32)> = Vec::new(); + for (wi, &w) in bits.iter().enumerate() { + if w == 0 { + continue; + } + for b in 0..64u32 { + if (w >> b) & 1 == 0 { + continue; + } + let cp = wi as u32 * 64 + b; + match out.last_mut() { + Some(last) if last.1 + 1 == cp => last.1 = cp, + _ => out.push((cp, cp)), + } + } + } + out +} + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod coverage_tests { + #[test] + fn coverage_ranges_match_the_charmap() { + let data = include_bytes!("../../../../assets/fonts/SourceSans3-Regular.ttf"); + let ranges = super::coverage_ranges(data); + let f = skrifa::FontRef::new(data).unwrap(); + use skrifa::MetadataProvider as _; + let in_ranges = |c: char| ranges.iter().any(|&(lo, hi)| (lo..=hi).contains(&u32::from(c))); + assert!(in_ranges('a') && !in_ranges('\u{10FFFE}')); + for c in ['a', 'Z', 'é', '\u{4E2D}', '\u{1F600}'] { + assert_eq!(in_ranges(c), f.charmap().map(c).is_some(), "{c:?}"); + } + assert!(ranges.windows(2).all(|w| w[0].1 + 1 < w[1].0), "sorted and merged"); + } +} + /// What [`FontDb::reload_fonts`] did: the faces cataloged by the rescan, and the installed font /// files that changed or were removed since they were read (their faces are read again, from the /// new file, the next time they are looked up). @@ -272,6 +326,8 @@ impl FontDb { sys.generation = sys.generation.wrapping_add(1); sys.misses.clear(); sys.chain_tried.clear(); + sys.coverage.clear(); + sys.coverage_ranges = 0; count } @@ -564,13 +620,32 @@ impl FontDb { } } for p in paths { - if std::fs::metadata(&p).map(|m| m.len() > 40 << 20).unwrap_or(true) { + let Some(stamp) = file_stamp(&p).filter(|s| s.0 <= 40 << 20) else { continue }; + let cached = + self.sys.lock().unwrap_or_else(PoisonError::into_inner).coverage.get(&p).filter(|(s, _)| *s == stamp).map(|(_, r)| r.clone()); + let mut data = None; + let ranges = match cached { + Some(r) => r, + None => { + let Some(d) = self.read_installed(&p) else { continue }; + let r = Arc::new(coverage_ranges(&d)); + let mut sys = self.sys.lock().unwrap_or_else(PoisonError::into_inner); + if sys.coverage_ranges + r.len() <= super::fallback::MAX_COVERAGE_RANGES { + sys.coverage_ranges += r.len(); + if let Some((_, old)) = sys.coverage.insert(p.clone(), (stamp, r.clone())) { + sys.coverage_ranges -= old.len(); + } + } + data = Some(d); + r + } + }; + let cp = u32::from(c); + if !ranges.iter().any(|&(lo, hi)| (lo..=hi).contains(&cp)) { continue; } - let Some(data) = self.read_installed(&p) else { continue }; - let hit = - enumerate_faces(&data).iter().any(|(i, _, _, _)| skrifa::FontRef::from_index(&data, *i).is_ok_and(|f| f.charmap().map(c).is_some())); - if hit && self.add_faces(data, FontSource::Installed(p)) > 0 && covered(self) { + let Some(data) = data.or_else(|| self.read_installed(&p)) else { continue }; + if self.add_faces(data, FontSource::Installed(p)) > 0 && covered(self) { return true; } } diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 8f9d8856e..a03667209 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -224,10 +224,17 @@ fn read_entry( return Err(too_big()); } let mut v = Vec::new(); - if f.take(limit.saturating_add(1)).read_to_end(&mut v).is_err() { + let read = f.take(limit.saturating_add(1)).read_to_end(&mut v); + let n = v.len() as u64; + // What a failed read produced was inflated all the same: it counts against the archive's budget. + if read.is_err() { + if stored { + *stored_left = stored_left.saturating_sub(n); + } else { + *left = left.saturating_sub(n); + } return Ok(None); } - let n = v.len() as u64; if n > limit { return Err(too_big()); } diff --git a/crates/format/src/tests.rs b/crates/format/src/tests.rs index ca57c7770..0e6f613cd 100644 --- a/crates/format/src/tests.rs +++ b/crates/format/src/tests.rs @@ -3,6 +3,25 @@ use designcraft_doc::build::NewDocument; use designcraft_doc::geom::Rect; use designcraft_doc::{Asset, ParaFormat, SpreadRef}; +/// An entry that fails after inflating (a bad checksum) still uses up the archive's byte budget. +#[test] +fn a_failed_read_is_charged_to_the_budget() { + let mut z = zip::ZipWriter::new(Cursor::new(Vec::new())); + let stored = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + z.start_file("a", stored).unwrap(); + z.write_all(&[7u8; 1000]).unwrap(); + let mut bytes = z.finish().unwrap().into_inner(); + // Flip a data byte: the entry reads in full, then fails its checksum. + // The payload is the only run of 1,000 consecutive 7s in the archive. + let start = bytes.windows(1000).position(|w| w.iter().all(|b| *b == 7)).expect("payload"); + bytes[start] = 8; + let mut archive = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap(); + let (mut left, mut stored_left) = (5000, 5000); + assert_eq!(read_entry(&mut archive, "a", 4000, &mut left, &mut stored_left).unwrap(), None); + // A failed stored read is charged to the stored budget only. + assert_eq!((left, stored_left), (5000, 4000)); +} + fn png_asset(d: &mut Document, data: &[u8]) -> AssetId { let aid = AssetId(d.alloc()); let a = diff --git a/crates/idml/src/export.rs b/crates/idml/src/export.rs index 717bb5d6c..fccbc7c2f 100644 --- a/crates/idml/src/export.rs +++ b/crates/idml/src/export.rs @@ -65,9 +65,20 @@ pub fn export_idml(doc: &Document) -> Vec { /// Export with options, using the index sort-key conflict policy of [`export_idml`]. pub fn export_idml_with(doc: &Document, opts: &ExportOptions) -> Vec { + export_idml_report(doc, opts).0 +} + +/// [`export_idml_with`] and its warnings: an image written as a link only because the export's +/// embedded-image limit was reached is named in one, so the package is not silently incomplete. +pub fn export_idml_report(doc: &Document, opts: &ExportOptions) -> (Vec, Vec) { + export_with_limit(doc, opts, MAX_EMBEDDED_BYTES) +} + +pub(crate) fn export_with_limit(doc: &Document, opts: &ExportOptions, embed_limit: usize) -> (Vec, Vec) { let mut ex = Ex::new(doc, opts); + ex.embed_limit = embed_limit; let parts = ex.build(); - zip_parts(&parts) + (zip_parts(&parts), ex.warnings) } fn zip_parts(parts: &[(String, Vec)]) -> Vec { @@ -106,8 +117,18 @@ struct Ex<'a> { /// Index topic path → first serialized sort key; See / See also cross-references. index_topics: BTreeMap, String>, index_xrefs: Vec<(Vec, &'static str, String)>, + /// Bytes of image data written into `` so far (see [`MAX_EMBEDDED_BYTES`]). + embedded: usize, + embed_limit: usize, + /// Images written as links because the embed limit was reached (one line per placement). + warnings: Vec, } +/// Image bytes one export embeds, counting each placement (IDML holds a copy per placement): past +/// it a graphic that has a link of its own is written as that link without its contents (an +/// image with no link is always embedded: it has nowhere else to live). +const MAX_EMBEDDED_BYTES: usize = 512 * 1024 * 1024; + impl<'a> Ex<'a> { fn new(d: &'a Document, opts: &'a ExportOptions) -> Self { let mut ex = Ex { @@ -121,6 +142,9 @@ impl<'a> Ex<'a> { link_text_sources: Vec::new(), index_topics: BTreeMap::new(), index_xrefs: Vec::new(), + embedded: 0, + embed_limit: MAX_EMBEDDED_BYTES, + warnings: Vec::new(), }; ex.assign_swatch_ids(); for s in d.stories.values() { diff --git a/crates/idml/src/export/item.rs b/crates/idml/src/export/item.rs index 457172253..ca27a8ca0 100644 --- a/crates/idml/src/export/item.rs +++ b/crates/idml/src/export/item.rs @@ -268,9 +268,19 @@ impl<'a> Ex<'a> { el.set("FillColor", id.clone()); el.set("FillTint", pct(g.fill.tint as f64)); } - let embed = !asset.data.is_empty() && (self.opts.embed_images || asset.link.is_none()); + let wanted = !asset.data.is_empty() && (self.opts.embed_images || asset.link.is_none()); + // An image with no link of its own has nowhere else to live, so it is always embedded + // (dropping its data would lose it); only an image with a real link can fall back to it. + let embed = wanted && (asset.link.is_none() || self.embedded.saturating_add(asset.data.len()) <= self.embed_limit); + if wanted && !embed { + self.warnings.push(format!( + "image {} is written as a link, not embedded: the package already holds {} bytes of images (limit {})", + asset.name, self.embedded, self.embed_limit + )); + } let mut props = Vec::new(); if embed { + self.embedded = self.embedded.saturating_add(asset.data.len()); let mut c = El::new("Contents"); c.children.push(Node::CData(base64_encode(&asset.data))); props.push(c); diff --git a/crates/idml/src/export/story.rs b/crates/idml/src/export/story.rs index 437376190..7ca41e231 100644 --- a/crates/idml/src/export/story.rs +++ b/crates/idml/src/export/story.rs @@ -278,7 +278,14 @@ impl<'a> Ex<'a> { None } - fn apply_tracked_changes(&mut self, s: &Story, f: &CharFormat, csrs: &mut [El], para_br: bool, para_end: usize) { + fn apply_tracked_changes( + &mut self, + f: &CharFormat, + csrs: &mut [El], + para_br: bool, + runs: &[(std::ops::Range, &CharFormat)], + para_end: usize, + ) { // Tracked changes wrap the text they mark. if let Some(kind) = change_type(f.over.change) { for c in csrs.iter_mut().filter(|c| c.name == "CharacterStyleRange" && !c.children.is_empty()) { @@ -288,7 +295,10 @@ impl<'a> Ex<'a> { } } if para_br && let Some(c) = csrs.last_mut() { - let nl = s.runs().find(|(rr, _)| rr.contains(¶_end)).and_then(|(_, nf)| change_type(nf.over.change)); + let nl = runs + .get(runs.partition_point(|(rr, _)| rr.end <= para_end)) + .filter(|(rr, _)| rr.start <= para_end) + .and_then(|(_, nf)| change_type(nf.over.change)); match nl { None => c.push(El::new("Br")), Some(kind) => match c.children.last_mut() { @@ -307,17 +317,27 @@ impl<'a> Ex<'a> { pub(super) fn story_paras(&mut self, s: &Story) -> Vec { let ranges = s.para_ranges(); let n = ranges.len(); - ranges.into_iter().enumerate().map(|(pi, r)| self.story_para(s, pi, r, pi + 1 == n)).collect() + // The runs and the marker offsets, once: each paragraph (and each marker) searched them from the start. + let runs: Vec<(std::ops::Range, &CharFormat)> = s.runs().collect(); + let marks = Marks::of(s); + ranges.into_iter().enumerate().map(|(pi, r)| self.story_para(s, pi, r, pi + 1 == n, &runs, &marks)).collect() } - fn paragraph_runs(s: &Story, r: &std::ops::Range) -> Vec<(std::ops::Range, CharFormat)> { - // Character runs intersecting the paragraph. + fn paragraph_runs( + s: &Story, + r: &std::ops::Range, + runs: &[(std::ops::Range, &CharFormat)], + ) -> Vec<(std::ops::Range, CharFormat)> { + // Character runs intersecting the paragraph (they ascend: the first is a search away). let mut segs: Vec<(std::ops::Range, CharFormat)> = Vec::new(); - for (rr, f) in s.runs() { + for (rr, f) in &runs[runs.partition_point(|(rr, _)| rr.end <= r.start)..] { + if rr.start >= r.end { + break; + } let a = rr.start.max(r.start); let b = rr.end.min(r.end); if a < b { - segs.push((a..b, f.clone())); + segs.push((a..b, (*f).clone())); } } if segs.is_empty() { @@ -354,11 +374,19 @@ impl<'a> Ex<'a> { } } - fn story_para(&mut self, s: &Story, pi: usize, r: std::ops::Range, last: bool) -> El { + fn story_para( + &mut self, + s: &Story, + pi: usize, + r: std::ops::Range, + last: bool, + runs: &[(std::ops::Range, &CharFormat)], + marks: &Marks, + ) -> El { let pf = s.paras.get(pi).cloned().unwrap_or_default(); let mut psr = self.psr_el(&pf); let ends_with_break = s.text.get(r.clone()).and_then(|t| t.chars().next_back()).is_some_and(st::is_break_char); - let segs = Self::paragraph_runs(s, &r); + let segs = Self::paragraph_runs(s, &r, runs); // A hyperlink on a run of text is written as a `HyperlinkTextSource` around the // ranges it covers, so the runs are cut at its edges first (#1446). let (links, segs) = self.cut_runs_at_links(s, &r, segs); @@ -414,7 +442,7 @@ impl<'a> Ex<'a> { } else if ch == designcraft_doc::ANCHOR_MARK { flush_text(&mut cur, &mut pending); flush_content(&mut pending, &mut out); - let k = s.text[..seg_start + ci].matches(designcraft_doc::ANCHOR_MARK).count(); + let k = marks.anchor.partition_point(|&at| at < seg_start + ci); if let Some(a) = s.anchors.get(k) { out.push(Node::El( El::new("HyperlinkTextDestination") @@ -428,14 +456,14 @@ impl<'a> Ex<'a> { // An anchored object: the item inside the range with its anchoring settings. flush_text(&mut cur, &mut pending); flush_content(&mut pending, &mut out); - let k = s.text[..seg_start + ci].matches(designcraft_doc::OBJECT_MARK).count(); + let k = marks.object.partition_point(|&at| at < seg_start + ci); if let Some(el) = self.anchored_object(s, k) { out.push(Node::El(el)); } } else if ch == designcraft_doc::INDEX_MARK { flush_text(&mut cur, &mut pending); flush_content(&mut pending, &mut out); - let k = s.text[..seg_start + ci].matches(designcraft_doc::INDEX_MARK).count(); + let k = marks.index.partition_point(|&at| at < seg_start + ci); if let Some(el) = self.index_reference(s, k) { out.push(Node::El(el)); } @@ -448,7 +476,7 @@ impl<'a> Ex<'a> { c.children.append(&mut out); csrs.push(c); } - let k = s.text[..seg_start + ci].matches(designcraft_doc::XREF_MARK).count(); + let k = marks.xref.partition_point(|&at| at < seg_start + ci); if let Some(x) = s.xrefs.get(k) { let me = self.fresh(); let fmt = self.d.xref_formats.iter().position(|ff| ff.name == x.format).map_or("n".to_string(), xref_format_self); @@ -525,7 +553,7 @@ impl<'a> Ex<'a> { c.children.extend(out); csrs.push(c); } - self.apply_tracked_changes(s, &f, &mut csrs, para_br, r.end); + self.apply_tracked_changes(&f, &mut csrs, para_br, runs, r.end); let link = links.iter().find(|(_, l)| l.contains(&seg_start) || (l.start == seg_start && l.start == l.end)).map(|(i, _)| *i); for c in csrs { psr_csrs.push((link, c)); @@ -655,3 +683,23 @@ pub(super) fn with_list_tab(mut a: ParaAttrs, resolved: impl FnOnce() -> (design a.tabs = Some(tabs); a } + +/// Byte offsets of a story's anchor, object, index and cross-reference markers, in text order. +pub(super) struct Marks { + anchor: Vec, + object: Vec, + index: Vec, + xref: Vec, +} + +impl Marks { + fn of(s: &Story) -> Self { + let at = |c: char| s.text.match_indices(c).map(|(i, _)| i).collect::>(); + Marks { + anchor: at(designcraft_doc::ANCHOR_MARK), + object: at(designcraft_doc::OBJECT_MARK), + index: at(designcraft_doc::INDEX_MARK), + xref: at(designcraft_doc::XREF_MARK), + } + } +} diff --git a/crates/idml/src/import/object_style.rs b/crates/idml/src/import/object_style.rs index 343e67211..6af6f845e 100644 --- a/crates/idml/src/import/object_style.rs +++ b/crates/idml/src/import/object_style.rs @@ -242,9 +242,11 @@ fn overlay(base: &mut El, top: &El, depth: usize) { *base = top.clone(); return; } + // Where `base`'s Properties child is, once (a search per attribute otherwise). + let props = base.children.iter().position(|n| matches!(n, Node::El(p) if p.local() == "Properties")); for (k, v) in &top.attrs { base.set(k, v); - if let Some(Node::El(p)) = base.children.iter_mut().find(|n| matches!(n, Node::El(p) if p.local() == "Properties")) { + if let Some(Node::El(p)) = props.and_then(|at| base.children.get_mut(at)) { p.children.retain(|n| !matches!(n, Node::El(x) if x.local() == k)); } } @@ -259,22 +261,26 @@ fn overlay(base: &mut El, top: &El, depth: usize) { // The k-th child of a name merges with the k-th child of that name in `base` (repeated elements // such as gradient stops pair up in order). let mut seen: HashMap<&str, usize> = HashMap::new(); + // Positions in `base.children` of its elements, by name, in order: the k-th is a lookup, not a + // scan from the first child (many repeated names made the scans quadratic). + let mut at: HashMap> = HashMap::new(); + for (i, b) in base.children.iter().enumerate() { + if let Node::El(b) = b { + at.entry(b.local().to_string()).or_default().push(i); + } + } for n in &top.children { match n { Node::El(c) => { let k = seen.entry(c.local()).or_insert(0); - let slot = base - .children - .iter_mut() - .filter_map(|b| match b { - Node::El(b) if b.local() == c.local() => Some(b), - _ => None, - }) - .nth(*k); + let slot = at.get(c.local()).and_then(|v| v.get(*k)).copied(); *k += 1; - match slot { - Some(b) => overlay(b, c, depth + 1), - None => base.children.push(n.clone()), + match slot.and_then(|i| base.children.get_mut(i)) { + Some(Node::El(b)) => overlay(b, c, depth + 1), + _ => { + at.entry(c.local().to_string()).or_default().push(base.children.len()); + base.children.push(n.clone()); + } } } Node::Text(_) | Node::CData(_) if has_text => base.children.push(n.clone()), diff --git a/crates/idml/src/lib.rs b/crates/idml/src/lib.rs index 8f7b22ff9..0c024dfab 100644 --- a/crates/idml/src/lib.rs +++ b/crates/idml/src/lib.rs @@ -57,7 +57,7 @@ mod xml; #[cfg(test)] mod tests; -pub use export::{ExportOptions, export_idml, export_idml_with}; +pub use export::{ExportOptions, export_idml, export_idml_report, export_idml_with}; pub use import::{Imported, import_idml, import_idml_report, import_idml_report_with_previews, import_idml_with}; /// The IDML package mimetype (content of the first, stored `mimetype` entry). diff --git a/crates/idml/src/tests/assets.rs b/crates/idml/src/tests/assets.rs index 48875c256..265183540 100644 --- a/crates/idml/src/tests/assets.rs +++ b/crates/idml/src/tests/assets.rs @@ -393,3 +393,42 @@ fn the_swatch_applied_to_an_image_is_kept() { assert!(!images.iter().any(|e| e.contains("Swatch/None")), "{images:?}"); assert_eq!(fills(&import_idml_with(&out, &reader).unwrap()), want); } + +/// Past the embedded-image limit an export still succeeds: the images that no longer fit are +/// written as links, and each is named in a warning. +#[test] +fn export_past_the_embed_limit_warns_for_each_image_written_as_a_link() { + let bytes = vec![42u8; 300]; + let reader = |p: &str| p.ends_with("couple.jpg").then(|| bytes.clone()); + let frames = vec![ + frame(1, 0.0, 1.0, &image(1, PHOTO, None)), + frame(2, 100.0, 2.0, &image(2, PHOTO, None)), + frame(3, 200.0, 3.0, &image(3, PHOTO, None)), + frame(4, 300.0, 4.0, &image(4, "file:/elsewhere/copy.jpg", Some(&bytes))), + ]; + let d = import_idml_with(&package(&frames), &reader).unwrap(); + let opts = ExportOptions { embed_images: true }; + // Four placements of 300 bytes each; the limit holds two. + let (out, warnings) = crate::export::export_with_limit(&d, &opts, 700); + assert_eq!(warnings.len(), 2, "{warnings:?}"); + assert!(warnings.iter().all(|w| w.contains("written as a link")), "{warnings:?}"); + assert_eq!(import_idml_with(&out, &reader).unwrap().assets.len(), 2); + assert!(crate::export::export_with_limit(&d, &opts, 1200).1.is_empty()); +} + +/// An image with no link of its own is embedded even past the limit: it has no other home, and +/// dropping its data would lose it. +#[test] +fn an_unlinked_image_is_embedded_past_the_embed_limit() { + let bytes = vec![42u8; 300]; + let reader = |p: &str| p.ends_with("couple.jpg").then(|| bytes.clone()); + let frames = vec![frame(1, 0.0, 1.0, &image(1, PHOTO, None)), frame(2, 100.0, 2.0, &image(2, PHOTO, None))]; + let mut d = import_idml_with(&package(&frames), &reader).unwrap(); + for a in d.assets.values_mut() { + std::sync::Arc::make_mut(a).link = None; + } + let (out, warnings) = crate::export::export_with_limit(&d, &ExportOptions { embed_images: true }, 100); + assert!(warnings.is_empty(), "{warnings:?}"); + let back = import_idml_with(&out, &|_: &str| None).unwrap(); + assert!(!back.assets.is_empty() && back.assets.values().all(|a| a.data.as_slice() == bytes.as_slice())); +} diff --git a/crates/images/src/lib.rs b/crates/images/src/lib.rs index 83c540e6e..98c058aa6 100644 --- a/crates/images/src/lib.rs +++ b/crates/images/src/lib.rs @@ -433,9 +433,32 @@ pub fn decode_rgba(bytes: &[u8]) -> Option { to_display_rgba(px, Some(&icc), &DisplayColour::active()) } +/// False when `bytes` is a WebP whose `ICCP` chunk declares more bytes than the file holds or than +/// [`colour::MAX_ICC_BYTES`]. The decoder sizes its buffer from the declared length before it reads +/// anything, so such a file must not reach `icc_profile()`; its pixels decode without it. +pub fn webp_icc_is_sane(bytes: &[u8]) -> bool { + if bytes.get(0..4) != Some(b"RIFF") || bytes.get(8..12) != Some(b"WEBP") { + return true; + } + let mut pos = 12usize; + while let Some(head) = bytes.get(pos..pos.saturating_add(8)) { + let size = u32::from_le_bytes([head[4], head[5], head[6], head[7]]) as usize; + let body = pos + 8; + if &head[..4] == b"ICCP" { + return size <= colour::MAX_ICC_BYTES && body.saturating_add(size) <= bytes.len(); + } + // Chunks are padded to an even size. + pos = body.saturating_add(size).saturating_add(size & 1); + } + true +} + /// The ICC profile embedded in a PNG, JPEG, TIFF or WebP. fn embedded_icc(bytes: &[u8]) -> Option> { use image::ImageDecoder; + if !webp_icc_is_sane(bytes) { + return None; + } let mut d = image::ImageReader::new(std::io::Cursor::new(bytes)).with_guessed_format().ok()?.into_decoder().ok()?; d.icc_profile().ok().flatten() } diff --git a/crates/images/src/tests.rs b/crates/images/src/tests.rs index 3e5daf8ab..cd577f09a 100644 --- a/crates/images/src/tests.rs +++ b/crates/images/src/tests.rs @@ -534,3 +534,30 @@ fn gif_tone_and_cmyk_inks_check_the_rgba_budget() { assert!(too_large_to_decode(&tiff)); assert_eq!(decode_cmyk_tiff(&tiff), None); } + +fn webp_with(chunks: &[(&[u8; 4], u32, &[u8])]) -> Vec { + let mut v = b"RIFF\0\0\0\0WEBP".to_vec(); + for (id, declared, body) in chunks { + v.extend_from_slice(*id); + v.extend_from_slice(&declared.to_le_bytes()); + v.extend_from_slice(body); + if body.len() % 2 == 1 { + v.push(0); + } + } + v +} + +/// A WebP that declares an `ICCP` chunk larger than the file (or than the ICC cap) never reaches the +/// decoder's profile read, which would size a buffer from the declared length. +#[test] +fn a_webp_icc_chunk_that_declares_more_than_it_holds_is_refused_before_decoding() { + assert!(webp_icc_is_sane(&webp_with(&[(b"VP8X", 10, &[0; 10]), (b"ICCP", 4, &[1; 4])]))); + // An odd-sized chunk before it is padded to even. + assert!(webp_icc_is_sane(&webp_with(&[(b"EXIF", 3, &[0; 3]), (b"ICCP", 4, &[1; 4])]))); + assert!(!webp_icc_is_sane(&webp_with(&[(b"VP8X", 10, &[0; 10]), (b"ICCP", u32::MAX, &[1; 4])]))); + assert!(!webp_icc_is_sane(&webp_with(&[(b"ICCP", 100, &[1; 4])]))); + assert!(embedded_icc(&webp_with(&[(b"ICCP", u32::MAX, &[1; 4])])).is_none()); + assert!(webp_icc_is_sane(b"\x89PNG not a webp")); + assert!(webp_icc_is_sane(&webp_with(&[(b"VP8L", 6, &[0; 6])]))); +} diff --git a/crates/pdf/src/export/images.rs b/crates/pdf/src/export/images.rs index ec7c76fd6..b50c6b20a 100644 --- a/crates/pdf/src/export/images.rs +++ b/crates/pdf/src/export/images.rs @@ -47,6 +47,9 @@ pub(super) struct RasterMode { /// The ICC profile embedded in an image file, if it has one. pub(super) fn embedded_icc(data: &[u8]) -> Option> { use image::ImageDecoder; + if !designcraft_images::webp_icc_is_sane(data) { + return None; + } let mut dec = image::ImageReader::new(std::io::Cursor::new(data)).with_guessed_format().ok()?.into_decoder().ok()?; dec.icc_profile().ok().flatten() } @@ -89,6 +92,9 @@ pub(super) fn jpeg(data: krilla::Data, icc: impl FnOnce() -> Option>, mo /// its own. fn untagged_rgb(data: &[u8]) -> bool { use image::ImageDecoder; + if !designcraft_images::webp_icc_is_sane(data) { + return false; + } let Some(mut dec) = image::ImageReader::new(std::io::Cursor::new(data)).with_guessed_format().ok().and_then(|r| r.into_decoder().ok()) else { return false; }; diff --git a/crates/pptx/src/fonts.rs b/crates/pptx/src/fonts.rs index 27c976aaf..ed02ff297 100644 --- a/crates/pptx/src/fonts.rs +++ b/crates/pptx/src/fonts.rs @@ -15,6 +15,9 @@ pub struct RunFont { pub italic: bool, } +/// Faces whose glyph-to-character map is kept at once. +const MAX_REVERSE_MAPS: usize = 4; + #[derive(Default)] pub struct Fonts { names: HashMap, @@ -116,6 +119,11 @@ impl Fonts { /// The character a glyph shows (the lowest code point mapping to it). pub fn char_of(&mut self, face: &FontFace, gid: u32) -> Option { + // A face's reverse map can hold over a million entries; keeping one per face a bundle uses + // is unbounded, so only the last few stay (a rebuilt map answers the same). + if !self.reverse.contains_key(&face.id()) && self.reverse.len() >= MAX_REVERSE_MAPS { + self.reverse.clear(); + } let map = self.reverse.entry(face.id()).or_insert_with(|| { let mut m = HashMap::new(); for (c, g) in face.chars() { @@ -216,3 +224,26 @@ pub fn eot(face: &FontFace) -> Result, NoEmbed> { h.extend_from_slice(data); Ok(h) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_a_few_reverse_maps_are_kept_and_answers_do_not_change() { + let db = designcraft_fonts::FontDb::global(); + let mut fonts = Fonts::default(); + let mut last = None; + for style in ["Regular", "Bold", "Italic", "Bold Italic", "Semibold", "Light", "Black", "ExtraLight", "Medium", "SemiBold Italic"] { + let face = db.face("Source Serif 4", style); + let gid = face.glyph_for('A'); + assert_eq!(fonts.char_of(&face, gid), Some('A'), "{style}"); + assert!(fonts.reverse.len() <= MAX_REVERSE_MAPS); + last = Some((face, gid)); + } + // The map evicted earlier is rebuilt on demand with the same answer. + if let Some((face, gid)) = last { + assert_eq!(fonts.char_of(&face, gid), Some('A')); + } + } +} diff --git a/crates/textimport/src/export.rs b/crates/textimport/src/export.rs index 2493f7f98..8d38780d1 100644 --- a/crates/textimport/src/export.rs +++ b/crates/textimport/src/export.rs @@ -20,33 +20,35 @@ fn plain_char(c: char) -> Option<&'static str> { } } -fn table_text(story: &Story, id: u64, nl: &str) -> String { +/// Cells of table `id` as rows of tab-separated cell text (the cells' visible text, nested +/// tables and notes left out), `nl` between rows. +fn table_text(doc: &Document, story: &Story, id: u64, nl: &str) -> String { let Some(t) = story.tables.get(&id) else { return String::new() }; (0..t.nrows()) - .map(|r| (0..t.ncols()).map(|c| t.cell(r, c).map(|x| x.text.text.replace('\n', " ")).unwrap_or_default()).collect::>().join("\t")) + .map(|r| { + (0..t.ncols()) + .map(|c| t.cell(r, c).map(|x| plain_text(doc, &x.text).replace("\r\n", " ").replace('\n', " ")).unwrap_or_default()) + .collect::>() + .join("\t") + }) .collect::>() .join(nl) } -/// Text Only: the story's text, CRLF between paragraphs. -pub fn plain_text(story: &Story) -> String { - // Tracked deletions aren't part of the text. - let deleted: Vec> = - story.runs().filter(|(_, f)| f.over.change == Some(designcraft_doc::ChangeMark::Deleted)).map(|(r, _)| r).collect(); +/// Text Only: the story's text as it shows (hidden conditional text and tracked deletions left +/// out), CRLF between paragraphs. +pub fn plain_text(doc: &Document, story: &Story) -> String { let mut out = String::with_capacity(story.text.len() + 16); + let spans = doc.visible_spans(story); for (i, r) in story.para_ranges().into_iter().enumerate() { if i > 0 { out.push_str("\r\n"); } - let start = r.start; - for (ci, c) in story.text[r].char_indices() { - if deleted.iter().any(|d| d.contains(&(start + ci))) { - continue; - } + for c in Document::visible_text(story, &spans, r).chars() { match (c, plain_char(c)) { (TABLE_ANCHOR, _) => { if let Some(id) = story.paras.get(i).and_then(|p| p.table) { - out.push_str(&table_text(story, id, "\r\n")); + out.push_str(&table_text(doc, story, id, "\r\n")); } } (_, Some("")) => out.push(c), @@ -178,7 +180,7 @@ pub fn rtf(doc: &Document, story: &Story) -> String { match c { TABLE_ANCHOR => { if let Some(id) = table { - for ch in table_text(story, *id, "\u{2028}").chars() { + for ch in table_text(doc, story, *id, "\u{2028}").chars() { rtf_escape(&mut out, ch); } } @@ -214,14 +216,42 @@ mod tests { #[test] fn text_variable_markers_are_dropped() { let st = Story::with_text(StoryId(1), "Page \u{E100} of", Default::default()); - assert_eq!(plain_text(&st), "Page of"); + assert_eq!(plain_text(&Document::new(&NewDocument::default()), &st), "Page of"); let r = rtf(&Document::new(&NewDocument::default()), &st); assert!(!r.contains("\\u-7936") && r.contains("Page of"), "{r}"); } #[test] fn text_only() { - assert_eq!(plain_text(&story()), "Plain bold {x}\r\nCafé\ntwo"); + assert_eq!(plain_text(&Document::new(&NewDocument::default()), &story()), "Plain bold {x}\r\nCafé\ntwo"); + } + + /// Text hidden by a condition set on the paragraph style, and text marked deleted, don't + /// reach Text Only: not in the story and not in a table cell. + #[test] + fn text_only_leaves_out_hidden_and_deleted_text() { + let mut doc = Document::new(&NewDocument::default()); + doc.conditions.push(designcraft_doc::Condition { name: "Secret".into(), color: [0, 0, 0], visible: false }); + doc.styles_mut().paragraph.push(designcraft_doc::ParagraphStyle { + name: "Hidden".into(), + based_on: None, + next_style: None, + para: Default::default(), + chars: CharAttrs { conditions: Some(vec!["Secret".into()]), ..Default::default() }, + shortcut: String::new(), + }); + let mut st = Story::with_text(StoryId(1), "shown\nhidden by style\nkept gone", Default::default()); + let h = st.text.find("hidden").unwrap(); + st.format_paras(h..h + 1, |p| p.style = "Hidden".into()); + let g = st.text.find(" gone").unwrap(); + st.format_chars(g..g + 5, |f| f.over = CharAttrs { change: Some(designcraft_doc::ChangeMark::Deleted), ..Default::default() }); + assert_eq!(plain_text(&doc, &st), "shown\r\n\r\nkept"); + let mut outer = Story::with_text(StoryId(2), &TABLE_ANCHOR.to_string(), Default::default()); + let mut t = designcraft_doc::Table::new(7, 1, 1, 0, 0, 100.0); + t.cells[0].text = st.clone(); + outer.tables.insert(7, std::sync::Arc::new(t)); + outer.paras[0].table = Some(7); + assert_eq!(plain_text(&doc, &outer), "shown kept"); } #[test] diff --git a/crates/textimport/src/tagged.rs b/crates/textimport/src/tagged.rs index c3668614d..572c732c8 100644 --- a/crates/textimport/src/tagged.rs +++ b/crates/textimport/src/tagged.rs @@ -74,13 +74,15 @@ pub fn tagged_text(doc: &Document, story: &Story) -> String { out.push_str("\r\n"); } let pf = story.paras.get(i).cloned().unwrap_or_default(); + let base = doc.styles.resolve_para(&pf).1; let _ = write!(out, "", escape_value(&pf.style)); for (rr, f) in story.runs() { let (a, b) = (rr.start.max(r.start), rr.end.min(r.end)); if a >= b { continue; } - if doc.conditions_hide(f.over.conditions.as_deref().unwrap_or(&[])) || f.over.change == Some(designcraft_doc::ChangeMark::Deleted) { + // Hidden conditional text (the styles' conditions included) and deletions aren't exported. + if doc.hides_run(&base, f) { continue; } let mut open = Vec::new(); diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 19f8557ad..0ce6b1974 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -273,6 +273,30 @@ fn dir_bytes(dir: &std::path::Path) -> u64 { total } +/// Whether the file bytes of the tar.gz at `tgz` add up to at most `max`, found by streaming them +/// through `tar -xzOf` and counting: nothing is written to disk, and a decompression bomb is cut +/// off (the child is killed) as soon as it passes `max`. +fn expands_within(tgz: &Path, max: u64) -> Result { + use std::io::Read; + let mut child = Command::new("tar") + .arg("-xzOf") + .arg(tgz) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .map_err(|e| format!("tar -xzOf: failed to spawn: {e}"))?; + let Some(out) = child.stdout.take() else { return Err("tar -xzOf: no stdout".into()) }; + let n = std::io::copy(&mut out.take(max.saturating_add(1)), &mut std::io::sink()).map_err(|e| format!("tar -xzOf: {e}"))?; + if n > max { + let _ = child.kill(); + let _ = child.wait(); + return Ok(false); + } + // Within the cap only counts when tar read the whole archive: an early exit proves nothing. + let status = child.wait().map_err(|e| format!("tar -xzOf: {e}"))?; + if status.success() { Ok(true) } else { Err(format!("tar -xzOf {} failed ({status}); archive refused", tgz.display())) } +} + fn cmd_corpus(download: bool) -> Result<(), String> { let corpus = root().join("corpus"); println!( @@ -309,6 +333,11 @@ Tests that use a corpus skip cleanly when it is absent. if !listing.status.success() || listing.stdout.iter().filter(|b| **b == b'\n').count() > PNGSUITE_MAX_ENTRIES { return refuse(format!("the PngSuite download isn't a tar.gz of at most {PNGSUITE_MAX_ENTRIES} entries")); } + match expands_within(&tgz, PNGSUITE_MAX_EXTRACTED) { + Ok(true) => {} + Ok(false) => return refuse(format!("the PngSuite archive expands to more than {PNGSUITE_MAX_EXTRACTED} bytes; nothing was extracted")), + Err(e) => return refuse(e), + } let mut tar = Command::new("tar"); tar.arg("-xzf").arg(&tgz).arg("-C").arg(&dest); if let Err(e) = run(tar, "tar -xzf PngSuite-2017jul19.tgz") { @@ -324,3 +353,26 @@ Tests that use a corpus skip cleanly when it is absent. println!("PngSuite: {n} PNG files in {}", dest.display()); if n == 0 { Err("no PNG files extracted".into()) } else { Ok(()) } } + +#[cfg(test)] +mod corpus_tests { + use super::*; + + #[test] + fn a_tarball_that_expands_past_the_cap_is_refused_without_extracting() { + let dir = std::env::temp_dir().join(format!("dc-xtask-corpus-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("big"), vec![0u8; 100_000]).unwrap(); + let tgz = dir.join("a.tgz"); + let made = Command::new("tar").arg("-czf").arg(&tgz).arg("-C").arg(&dir).arg("big").status(); + if made.is_ok_and(|s| s.success()) { + assert_eq!(expands_within(&tgz, 100_000), Ok(true)); + assert_eq!(expands_within(&tgz, 99_999), Ok(false)); + // A truncated archive makes tar fail: not a within-cap answer. + let bytes = std::fs::read(&tgz).unwrap(); + std::fs::write(&tgz, &bytes[..bytes.len() / 2]).unwrap(); + assert!(expands_within(&tgz, 1 << 20).is_err()); + } + let _ = std::fs::remove_dir_all(&dir); + } +}