mirror of
https://github.com/storytold/designcraft.git
synced 2026-10-24 10:16:59 -06:00
Never crash: doc and compose
expect() calls become let-else / fallbacks. A parent spread without pages (possible in a .designcraft file) no longer underflows in parent_page_for, and check() rejects a next_id so large that allocating ids would overflow. Both crates deny the panicking lints. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
192b72054d
commit
a539c76632
@@ -0,0 +1,4 @@
|
||||
allow-unwrap-in-tests = true
|
||||
allow-expect-in-tests = true
|
||||
allow-panic-in-tests = true
|
||||
allow-indexing-slicing-in-tests = true
|
||||
@@ -7,6 +7,7 @@
|
||||
//! breaks, vertical justification) → overset detection.
|
||||
//!
|
||||
//! Output coordinates are each frame's *inner* space (apply the frame item's `xf` for spread space).
|
||||
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
|
||||
#![forbid(unsafe_code)]
|
||||
|
||||
pub mod breaker;
|
||||
@@ -2162,13 +2163,14 @@ pub fn path_glyphs(
|
||||
};
|
||||
let m =
|
||||
place * designcraft_geom::Affine::translate((g.x, l.baseline + g.y)) * skew * designcraft_geom::Affine::scale_non_uniform(g.sx, g.sy);
|
||||
let bp = match runs.iter_mut().find(|r| r.0 == g.style) {
|
||||
Some(r) => &mut r.1,
|
||||
let k = match runs.iter().position(|r| r.0 == g.style) {
|
||||
Some(k) => k,
|
||||
None => {
|
||||
runs.push((g.style, designcraft_geom::BezPath::new()));
|
||||
&mut runs.last_mut().expect("pushed").1
|
||||
runs.len() - 1
|
||||
}
|
||||
};
|
||||
let bp = &mut runs[k].1;
|
||||
bp.extend((m * outline.as_ref().clone()).elements().iter().copied());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -339,7 +339,7 @@ fn emit(
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let lay = layouts[r * nc + c].as_ref().expect("owner composed");
|
||||
let Some(lay) = layouts.get(r * nc + c).and_then(Option::as_ref) else { continue };
|
||||
let clip = Rect::new(rect.x0 + cell.insets[1], rect.y0 + cell.insets[0], rect.x1 - cell.insets[3], rect.y1 - cell.insets[2]);
|
||||
let space = clip.height() - lay.content_h;
|
||||
let dy = match cell.vj {
|
||||
|
||||
+14
-1
@@ -544,7 +544,8 @@ impl Document {
|
||||
let page = self.page(abs)?;
|
||||
let pi = self.parent_index(page.parent?)?;
|
||||
let ps = &self.parents[pi];
|
||||
let idx = if ps.pages.len() >= 2 && page.side == PageSide::Left { 0 } else { ps.pages.len() - 1 };
|
||||
// A parent spread can come without pages from a file.
|
||||
let idx = if ps.pages.len() >= 2 && page.side == PageSide::Left { 0 } else { ps.pages.len().checked_sub(1)? };
|
||||
Some((pi, idx))
|
||||
}
|
||||
}
|
||||
@@ -567,6 +568,18 @@ mod tests {
|
||||
d.check().unwrap();
|
||||
}
|
||||
|
||||
/// A document whose parent spread has no pages passes `check()`; showing the parent behind a
|
||||
/// page used to compute index `0 - 1`.
|
||||
#[test]
|
||||
fn parent_without_pages_is_not_shown() {
|
||||
let mut d = Document::new(&NewDocument { pages: 2, ..Default::default() });
|
||||
assert!(d.parent_page_for(0).is_some());
|
||||
Arc::make_mut(&mut d.parents[0]).pages.clear();
|
||||
d.check().unwrap();
|
||||
assert_eq!(d.parent_page_for(0), None);
|
||||
assert_eq!(d.parent_page_for(1), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_facing_one_page_per_spread() {
|
||||
let d = Document::new(&NewDocument { pages: 3, facing_pages: false, ..Default::default() });
|
||||
|
||||
@@ -148,7 +148,8 @@ impl Document {
|
||||
for &i in &loc.path[1..loc.path.len() - 1] {
|
||||
it = Arc::make_mut(&mut it.children_mut().ok_or(DocError::NoItem(id))?[i]);
|
||||
}
|
||||
it.children_mut().ok_or(DocError::NoItem(id))?.remove(*loc.path.last().expect("non-empty"))
|
||||
let last = *loc.path.last().ok_or(DocError::NoItem(id))?;
|
||||
it.children_mut().ok_or(DocError::NoItem(id))?.remove(last)
|
||||
}
|
||||
};
|
||||
let mut frames = Vec::new();
|
||||
|
||||
@@ -176,7 +176,8 @@ impl crate::Document {
|
||||
if cur.text == fresh.text && cur.paras == fresh.paras && cur.chars == fresh.chars {
|
||||
return;
|
||||
}
|
||||
let st = Arc::make_mut(self.stories.get_mut(&sid).expect("checked"));
|
||||
let Some(st) = self.stories.get_mut(&sid) else { return };
|
||||
let st = Arc::make_mut(st);
|
||||
let (frames, rev) = (std::mem::take(&mut st.frames), st.rev);
|
||||
*st = Story { frames, rev: rev + 1, ..fresh };
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
//!
|
||||
//! Coordinates are points, y down. Each spread has its own space: pages sit side by side from
|
||||
//! x = 0 with their tops at y = 0. Items live in spread space via `Item::xf`.
|
||||
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
|
||||
#![forbid(unsafe_code)]
|
||||
|
||||
pub mod anchored;
|
||||
@@ -68,6 +69,10 @@ pub enum DocError {
|
||||
|
||||
pub type Result<T> = std::result::Result<T, DocError>;
|
||||
|
||||
/// Largest `next_id` a valid document has (ids count up from it; 2^53 also stays exact in JSON
|
||||
/// readers that use doubles).
|
||||
pub const MAX_NEXT_ID: u64 = 1 << 53;
|
||||
|
||||
/// Edit › Transparency Blend Space: the colour space transparency is flattened in (CMYK for
|
||||
/// print documents, RGB for screen ones, as InDesign defaults).
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
@@ -716,6 +721,10 @@ impl Document {
|
||||
if self.next_id < ids.iter().copied().max().unwrap_or(0) {
|
||||
return Err(DocError::Invalid("next_id behind existing ids".into()));
|
||||
}
|
||||
// Ids are allocated by counting up from `next_id`: leave room so that can't overflow.
|
||||
if self.next_id > MAX_NEXT_ID {
|
||||
return Err(DocError::Invalid(format!("next_id {} out of range", self.next_id)));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -58,6 +58,9 @@ impl Default for ParaFormat {
|
||||
}
|
||||
}
|
||||
|
||||
/// The format a story without runs reports (a valid story always has one run).
|
||||
static NO_FORMAT: std::sync::LazyLock<CharFormat> = std::sync::LazyLock::new(CharFormat::default);
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct CharFormat {
|
||||
@@ -186,7 +189,7 @@ impl Story {
|
||||
}
|
||||
start = end;
|
||||
}
|
||||
&self.chars.last().expect("at least one run").format
|
||||
self.chars.last().map_or(&NO_FORMAT, |r| &r.format)
|
||||
}
|
||||
|
||||
/// Iterate `(range, format)` of character runs.
|
||||
@@ -310,7 +313,7 @@ impl Story {
|
||||
}
|
||||
start += r.len;
|
||||
}
|
||||
&self.chars.last().expect("run").format
|
||||
self.chars.last().map_or(&NO_FORMAT, |r| &r.format)
|
||||
}
|
||||
|
||||
/// Apply `f` to the character format of every run intersecting `range` (splitting runs at the
|
||||
|
||||
@@ -342,7 +342,7 @@ impl Styles {
|
||||
if !existing(base) {
|
||||
return base.to_string();
|
||||
}
|
||||
(2..).map(|i| format!("{base} {i}")).find(|n| !existing(n)).expect("infinite")
|
||||
(2..=usize::MAX).map(|i| format!("{base} {i}")).find(|n| !existing(n)).unwrap_or_else(|| format!("{base} copy"))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -298,7 +298,7 @@ impl Table {
|
||||
let mut t = Table::new(id, data.len().max(1), cols, 0, 0, width);
|
||||
for (r, row) in data.iter().enumerate() {
|
||||
for (c, s) in row.iter().enumerate() {
|
||||
let cell = t.cell_mut(r, c).expect("in range");
|
||||
let Some(cell) = t.cell_mut(r, c) else { continue };
|
||||
cell.text = Story::with_text(StoryId(0), "", ParaFormat { table: None, ..para.clone() });
|
||||
cell.text.chars[0].format = fmt.clone();
|
||||
cell.text.insert(0, s);
|
||||
|
||||
@@ -100,3 +100,15 @@ fn document_serde_roundtrip() {
|
||||
assert_eq!(d, back);
|
||||
back.check().unwrap();
|
||||
}
|
||||
|
||||
/// A file can set `next_id` to `u64::MAX`; the next allocated id then overflowed (IDML export
|
||||
/// adds to it too). Such a document is rejected when it is checked on load.
|
||||
#[test]
|
||||
fn next_id_near_the_top_is_rejected() {
|
||||
let mut d = Document::new(&crate::build::NewDocument::default());
|
||||
d.check().unwrap();
|
||||
d.next_id = u64::MAX;
|
||||
assert!(d.check().is_err());
|
||||
d.next_id = crate::MAX_NEXT_ID;
|
||||
d.check().unwrap();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user