diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml index 970c6577a4..56d1ad0831 100644 --- a/.github/workflows/freebsd.yml +++ b/.github/workflows/freebsd.yml @@ -32,5 +32,7 @@ jobs: with: craft-fonts-ref: abb83316d96aa59c1cf64784289e378fe9fa5695 cargo-build-extra-args: "--features heif" - custom-test-command: "CARGO_PROFILE_DEV_DEBUG=0 cargo test --workspace --locked --features heif --exclude photocraft-ui-egui && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --lib && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --doc && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --test '*' -- --test-threads=1" + # The VM's software Vulkan driver crashes (SIGSEGV) when several tests set up a device at + # once, so photocraft-gpu's tests run one at a time, like ui-egui's integration tests. + custom-test-command: "CARGO_PROFILE_DEV_DEBUG=0 cargo test --workspace --locked --features heif --exclude photocraft-ui-egui --exclude photocraft-gpu && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-gpu --locked -- --test-threads=1 && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --lib && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --doc && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --test '*' -- --test-threads=1" run-package-check: true diff --git a/README.md b/README.md index a80c702233..4285f69513 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,10 @@

+

+ PhotoCraft icon: an engraved nine-tailed fox on a blue tile +

+

PhotoCraft

diff --git a/crates/affinity/src/access.rs b/crates/affinity/src/access.rs index 0b60ed7dbd..732feeef49 100644 --- a/crates/affinity/src/access.rs +++ b/crates/affinity/src/access.rs @@ -20,6 +20,15 @@ impl Stream { self.class(id) == Some(Tag::of(class)) } + /// The raw slot count of an object's array field (including null slots), for charging + /// traversal work before nulls are filtered out. `None` when the field is not an array. + pub fn array_len(&self, id: ObjId, tag: &[u8; 4]) -> Option { + match self.field(id, tag)? { + Value::Array(a) => Some(a.len()), + _ => None, + } + } + /// A child object (shared, linked or inline); `None` for null or missing. pub fn obj(&self, id: ObjId, tag: &[u8; 4]) -> Option { match self.field(id, tag)? { diff --git a/crates/affinity/src/geometry.rs b/crates/affinity/src/geometry.rs index 6c8ea602c9..6825abd511 100644 --- a/crates/affinity/src/geometry.rs +++ b/crates/affinity/src/geometry.rs @@ -210,7 +210,16 @@ fn anchors(records: &[Value]) -> Option> { fn subpath(anchors: &[Anchor], closed: bool, radii: Option<&Vec<(usize, f64)>>) -> Option { let first = anchors.first()?; if let Some(radii) = radii.filter(|r| !r.is_empty()) { - let corner = |a: &Anchor| radii.iter().filter(|(rec, _)| *rec == a.record).map(|(_, r)| *r).fold(0.0, f64::max); + // Index each record's maximum radius once. Scanning the whole radius list per anchor + // was O(anchors * radii), which a file with large anchor and radius arrays could drive + // into quadratic work. Duplicate record indices keep the maximum, matching the prior + // fold(0.0, f64::max) behavior. + let mut radius_by_record: std::collections::HashMap = std::collections::HashMap::with_capacity(radii.len()); + for &(rec, r) in radii { + let slot = radius_by_record.entry(rec).or_insert(0.0); + *slot = slot.max(r); + } + let corner = |a: &Anchor| radius_by_record.get(&a.record).copied().unwrap_or(0.0); let polyline = anchors.iter().all(|a| a.inh.is_none_or(|h| h == a.p) && a.out.is_none_or(|h| h == a.p)); if polyline { let mut pts: Vec<(Point, f64)> = anchors.iter().map(|a| (a.p, corner(a))).collect(); diff --git a/crates/affinity/src/model.rs b/crates/affinity/src/model.rs index 7c05072419..f77c970ae2 100644 --- a/crates/affinity/src/model.rs +++ b/crates/affinity/src/model.rs @@ -283,6 +283,12 @@ const MAX_TREE_DEPTH: usize = 128; /// Total layer nodes imported. const MAX_NODES: usize = 500_000; +/// Total raw `Chld` slots inspected across the whole traversal, including null slots and slots +/// reached through repeated references to a shared group. The node limit only counts produced +/// nodes, so a shared group holding a large null-filled child array could still be rescanned +/// many times; this bounds that work. Generous relative to MAX_NODES for real documents. +const MAX_CHILD_SLOTS: usize = 8_000_000; + /// Read a document: archive, `doc.dat` and the model. pub fn read(bytes: &[u8], limits: Limits) -> Result { let mut archive = Archive::open(bytes, limits)?; @@ -297,6 +303,7 @@ pub(crate) struct Reader<'s, 'a, 'b> { pub(crate) dpi: f64, warnings: BTreeMap, nodes: usize, + child_slots: usize, active: HashSet, pub(crate) geometry_budget: crate::geometry::Budget, /// Decoded pixels still available ([`Limits::max_pixel_bytes`] less what the layers read so @@ -312,6 +319,7 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { dpi: 72.0, warnings: BTreeMap::new(), nodes: 0, + child_slots: 0, active: HashSet::new(), geometry_budget: crate::geometry::Budget::default(), pixels_left: limits.max_pixel_bytes, @@ -383,6 +391,13 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { let s = self.s; let mut out = Vec::new(); let mut mask = None; + // Charge every raw child slot (null slots included) against a document-wide budget + // before filtering. A shared group with a large null-filled Chld array could otherwise + // be rescanned through each reference without the node limit noticing. + self.child_slots = self.child_slots.saturating_add(s.array_len(parent, b"Chld").unwrap_or(0)); + if self.child_slots > MAX_CHILD_SLOTS { + return Err(Error::Limit("too many child slots in the layer tree")); + } for c in s.objs(parent, b"Chld") { if s.is(c, b"MRst") { // A mask layer inside a group or layer masks that container. diff --git a/crates/affinity/src/stream.rs b/crates/affinity/src/stream.rs index a755747c45..2ffe12b7e5 100644 --- a/crates/affinity/src/stream.rs +++ b/crates/affinity/src/stream.rs @@ -73,11 +73,40 @@ pub struct Object { pub chain: Vec, /// Fields in stream order; positional fields have tag 0. pub fields: Vec<(Tag, Value)>, + /// First-occurrence index of each non-positional tag, so `get` is O(1). A shared object + /// can be referenced by many sibling layers, and each reference used to rescan all of its + /// fields; indexing once keeps repeated lookups from multiplying into quadratic work. + index: std::collections::HashMap, } impl Object { + pub fn new(class: Tag, kind: Kind, chain: Vec, fields: Vec<(Tag, Value)>) -> Self { + let mut index = std::collections::HashMap::with_capacity(fields.len()); + for (i, (t, _)) in fields.iter().enumerate() { + // Positional fields share tag 0 and are not looked up by tag; keep first-match + // semantics for real tags. + if t.0 != 0 { + index.entry(*t).or_insert(i); + } + } + Object { class, kind, chain, fields, index } + } + pub fn get(&self, tag: Tag) -> Option<&Value> { - self.fields.iter().find(|(t, _)| *t == tag).map(|(_, v)| v) + self.index.get(&tag).map(|&i| &self.fields[i].1) + } + + /// Replaces the fields (the root object is filled in after it is pushed) and rebuilds the + /// tag index so `get` stays correct. + fn set_fields(&mut self, fields: Vec<(Tag, Value)>) { + self.index.clear(); + self.index.reserve(fields.len()); + for (i, (t, _)) in fields.iter().enumerate() { + if t.0 != 0 { + self.index.entry(*t).or_insert(i); + } + } + self.fields = fields; } } @@ -124,10 +153,10 @@ pub fn parse(bytes: &[u8]) -> Result { c.u32()?; } let mut p = Parser { c, objects: Vec::new(), shared: std::collections::HashMap::new(), depth: 0, values: 0 }; - let root = p.push(Object { class, kind: Kind::Root, chain: vec![class], fields: Vec::new() })?; + let root = p.push(Object::new(class, Kind::Root, vec![class], Vec::new()))?; let fields = p.fields(true, class)?; if let Some(o) = p.objects.get_mut(root) { - o.fields = fields; + o.set_fields(fields); } Ok(Stream { root, objects: p.objects }) } @@ -252,7 +281,7 @@ impl Parser<'_> { } 0x30 => { let fields = self.fields(false, class)?; - let id = self.push(Object { class, kind: Kind::Positional, chain: Vec::new(), fields })?; + let id = self.push(Object::new(class, Kind::Positional, Vec::new(), fields))?; Value::Obj(Some(id)) } 0x31 | 0x32 if !tagged => return Err(Error::Malformed("object without a field tag")), @@ -350,7 +379,7 @@ impl Parser<'_> { } }; let fields = self.fields(true, class)?; - let id = self.push(Object { class, kind: Kind::Inline, chain: vec![class], fields })?; + let id = self.push(Object::new(class, Kind::Inline, vec![class], fields))?; Ok(Value::Obj(Some(id))) } @@ -369,7 +398,7 @@ impl Parser<'_> { return Err(Error::Malformed("object defined twice")); } // Register before the fields: an object's own fields may refer back to it. - let id = self.push(Object { class: Tag(0), kind: Kind::Shared, chain: Vec::new(), fields: Vec::new() })?; + let id = self.push(Object::new(Tag(0), Kind::Shared, Vec::new(), Vec::new()))?; self.shared.insert(key, id); let mut chain = Vec::new(); let mut fields = Vec::new(); @@ -395,7 +424,7 @@ impl Parser<'_> { let class = chain.first().copied().unwrap_or(Tag(0)); fields.extend(self.fields(true, class)?); if let Some(o) = self.objects.get_mut(id) { - *o = Object { class, kind: Kind::Shared, chain, fields }; + *o = Object::new(class, Kind::Shared, chain, fields); } Ok(Value::Obj(Some(id))) } diff --git a/crates/cms/src/clut.rs b/crates/cms/src/clut.rs index 186cd3f9f1..38992a2ed4 100644 --- a/crates/cms/src/clut.rs +++ b/crates/cms/src/clut.rs @@ -172,11 +172,16 @@ impl Clut { } let o = self.outputs; out[..o].iter_mut().for_each(|v| *v = 0.0); - for corner in 0..(1usize << n) { + // Only dimensions with more than one grid point contribute a cube axis. Degenerate + // dimensions have step 0 and ratio 0, so they always multiply the corner weight by 1 + // and never change the offset; enumerating 2^n corners over all declared inputs would + // do 2^(degenerate) redundant iterations. Interpolate over the effective dims only. + let eff: Vec = (0..n).filter(|&d| s[d] != 0).collect(); + for corner in 0..(1usize << eff.len()) { let mut w = 1.0f32; let mut off = base; - for d in 0..n { - if corner & (1 << d) != 0 { + for (bit, &d) in eff.iter().enumerate() { + if corner & (1 << bit) != 0 { w *= r[d]; off += s[d]; } else { @@ -245,4 +250,20 @@ mod tests { c.eval(&[0.5, 0.25], &mut o); assert!((o[0] - 0.75).abs() < 1e-6); } + + #[test] + fn degenerate_dimensions_do_not_change_the_result() { + // Dimensions with a single grid point contribute nothing, but the old code still + // enumerated 2^inputs corners over them. Interpolating over effective dims only must + // give the same answer as a reference built without the degenerate axes. + let f = |p: &[f32], o: &mut [f32]| o[0] = 0.25 * p[0] + 0.5 * p[2] + 0.125 * p[4]; + // Axes 1 and 3 have a single grid point (degenerate); axes 0, 2, 4 vary. + let c = Clut::sample(vec![3, 1, 4, 1, 5], 1, f); + let mut got = [0.0]; + for p in [[0.2f32, 0.0, 0.7, 0.0, 0.4], [1.0, 0.0, 0.0, 0.0, 1.0], [0.5, 0.0, 0.5, 0.0, 0.5]] { + c.eval_multilinear(&p, &mut got); + let want = 0.25 * p[0] + 0.5 * p[2] + 0.125 * p[4]; + assert!((got[0] - want).abs() < 1e-5, "got {} want {want}", got[0]); + } + } } diff --git a/crates/cms/src/curve.rs b/crates/cms/src/curve.rs index fe23e6e059..8b619842d1 100644 --- a/crates/cms/src/curve.rs +++ b/crates/cms/src/curve.rs @@ -99,6 +99,20 @@ impl Curve { (0..n).map(|i| self.eval64(i as f64 / (n - 1) as f64) as f32).collect() } + /// Inverse of the curve at each `y` in `ys`. For sampled-table curves the flat-run edge + /// indices are computed once for the whole batch instead of rescanning the table on every + /// call, so inverting a long constant table many times stays linear. + pub fn inverse_table(&self, ys: impl Iterator) -> Vec { + match self { + Curve::Table(t) if t.len() >= 2 => { + let increasing = t[t.len() - 1] >= t[0]; + let (edge_start, edge_end) = (edge_index(t, true), edge_index(t, false)); + ys.map(|y| table_inverse_with(t, y as f32, increasing, edge_start, edge_end)).collect() + } + _ => ys.map(|y| self.eval_inverse64(y) as f32).collect(), + } + } + /// `true` when the curve never decreases over `[0, 1]`. pub fn is_monotonic(&self) -> bool { let s = self.sample(1024); @@ -244,13 +258,24 @@ fn table_inverse(t: &[f32], y: f32) -> f32 { return y; } let increasing = t[n - 1] >= t[0]; + table_inverse_with(t, y, increasing, edge_index(t, true), edge_index(t, false)) +} + +/// [`table_inverse`] with the flat-run edge indices supplied. Computing those edges scans the +/// whole table, and transform construction inverts the same table thousands of times, so the +/// edges are computed once and reused across a batch (see [`Curve::inverse_table`]). +fn table_inverse_with(t: &[f32], y: f32, increasing: bool, edge_start: f32, edge_end: f32) -> f32 { + let n = t.len(); + if n < 2 { + return y; + } let (lo_v, hi_v) = if increasing { (t[0], t[n - 1]) } else { (t[n - 1], t[0]) }; if y <= lo_v { // Flat start: choose the last index still at the lowest value. - return if increasing { edge_index(t, true) } else { edge_index(t, false) }; + return if increasing { edge_start } else { edge_end }; } if y >= hi_v { - return if increasing { edge_index(t, false) } else { edge_index(t, true) }; + return if increasing { edge_end } else { edge_start }; } let (mut lo, mut hi) = (0usize, n - 1); while hi - lo > 1 { @@ -336,4 +361,24 @@ mod tests { let g = Curve::Gamma(2.2); assert!((g.eval64(-0.5) + 0.5f64.powf(2.2)).abs() < 1e-12); } + + #[test] + fn inverse_table_matches_per_call_including_flat_runs() { + // Batch inverse must equal calling eval_inverse64 per point. Include a table with flat + // runs at both ends (the case that used to rescan the table on every call). + let tables = [ + Curve::Table(vec![0.0, 0.0, 0.0, 0.25, 0.5, 1.0, 1.0, 1.0]), + Curve::Table(vec![0.0, 0.2, 0.4, 0.6, 0.8, 1.0]), + Curve::Table(vec![1.0, 0.5, 0.0]), + Curve::Gamma(2.2), + ]; + let ys: Vec = (0..=40).map(|i| i as f64 / 40.0).collect(); + for c in &tables { + let batch = c.inverse_table(ys.iter().copied()); + for (i, &y) in ys.iter().enumerate() { + let one = c.eval_inverse64(y) as f32; + assert!((batch[i] - one).abs() < 1e-6, "y={y}: batch {} vs per-call {one}", batch[i]); + } + } + } } diff --git a/crates/cms/src/transform.rs b/crates/cms/src/transform.rs index 04d11247f4..f30c17c82e 100644 --- a/crates/cms/src/transform.rs +++ b/crates/cms/src/transform.rs @@ -133,12 +133,11 @@ impl Transform { let tables = cs .iter() .map(|c| { - (0..OUT_TABLE) - .map(|i| { - let u = i as f64 / (OUT_TABLE - 1) as f64; - c.eval_inverse64(u * u * u * u).clamp(0.0, 1.0) as f32 - }) - .collect() + let ys = (0..OUT_TABLE).map(|i| { + let u = i as f64 / (OUT_TABLE - 1) as f64; + u * u * u * u + }); + c.inverse_table(ys).into_iter().map(|v| v.clamp(0.0, 1.0)).collect() }) .collect(); post = Some(Post { tables }); diff --git a/crates/codecs/src/codecs/deep_exr.rs b/crates/codecs/src/codecs/deep_exr.rs index 29f3762ebc..4fe9def532 100644 --- a/crates/codecs/src/codecs/deep_exr.rs +++ b/crates/codecs/src/codecs/deep_exr.rs @@ -341,15 +341,17 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res return Err(err(format!("chunk declares part {part}, expected {header_index}"))); } } - // Block coordinates → the block's global pixel range and its slot. - let (block_pixels, slot, row_w): (Vec, usize, usize) = match &header.blocks { + // Block coordinates → the block's pixel bounds and its slot. The per-pixel index is + // built only after the chunk's declared payload sizes are validated below, so a + // truncated or lying chunk cannot drive a large index allocation first. + let (block_rows, block_cols, slot, row_w, scanline_row): (std::ops::Range, std::ops::Range, usize, usize, bool) = match &header.blocks { BlockDescription::ScanLines => { let y = i32(&mut cur)?; let row = match y.checked_sub(y0) { Some(d) if d >= 0 && (d as usize) < h => d as usize, _ => return Err(err(format!("scan line block y={y} is outside the data window"))), }; - ((0..w).map(|x| row * w + x).collect(), row, w) + (row..row + 1, 0..w, row, w, true) } BlockDescription::Tiles(_) => { let (tx, ty, lx, ly) = (i32(&mut cur)?, i32(&mut cur)?, i32(&mut cur)?, i32(&mut cur)?); @@ -362,16 +364,11 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res let (bx, by) = ((tx as usize) * tile_w, (ty as usize) * tile_h); let rows = (by + tile_h).min(h); let cols = (bx + tile_w).min(w); - let mut px = Vec::with_capacity((rows - by) * (cols - bx)); - for yy in by..rows { - for xx in bx..cols { - px.push(yy.saturating_mul(w).saturating_add(xx)); - } - } let slot = (ty as usize).saturating_mul(tiles_x) + tx as usize; - (px, slot, cols - bx) + (by..rows, bx..cols, slot, cols - bx, false) } }; + let block_len = block_rows.len().saturating_mul(block_cols.len()); if seen.get(slot).copied().unwrap_or(true) { return Err(err(format!("two chunks cover block {slot}"))); } @@ -386,6 +383,20 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res if packed_table > remaining || packed_data > remaining.saturating_sub(packed_table) { return Err(err("deep chunk sizes point past the end of the file")); } + // The chunk's payload is present; now materialize the block's global pixel index. For + // scan lines this is one row; for tiles the clipped tile rectangle. + let block_pixels: Vec = if scanline_row { + let row = block_rows.start; + (0..w).map(|x| row * w + x).collect() + } else { + let mut px = Vec::with_capacity(block_len); + for yy in block_rows.clone() { + for xx in block_cols.clone() { + px.push(yy.saturating_mul(w).saturating_add(xx)); + } + } + px + }; let what = "the sample-count table"; let table_bytes = read_exact(&mut cur, packed_table as usize, what)?; let expected_table = block_pixels.len().checked_mul(4).ok_or_else(|| err("count table overflows"))?; @@ -552,7 +563,7 @@ fn header_end(bytes: &[u8]) -> Result { /// into straight alpha at the end. Without any alpha channel the front-most sample wins, /// opaque. Z, ZBack and other deep channels influence the flat result only through the /// ordering; volume samples (with ZBack) are approximated as point samples. -pub(crate) fn flatten(deep: &DeepImage) -> Result { +pub(crate) fn flatten(deep: &DeepImage, limits: &Limits) -> Result { let npx = deep.width as usize * deep.height as usize; let has = |n: &str| deep.channel(n).is_some(); let rgb = has("R") && has("G") && has("B"); @@ -569,6 +580,10 @@ pub(crate) fn flatten(deep: &DeepImage) -> Result { return Err(CodecError::unsupported(F, "no R/G/B or Y channels in the deep part")); }; let nc = layout.channels(); + // Admit the flattened output at its true size: nc f32 samples per pixel. The deep decode + // admitted the data window at four bytes per pixel, before the flattened channel layout + // was known, so re-check here with the actual layout and f32 sample size. + limits.check_bytes(deep.width, deep.height, nc as u64 * 4)?; let samples = |n: &str| -> &[f32] { deep.channel(n).map(|c| c.samples.as_slice()).unwrap_or(&[]) }; let (r, g, b) = (samples("R"), samples("G"), samples("B")); let (ar, ag, ab) = (samples("AR"), samples("AG"), samples("AB")); @@ -665,6 +680,10 @@ pub(crate) fn flatten(deep: &DeepImage) -> Result { // pads to the stage at its offset, transparent elsewhere (limits validated the size). let mut img = match deep.stage { Some((sw, sh)) => { + // The display stage can be larger than the data window; admit it at the real f32 + // layout size before allocating, since the earlier deep-decode check used four + // bytes per pixel and did not know the flattened channel count. + limits.check_bytes(sw, sh, nc as u64 * 4)?; let mut padded = vec![0f32; sw as usize * sh as usize * nc]; let row_len = deep.width as usize * nc; let dx = deep.origin.0.max(0) as usize; diff --git a/crates/codecs/src/codecs/exr.rs b/crates/codecs/src/codecs/exr.rs index 1b268ac559..2b32a2b7c4 100644 --- a/crates/codecs/src/codecs/exr.rs +++ b/crates/codecs/src/codecs/exr.rs @@ -164,7 +164,7 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result // composite the samples into the flat image (a warning says what was lost). if meta.requirements.has_deep_data { let deep = super::deep_exr::decode_deep(&meta, bytes, limits)?; - return super::deep_exr::flatten(&deep); + return super::deep_exr::flatten(&deep, limits); } let pick = pick_part(&meta); diff --git a/crates/codecs/src/codecs/exr_cryptomatte.rs b/crates/codecs/src/codecs/exr_cryptomatte.rs index 5f13f98621..370c7c7e56 100644 --- a/crates/codecs/src/codecs/exr_cryptomatte.rs +++ b/crates/codecs/src/codecs/exr_cryptomatte.rs @@ -156,9 +156,13 @@ pub(crate) fn layers(bytes: &[u8], limits: &Limits) -> Result> = std::collections::HashMap::new(); for key in cryptomatte_keys(&header.own_attributes.other) { let Some(name) = attribute(&header.own_attributes.other, &key, "name") else { continue }; - let channels = layer_streams(header, name.as_str()); + let channels = streams_for.entry(name.clone()).or_insert_with(|| layer_streams(header, name.as_str())).clone(); if channels.is_empty() { continue; } @@ -184,11 +188,15 @@ pub(crate) fn layers(bytes: &[u8], limits: &Limits) -> Result/` key of a header, in file order, without duplicates. fn cryptomatte_keys(other: &std::collections::HashMap) -> Vec { let mut keys = Vec::new(); + // Track seen keys in a set: the attribute names are file-controlled, so membership with + // keys.iter().any() is O(n^2) in the number of distinct `cryptomatte//` + // groups. The Vec still records file order; the set only answers "seen before". + let mut seen = std::collections::HashSet::new(); for name in other.keys() { let full = name.to_string(); let Some(rest) = full.strip_prefix("cryptomatte/") else { continue }; let Some((key, _field)) = rest.split_once('/') else { continue }; - if !key.is_empty() && !keys.iter().any(|k| k == key) { + if !key.is_empty() && seen.insert(key.to_string()) { keys.push(key.to_string()); } } @@ -217,14 +225,18 @@ pub(crate) fn decode(bytes: &[u8], layer_name: &str, limits: &Limits) -> Result< let too_large = || CodecError::malformed(F, "the layer is too large"); let (w32, h32) = (u32::try_from(w).map_err(|_| too_large())?, u32::try_from(h).map_err(|_| too_large())?); // Each pixel also gets a pair list, so count its header with the channel samples (four - // f32 channels per set: two ranks of ID and coverage). - let per_pixel = (layer.channels.len().max(1) as u64).saturating_mul(16).saturating_add(std::mem::size_of::>() as u64); + // f32 channels per set: two ranks of ID and coverage). Include the heap capacity the pair + // list can reach: up to one (id, coverage) pair per rank, 8 bytes each. Without this the + // returned per-pixel vectors could exceed the caller's max_alloc budget. + let ranks = layer.channels.len().saturating_mul(2); + let pair_capacity = (ranks as u64).saturating_mul(std::mem::size_of::<(f32, f32)>() as u64); + let per_pixel = + (layer.channels.len().max(1) as u64).saturating_mul(16).saturating_add(std::mem::size_of::>() as u64).saturating_add(pair_capacity); limits.check_bytes(w32, h32, per_pixel)?; let ex_layer = super::exr::read_layer(bytes, layer.part)?; // Two (ID, coverage) ranks per set: rank `slot * 2 + stream.rank`. - let ranks = layer.channels.len().saturating_mul(2); let mut ids: Vec> = vec![Vec::new(); ranks]; let mut coverages: Vec> = vec![Vec::new(); ranks]; for c in &ex_layer.channel_data.list { diff --git a/crates/codecs/src/codecs/fits.rs b/crates/codecs/src/codecs/fits.rs index 7e3f92ae67..214d968944 100644 --- a/crates/codecs/src/codecs/fits.rs +++ b/crates/codecs/src/codecs/fits.rs @@ -22,6 +22,8 @@ //! (strings longer than one card on `CONTINUE` cards). //! Tile-compressed files (fpack `ZIMAGE` tables) are recognised and refused. +use std::collections::HashMap; + use crate::Format; use crate::error::CodecError; use crate::fidelity::Plan; @@ -106,13 +108,19 @@ fn parse_card(raw: &[u8]) -> Card { struct Header { cards: Vec, + /// First-occurrence index of each non-HIERARCH keyword, so `get` is O(1). The card + /// count and the number of looked-up keywords (NAXIS1..NAXISn) are both file-controlled, + /// so a linear scan per lookup lets a crafted header multiply commentary cards by axis + /// count into quadratic work. + index: HashMap, /// Offset of the data unit. data_start: usize, } impl Header { fn get(&self, key: &str) -> Option<&Value> { - self.cards.iter().find(|c| !c.hierarch && c.key == key).and_then(|c| c.value.as_ref()) + let c = &self.cards[*self.index.get(key)?]; + c.value.as_ref() } fn int(&self, key: &str) -> Option { @@ -167,7 +175,15 @@ fn parse_header(b: &[u8], start: usize) -> Result { cards.push(card); } let data_start = pos.div_ceil(BLOCK).checked_mul(BLOCK).ok_or_else(|| err("header too long"))?; - Ok(Header { cards, data_start }) + // Index each non-HIERARCH keyword to its first occurrence, matching the previous + // `find(|c| !c.hierarch && c.key == key)` lookup while making `get` O(1). + let mut index = HashMap::with_capacity(cards.len()); + for (i, c) in cards.iter().enumerate() { + if !c.hierarch { + index.entry(c.key.clone()).or_insert(i); + } + } + Ok(Header { cards, index, data_start }) } /// The axis lengths (`NAXIS1`, `NAXIS2`, …). diff --git a/crates/codecs/src/codecs/heif.rs b/crates/codecs/src/codecs/heif.rs index 31da309ca3..efd9d2073a 100644 --- a/crates/codecs/src/codecs/heif.rs +++ b/crates/codecs/src/codecs/heif.rs @@ -45,6 +45,13 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits, keep_orientation: bool) -> R let sample = if info.sixteen_bit { SampleType::U16 } else { SampleType::U8 }; let (w, h) = if keep_orientation { (info.coded_width, info.coded_height) } else { (info.width, info.height) }; limits.check(w, h, layout, sample)?; + // The delegated decoder allocates the full coded image before applying any clean-aperture + // crop or orientation transform, so the coded dimensions must pass the limit policy even + // when the displayed (transformed) size is small. Checking only the transformed size let a + // tiny visible image hide a large coded allocation. + if !keep_orientation && (info.coded_width, info.coded_height) != (w, h) { + limits.check(info.coded_width, info.coded_height, layout, sample)?; + } let options = photocraft_heif::Options { max_pixels: limits.max_pixels, apply_transforms: !keep_orientation }; let decoded = photocraft_heif::decode(bytes, &options).map_err(err)?; // The decoder re-reads the container; trust its output's shape, not the probe's. diff --git a/crates/codecs/src/resolution.rs b/crates/codecs/src/resolution.rs index cb86001ac9..a787cacbc3 100644 --- a/crates/codecs/src/resolution.rs +++ b/crates/codecs/src/resolution.rs @@ -429,8 +429,14 @@ fn xmp_values(xmp: &str, key: &str) -> Vec<(usize, usize)> { continue; }; let start = xmp.len() - value.len(); - let Some(len) = value.find(close) else { continue }; + // Advance past this value before searching for the next key. Without this, `from` + // only moved past the key text, so property-like text repeated inside an unrelated + // attribute re-ran this full-suffix `find(close)` for every occurrence, which is + // quadratic in the packet size. A missing close means the value is unterminated, so + // no later value can parse either; stop scanning in that case. + let Some(len) = value.find(close) else { break }; out.push((start, start + len)); + from = start + len + close.len_utf8(); } out } diff --git a/crates/codecs/tests/resolution.rs b/crates/codecs/tests/resolution.rs index 15ddc95749..28c8203c96 100644 --- a/crates/codecs/tests/resolution.rs +++ b/crates/codecs/tests/resolution.rs @@ -284,6 +284,17 @@ fn truncated_metadata_never_panics_and_falls_back() { assert_eq!(xmp_resolution(x), None, "{x}"); let _ = xmp_with_resolution(x, (1.0, 1.0)); } + + // Property-like text repeated inside the packet must not trigger a full-suffix rescan + // per occurrence (it used to be quadratic). With the cursor advancing past each located + // value, this completes well under a second even with many decoys; a timeout here would + // indicate the quadratic scan returned. A clean packet still extracts correctly. + let start = std::time::Instant::now(); + let decoys = " tiff:ResolutionUnit>".repeat(50_000); + let _ = xmp_resolution(&decoys); + assert!(start.elapsed() < std::time::Duration::from_secs(2), "xmp scan took too long: {:?}", start.elapsed()); + let clean = "300/1300/12"; + near(xmp_resolution(clean), 300.0); } proptest! { diff --git a/crates/compose/src/masks.rs b/crates/compose/src/masks.rs index 802a0f2df9..7335be90d0 100644 --- a/crates/compose/src/masks.rs +++ b/crates/compose/src/masks.rs @@ -221,7 +221,14 @@ fn build_mask(layer: &Layer, canvas: Rect) -> Surface { } let area = area.intersect(&canvas); let mut s = Surface::with_default(FORMAT, &[p_def * v_out]); - if !area.is_empty() { + // Bound the transient dense working set: `area` is derived from sparse mask geometry grown + // by feather, so two distant changed pixels could span a canvas-sized rectangle. Building + // f32 planes over it (coverage, pixel-mask values, blur scratch) could be gigabytes during + // a one-pixel render. Above the budget, fall back to the constant default mask rather than + // allocating; feather detail is lost only for pathological geometry. + const MAX_MASK_WORKING_BYTES: u64 = 512 << 20; + let working = (area.width() as u64).saturating_mul(area.height() as u64).saturating_mul(std::mem::size_of::() as u64).saturating_mul(3); + if !area.is_empty() && working <= MAX_MASK_WORKING_BYTES { let (w, h) = (area.width() as usize, area.height() as usize); // A slot initializer must not enter Rayon: another tile requesting the // same mask could be stolen onto its stack and wait for its own build. diff --git a/crates/compose/src/pattern.rs b/crates/compose/src/pattern.rs index d107ecf4e7..b66ab0ad1e 100644 --- a/crates/compose/src/pattern.rs +++ b/crates/compose/src/pattern.rs @@ -12,6 +12,10 @@ use photocraft_doc::{ColorMode, Pattern, PixelFormat, Rect, SampleType}; pub(crate) const PREPARED_PATTERN_BYTES: usize = 64 << 20; const MAX_PREPARED_PATTERNS: usize = 64; +/// Absolute cap on a single pattern's RGBA conversion on the uncached path. A pattern this +/// large cannot be cached, so re-expanding it for every output tile would be a DoS; above the +/// cap it is skipped instead. Independent of the cache budget so small patterns still convert. +const MAX_UNCACHED_PATTERN_BYTES: usize = 256 << 20; type PreparedSlot = Arc>>>; struct PreparedState { @@ -78,6 +82,13 @@ impl<'a> PreparedPatterns<'a> { // Run on the requesting worker to preserve its active CMYK profile. slot.get_or_init(|| Tile::new_in(p, self.mode).map(Arc::new)).clone() } + // Uncached path: a pattern whose full conversion would exceed an absolute cap is + // not materialized at all. Otherwise every output tile would re-expand the entire + // sparse pattern into RGBA floats (and could do so concurrently), exhausting CPU and + // memory even though cache admission already refused to retain it. The cap is + // independent of the (possibly zero) cache budget, so small patterns still convert + // on demand when nothing is cached. + None if bytes.is_none_or(|b| b > MAX_UNCACHED_PATTERN_BYTES) => None, None => Tile::new_in(p, self.mode).map(Arc::new), } } diff --git a/crates/doc/src/slices.rs b/crates/doc/src/slices.rs index e22fcc0b79..7fb6ba3ad5 100644 --- a/crates/doc/src/slices.rs +++ b/crates/doc/src/slices.rs @@ -202,24 +202,42 @@ pub fn auto_slices(canvas: Rect, taken: &[Rect]) -> Vec { xs.dedup(); ys.sort_unstable(); ys.dedup(); - let covered = |x0: i32, y0: i32| taken.iter().any(|r| x0 >= r.x0 && x0 < r.x1 && y0 >= r.y0 && y0 < r.y1); - // Horizontal runs per row band. + // Clip the taken rectangles to the canvas once, so band coverage only considers rects that + // can actually cover a cell. + let clipped: Vec = taken.iter().map(|r| r.intersect(&canvas)).filter(|r| !r.is_empty()).collect(); + // Map an x edge to its cell index in `xs` (xs is sorted/deduped). + let x_cell = |x: i32| xs.partition_point(|&e| e <= x).saturating_sub(1); + // Horizontal runs per row band. For each band, mark covered x-cells in a single pass over + // the rects intersecting that band, instead of scanning every rect for every cell (which + // was cubic in the slice count and let a crafted document stall canvas drawing). + let ncells = xs.len().saturating_sub(1); let mut runs: Vec = Vec::new(); for yw in ys.windows(2) { + let (band_y0, band_y1) = (yw[0], yw[1]); + let mut covered_cell = vec![false; ncells]; + for r in &clipped { + if r.y0 <= band_y0 && band_y1 <= r.y1 { + // Rect spans this whole band vertically; mark the x-cells it covers. + let (c0, c1) = (x_cell(r.x0), x_cell(r.x1)); + for c in covered_cell.iter_mut().take(c1).skip(c0) { + *c = true; + } + } + } let mut start: Option = None; - for xw in xs.windows(2) { - let free = !covered(xw[0], yw[0]); + for (ci, xw) in xs.windows(2).enumerate() { + let free = !covered_cell.get(ci).copied().unwrap_or(false); match (free, start) { (true, None) => start = Some(xw[0]), (false, Some(s)) => { - runs.push(Rect::new(s, yw[0], xw[0], yw[1])); + runs.push(Rect::new(s, band_y0, xw[0], band_y1)); start = None; } _ => {} } } if let Some(s) = start { - runs.push(Rect::new(s, yw[0], *xs.last().unwrap_or(&canvas.x1), yw[1])); + runs.push(Rect::new(s, band_y0, *xs.last().unwrap_or(&canvas.x1), band_y1)); } } // Merge vertically adjacent runs with the same columns. @@ -299,6 +317,53 @@ mod tests { assert_eq!(auto_slices(canvas, &[]), vec![canvas]); } + #[test] + fn auto_slices_cover_exactly_with_many_overlapping_slices() { + // Several overlapping slices form a non-trivial grid. The per-band coverage pass must + // still tile the complement exactly: auto area + taken-union area == canvas, with no + // auto slice overlapping a taken rect or another auto slice. + let canvas = Rect::new(0, 0, 200, 150); + let taken = [Rect::new(10, 10, 90, 70), Rect::new(50, 40, 160, 120), Rect::new(120, 0, 200, 50), Rect::new(0, 100, 60, 150)]; + let auto = auto_slices(canvas, &taken); + // No auto slice intersects any taken rect. + for a in &auto { + for t in &taken { + assert!(a.intersect(t).is_empty(), "{a:?} overlaps taken {t:?}"); + } + } + // Auto slices do not overlap each other. + for (i, a) in auto.iter().enumerate() { + for b in &auto[i + 1..] { + assert!(a.intersect(b).is_empty(), "{a:?} overlaps {b:?}"); + } + } + // Auto area equals the canvas minus the union of taken rects (computed on the same grid). + let union_cells: i64 = { + // Sum cell areas covered by at least one taken rect, over the fine grid. + let mut xs = vec![canvas.x0, canvas.x1]; + let mut ys = vec![canvas.y0, canvas.y1]; + for t in &taken { + xs.extend([t.x0, t.x1]); + ys.extend([t.y0, t.y1]); + } + xs.sort_unstable(); + xs.dedup(); + ys.sort_unstable(); + ys.dedup(); + let mut sum = 0i64; + for yw in ys.windows(2) { + for xw in xs.windows(2) { + let covered = taken.iter().any(|r| xw[0] >= r.x0 && xw[0] < r.x1 && yw[0] >= r.y0 && yw[0] < r.y1); + if covered { + sum += i64::from(xw[1] - xw[0]) * i64::from(yw[1] - yw[0]); + } + } + } + sum + }; + assert_eq!(area(&auto) + union_cells, i64::from(canvas.width()) * i64::from(canvas.height())); + } + #[test] fn resolve_numbers_top_to_bottom() { let mut d = Document::new("site.psd", Size::new(100, 80), ColorMode::Rgb, SampleType::U8); diff --git a/crates/engine/src/actions_cmds.rs b/crates/engine/src/actions_cmds.rs index f831d63878..6f559a5659 100644 --- a/crates/engine/src/actions_cmds.rs +++ b/crates/engine/src/actions_cmds.rs @@ -81,6 +81,8 @@ pub struct ActionsFile { pub actions: Vec, } +pub const MAX_PLAYBACK_STEPS: u64 = 100_000; + /// Actions list and the recording / playback flags. Only `list` is persisted. #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] #[serde(default)] @@ -95,6 +97,9 @@ pub struct ActionState { /// Active action names, for cycle detection in engine and desktop playback. #[serde(skip)] pub playback_stack: Vec, + /// Number of steps executed by the current top-level playback tree. + #[serde(skip)] + pub playback_steps: u64, /// Bumped whenever `list` changes, so the preset store can skip unchanged writes. #[serde(skip)] pub rev: u64, @@ -143,6 +148,15 @@ pub fn save_target_closed(s: &Session, id: &str, target: Option bool { + if s.actions.playback_steps >= MAX_PLAYBACK_STEPS { + false + } else { + s.actions.playback_steps += 1; + true + } +} /// View-only menu commands recorded by the desktop shell. Headless playback reports /// them as unsupported instead of silently dropping them; Batch and droplets skip them. pub fn shell_view_command(id: &str) -> bool { @@ -348,6 +362,10 @@ fn run_recorded(s: &mut Session, action: &Action, from: usize) -> (u64, Option (u64, Option Result> { flush_recording(s)?; + if s.actions.playing == 0 { + s.actions.playback_steps = 0; + } let recording = s.actions.recording.take(); s.actions.playing = s.actions.playing.saturating_add(1); s.actions.playback_stack.push(action.name.clone()); @@ -1625,3 +1646,16 @@ mod tests { assert!(s.actions.list[0].color.is_empty(), "rejected colours change nothing"); } } + +#[cfg(test)] +mod security_tests { + use super::*; + + #[test] + fn playback_budget_is_shared_by_nested_calls() { + let mut session = Session::new(); + session.actions.playback_steps = MAX_PLAYBACK_STEPS - 1; + assert!(consume_playback_step(&mut session)); + assert!(!consume_playback_step(&mut session)); + } +} diff --git a/crates/engine/src/atn_cmds.rs b/crates/engine/src/atn_cmds.rs index 65ae9fefd2..f166c239b3 100644 --- a/crates/engine/src/atn_cmds.rs +++ b/crates/engine/src/atn_cmds.rs @@ -3907,12 +3907,34 @@ pub fn export( // ------------------------------------------------------------------ commands -/// A name not used by any action in `taken`: `name`, `name 2`, `name 3`… -fn unique(name: &str, taken: &[String]) -> String { - if !taken.iter().any(|t| t == name) { - return name.to_string(); +/// Assigns names not already used, as `name`, `name 2`, `name 3`… A plain linear scan of the +/// taken list per candidate is cubic when an action file repeats one base name many times, so +/// membership uses a set and the next suffix to try is cached per base name. +struct UniqueNames { + taken: std::collections::HashSet, + next_suffix: std::collections::HashMap, +} + +impl UniqueNames { + fn new>(existing: I) -> Self { + UniqueNames { taken: existing.into_iter().collect(), next_suffix: std::collections::HashMap::new() } + } + + /// Reserves and returns a unique name derived from `name`, recording it as taken. + fn claim(&mut self, name: &str) -> String { + if self.taken.insert(name.to_string()) { + return name.to_string(); + } + let mut i = self.next_suffix.get(name).copied().unwrap_or(2); + loop { + let candidate = format!("{name} {i}"); + i += 1; + if self.taken.insert(candidate.clone()) { + self.next_suffix.insert(name.to_string(), i); + return candidate; + } + } } - (2..).map(|i| format!("{name} {i}")).find(|n| !taken.contains(n)).unwrap_or_else(|| name.to_string()) } fn load(s: &mut Session, p: &Value) -> Result { @@ -3940,19 +3962,18 @@ fn load(s: &mut Session, p: &Value) -> Result { if replace { s.actions.list.retain(|a| a.set.as_deref() != Some(imp.set.as_str())); } - let mut taken: Vec = s.actions.list.iter().map(|a| a.name.clone()).collect(); + let mut taken = UniqueNames::new(s.actions.list.iter().map(|a| a.name.clone())); let mut names = Vec::with_capacity(imp.actions.len()); // The file's name of each action that had to be renamed, and its new name. let mut renamed: std::collections::HashMap = std::collections::HashMap::new(); let mut steps = 0usize; let mut added = Vec::with_capacity(imp.actions.len()); for mut a in imp.actions { - let name = unique(&a.name, &taken); + let name = taken.claim(&a.name); if name != a.name { renamed.entry(a.name.clone()).or_insert_with(|| name.clone()); } a.name = name.clone(); - taken.push(name.clone()); steps = steps.saturating_add(a.steps.len()); names.push(name); added.push(a); diff --git a/crates/engine/src/atn_cmds/tests.rs b/crates/engine/src/atn_cmds/tests.rs index b18cdeae81..d83d69e624 100644 --- a/crates/engine/src/atn_cmds/tests.rs +++ b/crates/engine/src/atn_cmds/tests.rs @@ -262,6 +262,18 @@ fn load_renames_each_play_target_once() { assert_eq!(s.actions.list[1].steps[0].1, json!({"action": "A"}), "B still plays the A it had"); } +#[test] +fn load_many_duplicate_names_get_sequential_suffixes() { + // Several actions share one base name. Deduplication must stay O(n) overall and produce + // the sequence A, A 2, A 3, A 4 (the next-suffix cache cannot skip or repeat a number). + let mut s = Session::new(); + let dupes: Vec = (0..4).map(|_| Action::new("A", vec![])).collect(); + let refs: Vec<&Action> = dupes.iter().collect(); + let (bytes, _) = export("File", &refs, &[], false).unwrap(); + let r = s.execute("actions.load", json!({"data": b64(&bytes)})).unwrap(); + assert_eq!(r["actions"], json!(["A", "A 2", "A 3", "A 4"])); +} + #[test] fn save_round_trips_through_photoshop_events() { let mut s = Session::new(); diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index b473210fc1..e3f6e815e0 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -1419,7 +1419,9 @@ fn batch(s: &mut Session, p: &Value) -> Result { let actions = crate::actions_cmds::ActionState { list, playing: s.actions.playing, playback_stack: s.actions.playback_stack.clone(), ..Default::default() }; let names = batch_naming::plan(&inputs, &output, &format, p)?; let save = SaveOpts::from_params(s, p, cmd)?; - let run = |scratch: &mut Session, out: &str| { + let authorize = s.authorize; + let run = move |scratch: &mut Session, out: &str| { + scratch.authorize = authorize; scratch.actions = actions.clone(); // The document as the last recorded save found it, written once the action has run so a // failing step still writes nothing. diff --git a/crates/engine/src/type_spell_cmds.rs b/crates/engine/src/type_spell_cmds.rs index 915b21229a..d42500c722 100644 --- a/crates/engine/src/type_spell_cmds.rs +++ b/crates/engine/src/type_spell_cmds.rs @@ -27,8 +27,30 @@ fn byte_at(text: &str, ci: usize) -> usize { text.char_indices().nth(ci).map_or(text.len(), |(b, _)| b) } -fn char_at(text: &str, bi: usize) -> usize { - text[..bi.min(text.len())].chars().count() +/// Maps ascending byte offsets to character offsets in a single forward pass. Calling +/// `char_at` per offset rescans the text prefix each time, which is quadratic when a layer +/// has many misspellings; this walks the text once for the whole batch. `offsets` must be +/// sorted ascending (misspelling start/end pairs are, since words are found left to right). +fn char_offsets(text: &str, offsets: &[usize]) -> Vec { + // (byte offset, char index) for each char boundary, plus the end-of-text boundary. + let mut boundaries = text.char_indices().map(|(b, _)| b); + let mut char_index = 0usize; + let mut current = boundaries.next(); + let mut result = Vec::with_capacity(offsets.len()); + for &target in offsets { + let clamped = target.min(text.len()); + // Advance to the first boundary at or past `clamped`; byte offsets from char + // boundaries land exactly on one, so the char index there is the answer. + while let Some(b) = current { + if b >= clamped { + break; + } + char_index += 1; + current = boundaries.next(); + } + result.push(char_index); + } + result } /// Type layers top to bottom (Layers panel order), skipping fully locked ones. @@ -76,10 +98,17 @@ fn list(s: &Session, p: &Value) -> Result { let mut out = Vec::new(); for id in layers_param(s, p) { let Some(t) = text(&st.doc, id) else { continue }; - for m in dict.misspellings(t, &ok) { - out.push( - json!({ "layer": id.0, "start": char_at(t, m.start), "end": char_at(t, m.end), "word": m.word, "suggestions": dict.suggest(&m.word, max) }), - ); + let found = dict.misspellings(t, &ok); + // Convert every start/end byte offset to a character offset in one pass over the + // text, instead of rescanning the prefix with char_at twice per misspelling. + let mut offsets = Vec::with_capacity(found.len() * 2); + for m in &found { + offsets.push(m.start); + offsets.push(m.end); + } + let chars = char_offsets(t, &offsets); + for (i, m) in found.iter().enumerate() { + out.push(json!({ "layer": id.0, "start": chars[i * 2], "end": chars[i * 2 + 1], "word": m.word, "suggestions": dict.suggest(&m.word, max) })); } } Ok(json!({ "misspellings": out, "count": out.len() })) @@ -263,6 +292,17 @@ pub fn specs() -> Vec { mod tests { use super::*; + #[test] + fn char_offsets_matches_per_offset_counting() { + // Multibyte text so byte offsets and char offsets differ. The batch result must + // equal counting chars in each prefix individually, including the end-of-text offset. + let text = "café naïve ☃ résumé"; + let targets: Vec = text.char_indices().map(|(b, _)| b).chain(std::iter::once(text.len())).collect(); + let got = char_offsets(text, &targets); + let want: Vec = targets.iter().map(|&b| text[..b.min(text.len())].chars().count()).collect(); + assert_eq!(got, want); + } + fn session(text: &str) -> (Session, u64) { let mut s = Session::new(); s.execute("file.new", json!({"width": 300, "height": 100, "background": "white"})).unwrap(); diff --git a/crates/engine/src/video_cmds.rs b/crates/engine/src/video_cmds.rs index 51c888b95d..7713e0d36f 100644 --- a/crates/engine/src/video_cmds.rs +++ b/crates/engine/src/video_cmds.rs @@ -227,16 +227,24 @@ struct Footage { /// do, since they would otherwise play at an unintended 50-100 fps. fn fps_from_delays(delays: &[u32]) -> Option { let clean = |d: u32| if d < 20 { 100 } else { d }; - let mut counts: Vec<(u32, usize)> = Vec::new(); + // Count delays with a map keyed on the cleaned delay, recording first-seen order + // separately. A linear scan per frame (iter_mut().find) was O(n^2) for a GIF with many + // distinct delays. order[] preserves the tie-break: the earliest-seen delay wins. + let mut counts: std::collections::HashMap = std::collections::HashMap::new(); + let mut order: Vec = Vec::new(); for d in delays.iter().copied().map(clean) { - match counts.iter_mut().find(|(k, _)| *k == d) { - Some((_, n)) => *n += 1, - None => counts.push((d, 1)), - } + let n = counts.entry(d).or_insert_with(|| { + order.push(d); + 0 + }); + *n += 1; } - let best = counts.iter().fold(None::<(u32, usize)>, |best, &(d, n)| match best { - Some((_, bn)) if bn >= n => best, - _ => Some((d, n)), + let best = order.iter().fold(None::<(u32, usize)>, |best, &d| { + let n = counts[&d]; + match best { + Some((_, bn)) if bn >= n => best, + _ => Some((d, n)), + } }); best.map(|(d, _)| 1000.0 / d as f32) } diff --git a/crates/engine/src/video_cmds/tests.rs b/crates/engine/src/video_cmds/tests.rs index 866d1c836b..a3f304ce8e 100644 --- a/crates/engine/src/video_cmds/tests.rs +++ b/crates/engine/src/video_cmds/tests.rs @@ -653,3 +653,19 @@ fn render_video_rejects_gif_frames_too_large_to_stage() { let err = s.execute("file.export.renderVideo", json!({"dir": tmpdir("large-gif"), "format": "gif"})).unwrap_err(); assert!(err.to_string().contains("too large"), "{err}"); } + +#[test] +fn fps_from_delays_picks_most_common_and_breaks_ties_by_first_seen() { + // Most common delay wins: 100 ms appears twice, 40 ms once -> 10 fps. + assert_eq!(fps_from_delays(&[100, 40, 100]), Some(10.0)); + // Tie between 40 ms (25 fps) and 50 ms (20 fps): the first seen (40) wins. + assert_eq!(fps_from_delays(&[40, 50]), Some(25.0)); + // Delays under 20 ms are treated as 100 ms. + assert_eq!(fps_from_delays(&[0, 10, 5]), Some(10.0)); + assert_eq!(fps_from_delays(&[]), None); + // Many distinct delays used to be quadratic; this must stay fast and correct. Delay 20 + // (the one appearing twice) wins at 50 fps. + let mut many: Vec = (20..5_020).collect(); + many.push(20); + assert_eq!(fps_from_delays(&many), Some(50.0)); +} diff --git a/crates/io/src/comps_map.rs b/crates/io/src/comps_map.rs index 4cade56e2e..3348061b34 100644 --- a/crates/io/src/comps_map.rs +++ b/crates/io/src/comps_map.rs @@ -12,6 +12,8 @@ //! Both are re-emitted verbatim while they still decode to the document's state, and are //! regenerated otherwise (comp appearance is not written to PSD; `.pcraft` keeps it). +use std::collections::HashMap; + use photocraft_doc::comps::layer_position; use photocraft_doc::{Artboard, ArtboardBackground, CompLayerState, Document, Guides, Layer, LayerComp}; use photocraft_geom::Rect; @@ -203,6 +205,14 @@ pub(crate) fn decode_comps(resource: Option<&[u8]>, doc: &Document) -> (Vec = HashMap::with_capacity(comps.len()); + for (i, c) in comps.iter().enumerate() { + id_to_index.entry(c.id).or_insert(i); + } let mut last = LayerComp { id: 0, name: "Last Document State".into(), @@ -239,7 +249,7 @@ pub(crate) fn decode_comps(resource: Option<&[u8]>, doc: &Document) -> (Vec None, }; for id in &s.comps { - let target = if *id == 0 { Some(&mut last) } else { comps.iter_mut().find(|c| c.id == *id) }; + let target = if *id == 0 { Some(&mut last) } else { id_to_index.get(id).map(|&i| &mut comps[i]) }; let Some(c) = target else { continue }; let visible = if *id != 0 && c.apply_visibility { Some(resolved) } else { s.visible }; if visible.is_none() && s.offset.is_none() { diff --git a/crates/io/src/pdn.rs b/crates/io/src/pdn.rs index 08e294b7bc..1e2d99f7af 100644 --- a/crates/io/src/pdn.rs +++ b/crates/io/src/pdn.rs @@ -154,6 +154,12 @@ pub(super) fn import(name: &str, bytes: &[u8], ctl: &Interrupt<'_>) -> Result) -> Result 255 { return Err(invalid("opacity exceeds 255")); } - let mut l = Layer::raster(graph.string(graph.field(props, "name")?)?, PixelFormat::RGBA8); + let mut l = Layer::raster( + { + let name = graph.string(graph.field(props, "name")?)?; + if name.len() > MAX_NAME_BYTES { + return Err(invalid("layer name is too long")); + } + name_bytes = name_bytes.saturating_add(name.len()); + if name_bytes > MAX_TOTAL_NAME_BYTES { + return Err(invalid("layer names exceed the total metadata budget")); + } + name.to_string() + }, + PixelFormat::RGBA8, + ); l.visible = graph.boolean(graph.field(props, "visible")?)?; l.opacity = opacity as f32 / 255.0; l.blend = blend(&graph, layer, props)?; diff --git a/crates/io/src/raw.rs b/crates/io/src/raw.rs index 420b3f2cf7..04158deddc 100644 --- a/crates/io/src/raw.rs +++ b/crates/io/src/raw.rs @@ -36,8 +36,13 @@ fn limits() -> Limits { /// has one; otherwise the raw's IFD0 orientation applies (TIFF-based raws record it there, and /// their previews are usually stored as the sensor reads out). Developed raws are oriented by /// `photocraft-raw` and their EXIF carries no orientation, so nothing is turned twice. -fn upright_preview(raw: &[u8], jpeg: &[u8]) -> Result { - let img = codecs::decode_as_with(Format::Jpeg, jpeg, &codecs::DecodeOptions { keep_orientation: true, ..Default::default() })?; +fn upright_preview(raw: &[u8], jpeg: &[u8], limits: &photocraft_raw::Limits) -> Result { + // Decode the embedded preview under the caller's limits, not the codec defaults: an + // embedded JPEG can declare far more decoded pixels than the raw development the caller + // authorized, so the preview fallback must honor the same policy. + let codec_limits = + codecs::Limits { max_width: limits.max_width, max_height: limits.max_height, max_pixels: limits.max_pixels, max_alloc: limits.max_alloc }; + let img = codecs::decode_as_with(Format::Jpeg, jpeg, &codecs::DecodeOptions { keep_orientation: true, limits: codec_limits })?; let own = img.meta.exif.as_deref().map_or(1, codecs::exif_orientation); let o = if own != 1 { own } else { photocraft_raw::orientation(raw).unwrap_or_else(|| codecs::exif_orientation(raw)) }; Ok(img.oriented(o)?) @@ -223,7 +228,7 @@ pub fn fit_preview_look(raw: &[u8], sensor: &Sensor, opts: &DevelopOptions) -> R .min_by_key(|p| long(p)) .or_else(|| previews.iter().max_by_key(|p| long(p))) .ok_or("no embedded JPEG preview")?; - let img = upright_preview(raw, pick.jpeg).map_err(|e| format!("the embedded preview does not decode: {e}"))?; + let img = upright_preview(raw, pick.jpeg, &opts.limits).map_err(|e| format!("the embedded preview does not decode: {e}"))?; if img.layout() != ChannelLayout::Rgb || img.sample_type() != SampleType::U8 { return Err("the embedded preview is not 8-bit RGB".into()); } @@ -277,7 +282,7 @@ pub fn import_raw_with(name: &str, bytes: &[u8], opts: &DevelopOptions) -> Resul } Err(RawError::Unsupported(reason)) => match photocraft_raw::embedded_preview(bytes) { Some(p) => { - let img = upright_preview(bytes, p.jpeg)?; + let img = upright_preview(bytes, p.jpeg, &opts.limits)?; let mut r = image_to_document(name, &img)?; r.warnings.insert( 0, diff --git a/crates/io/src/xcf.rs b/crates/io/src/xcf.rs index 15586a55dd..d5cda4aa4d 100644 --- a/crates/io/src/xcf.rs +++ b/crates/io/src/xcf.rs @@ -17,6 +17,7 @@ #[cfg(test)] mod tests; +use std::collections::HashMap; use std::io::Read; use std::sync::Arc; @@ -42,6 +43,10 @@ pub(crate) const MAX_LAYERS: usize = 8192; const MAX_CHANNELS: usize = 1024; /// Group nesting. const MAX_DEPTH: usize = 64; +/// Total copied layer metadata (names and item paths). A single reused record with a large +/// name is bounded by MAX_STRING, but repeated layer offsets referencing it must not multiply +/// into unbounded retained metadata. +const MAX_METADATA_BYTES: usize = 32 << 20; /// A string field (layer names, parasite names). const MAX_STRING: usize = 1 << 20; /// A property payload or a parasite that is read into memory. @@ -290,13 +295,21 @@ fn be_f32(b: &[u8]) -> Option { fn parasites(data: &[u8]) -> Vec<(String, Vec)> { let mut c = Cursor { b: data, pos: 0, wide: false }; let mut out: Vec<(String, Vec)> = Vec::new(); + // Map each name to its output slot so duplicate replacement is O(1). The parasite + // records are file-controlled, so scanning `out` with iter_mut().find() per record is + // O(n^2) and lets a crafted payload with many distinct names stall import. The map + // preserves insertion order and last-value-wins behavior. + let mut index: HashMap = HashMap::new(); while c.pos < data.len() { let Ok(name) = c.string() else { break }; let (Ok(_flags), Ok(len)) = (c.u32(), c.u32()) else { break }; let Ok(payload) = c.take(len as usize) else { break }; - match out.iter_mut().find(|(n, _)| *n == name) { - Some(slot) => slot.1 = payload.to_vec(), - None => out.push((name, payload.to_vec())), + match index.get(&name) { + Some(&i) => out[i].1 = payload.to_vec(), + None => { + index.insert(name.clone(), out.len()); + out.push((name, payload.to_vec())); + } } } out @@ -381,6 +394,10 @@ struct Image<'a> { base_type: u32, colormap: Vec<[u8; 3]>, budget: usize, + /// Remaining allowance for copied layer metadata (names, item paths). Repeated layer + /// offsets referencing one record with a large name would otherwise allocate an owned + /// copy per reference, multiplying a small file into gigabytes of retained metadata. + meta_budget: usize, warnings: Warnings, /// Layers GIMP composites with "Clip to backdrop" (its default for every mode but Normal). clipped_to_backdrop: usize, @@ -685,7 +702,18 @@ pub(crate) fn import(name: &str, bytes: &[u8], ctl: &Interrupt<'_>) -> Result Image<'a> { let mut c = self.c.at(ptr); let (lw, lh, kind) = (c.u32()?, c.u32()?, c.u32()?); let name = c.string()?; + // Charge the owned name against the document-wide metadata budget: repeated layer + // offsets referencing one large name must not retain unbounded copies. + self.meta_budget = self.meta_budget.checked_sub(name.len()).ok_or_else(|| invalid("layer metadata exceeds the budget"))?; let lprops = props(&mut c)?; let hptr = c.ptr()?; let mptr = c.ptr()?; @@ -902,7 +933,11 @@ impl<'a> Image<'a> { 26 => text = true, 28 => locks.pixels = v != 0, 29 => group = true, - 30 => path = p.data.as_chunks::<4>().0.iter().map(|s| u32::from_be_bytes(*s)).collect(), + 30 => { + path = p.data.as_chunks::<4>().0.iter().map(|s| u32::from_be_bytes(*s)).collect(); + self.meta_budget = + self.meta_budget.checked_sub(path.len().saturating_mul(4)).ok_or_else(|| invalid("layer metadata exceeds the budget"))?; + } 31 => expanded = v & 1 != 0, 32 => locks.position = v != 0, 34 => tag = label(v), diff --git a/crates/io/src/xcf/tests.rs b/crates/io/src/xcf/tests.rs index ee96f16509..400887f92b 100644 --- a/crates/io/src/xcf/tests.rs +++ b/crates/io/src/xcf/tests.rs @@ -702,3 +702,33 @@ fn a_path_failing_below_the_top_level_lands_at_the_top_level() { assert_eq!(children, ["A"], "the valid sibling placed after the stray one still lands in the group"); assert!(r.warnings.iter().any(|w| w.contains("Stray") && w.contains("item path")), "{:?}", r.warnings); } + +/// A parasite name field: u32 `len+1`, then the bytes and a trailing NUL. +fn parasite_name(out: &mut Vec, name: &str) { + out.extend_from_slice(&((name.len() as u32) + 1).to_be_bytes()); + out.extend_from_slice(name.as_bytes()); + out.push(0); +} + +/// One PROP_PARASITES record: name, u32 flags, u32 len, then the payload. +fn parasite_record(out: &mut Vec, name: &str, payload: &[u8]) { + parasite_name(out, name); + out.extend_from_slice(&0u32.to_be_bytes()); + out.extend_from_slice(&(payload.len() as u32).to_be_bytes()); + out.extend_from_slice(payload); +} + +#[test] +fn duplicate_parasites_keep_order_and_last_value() { + // Duplicate names must resolve to the last payload while keeping first-seen order. + // This is the behavior the name->index map must preserve versus the old linear scan. + let mut data = Vec::new(); + parasite_record(&mut data, "gimp-comment", b"first"); + parasite_record(&mut data, "gimp-image-grid", b"grid"); + parasite_record(&mut data, "gimp-comment", b"second"); + let out = parasites(&data); + assert_eq!(out.len(), 2, "duplicate collapses to one slot"); + assert_eq!(out[0].0, "gimp-comment"); + assert_eq!(out[0].1, b"second", "last value wins"); + assert_eq!(out[1].0, "gimp-image-grid", "insertion order preserved"); +} diff --git a/crates/ipp/src/lib.rs b/crates/ipp/src/lib.rs index c778807d03..d0535a86ce 100644 --- a/crates/ipp/src/lib.rs +++ b/crates/ipp/src/lib.rs @@ -521,8 +521,19 @@ fn dechunk(mut b: &[u8]) -> Result> { } /// Where `/path` lands on the server: `/printers/` or `/classes/`. -fn queue_path(name: &str, is_class: bool) -> String { - format!("/{}/{name}", if is_class { "classes" } else { "printers" }) +fn queue_path(name: &str, is_class: bool) -> Result { + if name.is_empty() || name.bytes().any(|b| b < 0x20 || b == 0x7f) { + return Err(Error::Protocol("printer name contains an invalid control character".into())); + } + let mut encoded = String::with_capacity(name.len()); + for &b in name.as_bytes() { + if b.is_ascii_alphanumeric() || matches!(b, b'-' | b'.' | b'_' | b'~') { + encoded.push(char::from(b)); + } else { + encoded.push_str(&format!("%{b:02X}")); + } + } + Ok(format!("/{}/{encoded}", if is_class { "classes" } else { "printers" })) } /// The printers the server knows (CUPS-Get-Printers), then its classes (CUPS-Get-Classes, which @@ -609,7 +620,7 @@ pub struct Job<'a> { /// Submits `job` (Print-Job) and returns the job id. pub fn print_job(endpoint: &Endpoint, job: &Job<'_>) -> Result { let mut r = Request::new(OP_PRINT_JOB, 3); - let path = queue_path(job.printer, job.is_class); + let path = queue_path(job.printer, job.is_class)?; r.attr(TAG_URI, "printer-uri", format!("ipp://{}{path}", endpoint.host()).as_bytes()); r.attr(TAG_NAME, "requesting-user-name", job.user.as_bytes()); r.attr(TAG_NAME, "job-name", job.title.as_bytes()); diff --git a/crates/ipp/src/tests.rs b/crates/ipp/src/tests.rs index 222e736318..394ff758d1 100644 --- a/crates/ipp/src/tests.rs +++ b/crates/ipp/src/tests.rs @@ -471,3 +471,9 @@ fn an_impossible_chunk_size_is_a_protocol_error_not_a_panic() { } } } + +#[test] +fn printer_names_are_encoded_as_one_request_target_segment() { + assert_eq!(queue_path("office printer/1", false).unwrap(), "/printers/office%20printer%2F1"); + assert!(queue_path("printer\r\nX-Injected: yes", false).is_err()); +} diff --git a/crates/psd/src/filter_effects.rs b/crates/psd/src/filter_effects.rs index e8b5efb698..cadb3ac8d0 100644 --- a/crates/psd/src/filter_effects.rs +++ b/crates/psd/src/filter_effects.rs @@ -170,7 +170,10 @@ impl FilterEffects { let pad = ((4 - len % 4) % 4).min(r.remaining() as u64); r.skip(pad as usize)?; let rest = r.peek_rest(); - if rest.iter().all(|&b| b == 0) && rest.len() <= 3 { + // Padding is at most three bytes. Check the length before scanning the suffix for + // zeros: otherwise each iteration scans the whole remaining buffer, which is + // quadratic when a block contains many empty sections. + if rest.len() <= 3 && rest.iter().all(|&b| b == 0) { break; } if rest.len() < 8 { diff --git a/crates/raw/src/develop.rs b/crates/raw/src/develop.rs index 5d28508750..71a8384cd0 100644 --- a/crates/raw/src/develop.rs +++ b/crates/raw/src/develop.rs @@ -460,12 +460,11 @@ fn gain_row(s: &Sensor, y: usize, x0: usize, w: usize, k: usize) -> Option= m.plane.saturating_add(m.planes) || ay < m.top || ay >= m.bottom || !(ay - m.top).is_multiple_of(m.row_pitch) { continue; } - let row = m.row(rv, k - m.plane); let o = out.get_or_insert_with(|| vec![1.0; w]); for (i, f) in o.iter_mut().enumerate() { let x = x0 + i; if x >= a.x && m.covers(x - a.x, ay) { - *f *= m.at(&row, ((x - a.x) as f64 + 0.5) / a.width as f64); + *f *= m.at_rv(rv, k - m.plane, ((x - a.x) as f64 + 0.5) / a.width as f64); } } } diff --git a/crates/raw/src/ljpeg.rs b/crates/raw/src/ljpeg.rs index e9c42bdeb4..4ab32df8b8 100644 --- a/crates/raw/src/ljpeg.rs +++ b/crates/raw/src/ljpeg.rs @@ -299,6 +299,11 @@ fn parse(data: &[u8]) -> Result> { let mut pos = 2; let mut frame: Option = None; let mut comp_ids: Vec = Vec::new(); + // A lossless JPEG uses at most a handful of Huffman tables. Each Huffman::new allocates a + // ~48 KiB lookup pair and fills it, so a stream that repeats DHT definitions could rebuild + // tables thousands of times. Cap the cumulative builds to bound that header-processing work. + const MAX_HUFFMAN_BUILDS: usize = 64; + let mut huffman_builds = 0usize; let mut huff: [Option; 4] = [None, None, None, None]; let mut restart = 0usize; loop { @@ -355,6 +360,10 @@ fn parse(data: &[u8]) -> Result> { let total: usize = counts.iter().map(|&c| usize::from(c)).sum(); let vals = seg.get(p + 17..p + 17 + total).ok_or_else(|| RawError::malformed("short DHT"))?; if id < 4 { + huffman_builds += 1; + if huffman_builds > MAX_HUFFMAN_BUILDS { + return Err(RawError::malformed("lossless JPEG defines too many Huffman tables")); + } huff[id] = Some(Huffman::new(&counts, vals)?); } p += 17 + total; @@ -573,4 +582,26 @@ mod tests { let lossy = [0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x0B, 8, 0, 1, 0, 1, 1, 1, 0x11, 0]; assert!(matches!(decode(&lossy, usize::MAX), Err(RawError::Unsupported(_)))); } + + #[test] + fn too_many_huffman_tables_are_rejected() { + // A valid stream still decodes; the same stream with thousands of repeated DHT + // definitions injected after the SOI is rejected before rebuilding tables endlessly. + let (w, h) = (8, 8); + let v = samples(w * h, 12, 5); + let j = lj92_encode(&v, w, h, 1, 12, 1, None); + assert!(decode(&j, usize::MAX).is_ok()); + + // Minimal DHT for id 0 (one length-1 code, value 0): FF C4, len=0x0014, then + // tc_th, 16 count bytes, one value. + let mut dht = vec![0xFF, 0xC4, 0x00, 0x14, 0x00]; + dht.extend_from_slice(&[1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]); // counts + dht.push(0x00); // one value + let mut flooded = j[..2].to_vec(); // SOI + for _ in 0..5000 { + flooded.extend_from_slice(&dht); + } + flooded.extend_from_slice(&j[2..]); // the rest of the valid stream + assert!(matches!(decode(&flooded, usize::MAX), Err(RawError::Malformed(_)))); + } } diff --git a/crates/raw/src/opcodes.rs b/crates/raw/src/opcodes.rs index 9dc231ca61..6be6d45077 100644 --- a/crates/raw/src/opcodes.rs +++ b/crates/raw/src/opcodes.rs @@ -58,6 +58,27 @@ impl GainMap { a + (b - a) * t } + /// Bilinearly interpolates the gain at image-relative row `rv` and column `rh` for map + /// plane `mp`, touching only the four surrounding grid points. This avoids building the + /// whole `points_h`-wide row per image row, which made development work proportional to + /// image height times horizontal map size even when each output row needs few values. + #[inline] + pub fn at_rv(&self, rv: f64, mp: usize, rh: f64) -> f32 { + let mp = mp.min(self.map_planes.saturating_sub(1)); + let fv = ((rv - self.origin_v) / self.spacing_v).clamp(0.0, (self.points_v - 1) as f64); + let i0 = (fv.floor() as usize).min(self.points_v - 1); + let i1 = (i0 + 1).min(self.points_v - 1); + let tv = (fv - i0 as f64) as f32; + let fh = ((rh - self.origin_h) / self.spacing_h).clamp(0.0, (self.points_h - 1) as f64); + let j0 = (fh.floor() as usize).min(self.points_h - 1); + let j1 = (j0 + 1).min(self.points_h - 1); + let th = (fh - j0 as f64) as f32; + let g = |i: usize, j: usize| self.gains.get((i * self.points_h + j) * self.map_planes + mp).copied().unwrap_or(1.0); + let top = g(i0, j0) + (g(i0, j1) - g(i0, j0)) * th; + let bot = g(i1, j0) + (g(i1, j1) - g(i1, j0)) * th; + top + (bot - top) * tv + } + /// `true` if the map covers active-area pixel (`ax`, `ay`). #[inline] pub fn covers(&self, ax: usize, ay: usize) -> bool { @@ -216,4 +237,21 @@ mod tests { huge[4 + 16 + 32..4 + 16 + 40].copy_from_slice(&[0xFF; 8]); // points assert!(parse(&huge).0.is_empty()); } + + #[test] + fn at_rv_matches_row_then_at() { + // at_rv (direct 4-corner bilinear) must equal the old row() + at() path for a range of + // sample positions, including points outside the grid that clamp. + let b = list(&[(9, gain_map_params([0, 0, 10, 10], 1, &[[1.0, 2.0], [3.0, 4.5]]))]); + let (maps, _) = parse(&b); + let m = &maps[0]; + for &rv in &[-1.0, 0.0, 0.17, 0.5, 0.83, 1.0, 2.0] { + let row = m.row(rv, 0); + for &rh in &[-1.0, 0.0, 0.1, 0.5, 0.9, 1.0, 3.0] { + let want = m.at(&row, rh); + let got = m.at_rv(rv, 0, rh); + assert!((got - want).abs() < 1e-6, "rv={rv} rh={rh}: {got} vs {want}"); + } + } + } } diff --git a/crates/raw/src/preview.rs b/crates/raw/src/preview.rs index a090dee6c7..e85a2851fd 100644 --- a/crates/raw/src/preview.rs +++ b/crates/raw/src/preview.rs @@ -74,12 +74,13 @@ pub fn embedded_previews(bytes: &[u8]) -> Vec> { if let (Some(off), Some(len)) = (t.tag_uint(&ifd, tag::JPEG_INTERCHANGE_FORMAT), t.tag_uint(&ifd, tag::JPEG_INTERCHANGE_FORMAT_LENGTH)) { found.extend(candidate(bytes, off as usize, len as usize)); } - if matches!(t.tag_uint(&ifd, tag::COMPRESSION), Some(6 | 7)) { - let offs = t.tag_uints(&ifd, tag::STRIP_OFFSETS); - let lens = t.tag_uints(&ifd, tag::STRIP_BYTE_COUNTS); - if let ([off], [len]) = (offs.as_slice(), lens.as_slice()) { - found.extend(candidate(bytes, *off as usize, *len as usize)); - } + if matches!(t.tag_uint(&ifd, tag::COMPRESSION), Some(6 | 7)) + && let (Some(offsets), Some(lengths)) = (ifd.get(tag::STRIP_OFFSETS), ifd.get(tag::STRIP_BYTE_COUNTS)) + && offsets.count == 1 + && lengths.count == 1 + && let (Some(off), Some(len)) = (t.uint(offsets, 0), t.uint(lengths, 0)) + { + found.extend(candidate(bytes, off as usize, len as usize)); } // Panasonic RW2 JpgFromRaw (0x002E) holds a complete JPEG. if let Some(e) = ifd.get(0x002E).filter(|e| e.typ == 7) diff --git a/crates/raw/src/rw2.rs b/crates/raw/src/rw2.rs index c50d32c09c..ac9ada4189 100644 --- a/crates/raw/src/rw2.rs +++ b/crates/raw/src/rw2.rs @@ -138,10 +138,20 @@ struct Bits<'a> { } impl<'a> Bits<'a> { + #[cfg(test)] fn new(buf: &'a [u8], base: usize) -> Self { Bits { buf, base, pos: 0, nbits: 0 } } + /// Positions the cursor `skip_in_page` bytes into the page at `base`, in constant time. + /// Equivalent to `new(buf, base)` followed by `skip_in_page` calls of `get(8)`: the first + /// read loads one page (`pos = PAGE`), and each byte skipped consumes 8 bits. Narrow images + /// repeated this replay per row, which made cursor setup quadratic. + fn new_at(buf: &'a [u8], base: usize, skip_in_page: usize) -> Self { + debug_assert!(skip_in_page < PAGE); + Bits { buf, base, pos: PAGE, nbits: PAGE as u32 * 8 - 8 * skip_in_page as u32 } + } + #[inline] fn get(&mut self, num: u32) -> u32 { if num > self.nbits { @@ -162,10 +172,7 @@ impl<'a> Bits<'a> { /// Decodes one row of the vendor-compressed stream. fn compressed_row(buf: &[u8], row_bytes: usize, row: usize, out: &mut [u16]) { let skip = row_bytes * row; - let mut bits = Bits::new(buf, skip / PAGE * PAGE); - for _ in 0..(skip % PAGE) { - bits.get(8); - } + let mut bits = Bits::new_at(buf, skip / PAGE * PAGE, skip % PAGE); let mut sh: i32 = 0; for group in out.as_chunks_mut::<14>().0 { let mut pred = [0i32; 2]; @@ -354,6 +361,29 @@ mod tests { assert_eq!(b.get(8), 0x55); } + #[test] + fn new_at_matches_byte_replay() { + // new_at must position the cursor identically to new() followed by skip_in_page + // get(8) calls, so the quadratic per-row replay can be removed safely. + let mut buf = vec![0u8; 0x8000]; + for (i, b) in buf.iter_mut().enumerate() { + *b = (i * 7 + 3) as u8; + } + for base in [0usize, PAGE, 2 * PAGE] { + for skip in [0usize, 1, 7, 100, PAGE - 1] { + let mut replay = Bits::new(&buf, base); + for _ in 0..skip { + replay.get(8); + } + let mut direct = Bits::new_at(&buf, base, skip); + for _ in 0..20 { + assert_eq!(replay.get(5), direct.get(5), "base {base} skip {skip}"); + assert_eq!(replay.get(8), direct.get(8), "base {base} skip {skip}"); + } + } + } + } + #[test] fn compressed_group_walks_predictors() { // One 14-pixel group: both parities start absolute, a delta of 0x80 diff --git a/crates/raw/src/sensor.rs b/crates/raw/src/sensor.rs index f2cffdb808..cab5ce2a05 100644 --- a/crates/raw/src/sensor.rs +++ b/crates/raw/src/sensor.rs @@ -255,6 +255,19 @@ pub(crate) fn read_plane(t: &Tiff, ifd: &Ifd, limits: &Limits, layout: JpegLayou } limits.check(width as u64, height as u64, samples as u64 * 2)?; let segments = segments(t, ifd, width, height)?; + // The decoded tiles are all retained in `decoded` before assembly copies only the useful + // pixels. Each tile is charged individually below, but file-controlled padding and repeated + // source ranges can make the *cumulative* retained samples far exceed both the input and the + // useful image. Reject that up front: the sum of padded tile samples (two bytes each) must + // fit the allocation budget. + let mut total_tile_samples: u64 = 0; + for s in &segments { + let tile = (s.w as u64).saturating_mul(s.h as u64).saturating_mul(samples as u64); + total_tile_samples = total_tile_samples.saturating_add(tile); + } + if total_tile_samples.saturating_mul(2) > limits.max_alloc { + return Err(RawError::LimitExceeded(format!("{} bytes of retained raw tiles exceed {}", total_tile_samples.saturating_mul(2), limits.max_alloc))); + } let le = t.le; let tiled = t.tag_uint(ifd, tag::TILE_WIDTH).is_some(); let decoded: Vec>> = par::map(segments.len(), |i| { diff --git a/crates/raw/src/sony.rs b/crates/raw/src/sony.rs index e1961680df..b00fd0360a 100644 --- a/crates/raw/src/sony.rs +++ b/crates/raw/src/sony.rs @@ -114,11 +114,23 @@ pub(crate) fn read_craw(t: &Tiff, ifd: &Ifd, limits: &Limits) -> Result { } let lut = tone_lut(&t.tag_uints(ifd, SONY_TONE_CURVE)).ok_or_else(|| RawError::unsupported("Sony compressed ARW without a usable tone curve (0x7010)"))?; limits.check(width as u64, height as u64, 2)?; + // The cRAW reader treats every segment as a sequence of full-width image rows. A tiled + // layout does not describe full-width rows, so accepting one would let repeated tile + // offsets push far more than `height` row references before the coverage check below. + // cRAW is only stored as strips, so reject tiles outright. + if t.tag_uint(ifd, tag::TILE_WIDTH).is_some() || t.tag_uint(ifd, tag::TILE_LENGTH).is_some() { + return Err(RawError::unsupported("tiled Sony compressed ARW is not supported")); + } // One byte per pixel: locate every row in the strips. let mut rows: Vec<&[u8]> = Vec::with_capacity(height); for s in segments(t, ifd, width, height)? { let n = s.h.min(height.saturating_sub(s.y)); for r in 0..n { + // Never collect more rows than the image has; a malformed strip layout cannot + // grow the reference buffer past the image height. + if rows.len() >= height { + return Err(RawError::malformed("compressed ARW strips describe more rows than the image")); + } let at = s.offset.checked_add(r * width).ok_or_else(|| RawError::malformed("raw strip offset overflows"))?; if (r + 1) * width > s.len { return Err(RawError::malformed("compressed ARW strip is shorter than its rows")); diff --git a/crates/raw/src/tiff.rs b/crates/raw/src/tiff.rs index 2f685ccb2e..ca190a92f0 100644 --- a/crates/raw/src/tiff.rs +++ b/crates/raw/src/tiff.rs @@ -214,9 +214,11 @@ impl<'a> Tiff<'a> { } for t in [tag::SUB_IFDS, tag::EXIF_IFD] { if let Some(e) = ifd.get(t) { - // Reverse so the first SubIFD is visited first. - let mut v = self.uints(e); - v.truncate(16); + // Linked-directory traversal only needs the first sixteen offsets. Read + // those values directly instead of expanding the complete file-controlled + // array and truncating it afterward. + let n = (e.count as usize).min(16); + let v = (0..n).map(|i| self.uint(e, i)).collect::>>().unwrap_or_default(); stack.extend(v.into_iter().rev().map(|o| o as usize)); } } @@ -329,13 +331,32 @@ pub(crate) fn orientation(v: Option) -> u16 { /// A human-readable listing of every IFD and entry (debugging aid). pub(crate) fn dump(data: &[u8]) -> String { use std::fmt::Write; + // Caps so a crafted TIFF (many entries aliasing one large ASCII value) cannot expand a + // small diagnostic input into a multi-gigabyte String. + const MAX_ASCII_PREVIEW: usize = 256; + const MAX_OUTPUT: usize = 1 << 20; let Some(t) = Tiff::new(data) else { return "not a TIFF-structured file".into() }; let mut s = String::new(); - for ifd in t.all_ifds() { + 'ifds: for ifd in t.all_ifds() { let _ = writeln!(s, "IFD @{}", ifd.offset); for e in &ifd.entries { + if s.len() >= MAX_OUTPUT { + s.push_str("… (output truncated)\n"); + break 'ifds; + } let preview: String = if e.typ == 2 { - t.ascii(e).unwrap_or_default() + let mut a = t.ascii(e).unwrap_or_default(); + if a.len() > MAX_ASCII_PREVIEW { + // Truncate at a char boundary at or below the cap (lossy UTF-8 can hold + // multi-byte replacement chars). + let mut cut = MAX_ASCII_PREVIEW; + while cut > 0 && !a.is_char_boundary(cut) { + cut -= 1; + } + a.truncate(cut); + a.push('…'); + } + a } else { let n = (e.count as usize).min(12); let v: Vec = (0..n).map(|i| t.float(e, i).map(|f| format!("{f}")).unwrap_or_else(|| "?".into())).collect(); @@ -419,4 +440,50 @@ mod tests { let ifd = t.ifd_at(8, 0).unwrap(); assert_eq!(t.float(ifd.get(50730).unwrap(), 0), None); } + + #[test] + fn dump_output_is_bounded_for_aliased_ascii() { + // Many ASCII entries all pointing at one large value must not expand the diagnostic + // dump without bound. Build an IFD whose entries alias a big ASCII blob. + const ENTRIES: usize = 4000; + const BLOB: usize = 100_000; + let header = 8u32; + let ifd_at = header; + let entry_count_bytes = 2; + let entries_bytes = ENTRIES * 12; + let next_ptr_bytes = 4; + let blob_at = ifd_at as usize + entry_count_bytes + entries_bytes + next_ptr_bytes; + let mut b = b"II*\0".to_vec(); + b.extend_from_slice(&ifd_at.to_le_bytes()); + b.extend_from_slice(&(ENTRIES as u16).to_le_bytes()); + for i in 0..ENTRIES { + b.extend_from_slice(&(1000 + i as u16).to_le_bytes()); // tag + b.extend_from_slice(&2u16.to_le_bytes()); // type ASCII + b.extend_from_slice(&(BLOB as u32).to_le_bytes()); // count + b.extend_from_slice(&(blob_at as u32).to_le_bytes()); // value offset (aliased) + } + b.extend_from_slice(&0u32.to_le_bytes()); // next IFD = none + b.resize(blob_at + BLOB, b'A'); + let out = dump(&b); + // Comfortably below entries * blob (~400 MB); the caps keep it near 1 MiB. + assert!(out.len() < 2 << 20, "dump output was {} bytes", out.len()); + assert!(out.contains("output truncated") || out.len() < 1 << 20); + } +} + +#[test] +fn linked_directory_values_are_read_only_as_needed() { + // The traversal limit is independent of the TIFF entry count: callers should + // never need a complete attacker-controlled offset array to visit SubIFDs. + let mut b = b"II*\0\x08\0\0\0".to_vec(); + b.extend_from_slice(&1u16.to_le_bytes()); + b.extend_from_slice(&330u16.to_le_bytes()); + b.extend_from_slice(&4u16.to_le_bytes()); + b.extend_from_slice(&(1_000_000u32).to_le_bytes()); + b.extend_from_slice(&26u32.to_le_bytes()); + b.extend_from_slice(&0u32.to_le_bytes()); + b.extend_from_slice(&0u32.to_le_bytes()); + b.resize(26 + 16 * 4, 0); + let t = Tiff::new(&b).unwrap(); + assert!(t.all_ifds().len() <= 17); } diff --git a/crates/ui-egui/src/actions.rs b/crates/ui-egui/src/actions.rs index 5acacc6939..3e946b0d72 100644 --- a/crates/ui-egui/src/actions.rs +++ b/crates/ui-egui/src/actions.rs @@ -125,6 +125,10 @@ fn play_owned(app: &mut PhotocraftApp, params: &Value, suspended: Option) let mut failed = None; let mut paused = None; for (i, (id, p)) in action.enabled_steps().skip_while(|(i, _)| *i < from) { + if !actions_cmds::consume_playback_step(&mut app.session) { + failed = Some(json!({"step": i, "id": id, "error": format!("action playback exceeded the {}-step limit", actions_cmds::MAX_PLAYBACK_STEPS)})); + break; + } // The dialog toggle: show the step's dialog with its recorded values and continue // from the next step when it is confirmed (`resume`). Agents play headlessly. if action.dialogs.contains(&i) && !app.automation_input && (crate::filter_dialog::has_dialog(id) || crate::adjust_dialog::has_dialog(id)) { diff --git a/xtask/src/pinned.rs b/xtask/src/pinned.rs index d480051599..baf798f1de 100644 --- a/xtask/src/pinned.rs +++ b/xtask/src/pinned.rs @@ -71,13 +71,53 @@ impl PinnedCorpus { .collect() } + /// Upper bound on directories visited and files collected during a single `collect` walk. + /// A corpus clone is operator-selected but can still contain attacker-planted symlinks; these + /// budgets keep a malformed tree from expanding into an unbounded traversal before the + /// manifest check runs. Real corpora are a few thousand files at most. + const MAX_DIRS: usize = 100_000; + const MAX_FILES: usize = 1_000_000; + fn collect(&self, dir: &Path, base: &Path, out: &mut Vec) { + let mut visited = std::collections::HashSet::new(); + let mut dirs = 0usize; + self.collect_inner(dir, base, out, &mut visited, &mut dirs); + } + + /// Walks `dir`, skipping directory symlinks and directories already seen through another + /// alias, and stops once the traversal or file budget is reached. `visited` holds resolved + /// directory identities (canonical paths) so two links to the same ancestor cannot make the + /// walk revisit it. + fn collect_inner(&self, dir: &Path, base: &Path, out: &mut Vec, visited: &mut std::collections::HashSet, dirs: &mut usize) { + if out.len() >= Self::MAX_FILES || *dirs >= Self::MAX_DIRS { + return; + } + // Record this directory by its resolved identity; bail if we have already walked it + // (a symlink cycle) or cannot resolve it. + match std::fs::canonicalize(dir) { + Ok(real) => { + if !visited.insert(real) { + return; + } + } + Err(_) => return, + } + *dirs += 1; let Ok(rd) = std::fs::read_dir(dir) else { return }; for e in rd.flatten() { + if out.len() >= Self::MAX_FILES || *dirs >= Self::MAX_DIRS { + return; + } let p = e.path(); - if p.is_dir() { - self.collect(&p, base, out); - } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| self.exts.iter().any(|x| e.eq_ignore_ascii_case(x))) { + // Use the entry's own metadata (do not follow links) to classify it. A directory + // symlink is skipped entirely so aliases to ancestors cannot drive the recursion. + let Ok(ft) = e.file_type() else { continue }; + if ft.is_symlink() { + continue; + } + if ft.is_dir() { + self.collect_inner(&p, base, out, visited, dirs); + } else if ft.is_file() && p.extension().and_then(|e| e.to_str()).is_some_and(|e| self.exts.iter().any(|x| e.eq_ignore_ascii_case(x))) { out.push(p.strip_prefix(base).unwrap_or(&p).to_path_buf()); } } @@ -89,7 +129,11 @@ impl PinnedCorpus { self.collect(dir, dir, &mut found); let mut out = Vec::new(); for f in found { - let rel = f.to_string_lossy().replace('\\', "/"); + // Build the manifest-relative path from the path's own components, joined with `/`. + // This preserves platform filename semantics: a Unix file literally named `a\b` keeps + // its backslash instead of being split into a fake `a/b` traversal component. Reject + // anything that is not a sequence of plain names (absolute roots, `..`, etc.). + let rel = rel_manifest_path(&f).ok_or_else(|| format!("unsafe corpus path {}", f.display()))?; let bytes = std::fs::read(dir.join(&f)).map_err(|e| format!("read {rel}: {e}"))?; out.push((sha256::hex(&bytes), format!("{prefix}{rel}"))); } @@ -144,12 +188,14 @@ impl PinnedCorpus { } }; for (_, file) in files { + let to = safe_dest_join(&dest, file).ok_or_else(|| format!("{file}: escapes corpus destination"))?; let from = locate(file).ok_or_else(|| format!("{file}: no upstream"))?; - place(&from, &dest.join(file))?; + place(&from, &to)?; } - for (from, to) in extras { + for (from, to_rel) in extras { if from.is_file() { - place(from, &dest.join(to))?; + let to = safe_dest_join(&dest, to_rel).ok_or_else(|| format!("{to_rel}: escapes corpus destination"))?; + place(from, &to)?; } } std::fs::write(dest.join("SOURCES.md"), (self.sources_md)(self)).map_err(|e| format!("write SOURCES.md: {e}"))?; @@ -269,6 +315,49 @@ impl PinnedCorpus { } } +/// Builds a `/`-joined manifest-relative path from a scanned relative path, using only the path's +/// own `Normal` components. Returns `None` if any component is a root, prefix, current-dir, or +/// parent-dir (`..`) element, so a scanned entry can never introduce traversal. Literal +/// backslashes inside a single Unix filename are preserved (they are part of the name, not a +/// separator). +fn rel_manifest_path(rel: &Path) -> Option { + use std::path::Component; + let mut parts = Vec::new(); + for comp in rel.components() { + match comp { + Component::Normal(os) => parts.push(os.to_str()?.to_string()), + // Root, prefix, `.` and `..` are all rejected: a corpus entry must be a plain, + // forward relative path beneath the scan root. + _ => return None, + } + } + if parts.is_empty() { + return None; + } + Some(parts.join("/")) +} + +/// Joins a manifest-relative corpus path onto `dest`, guaranteeing the result stays within +/// `dest`. Rejects absolute paths, parent-directory components, and any entry that would resolve +/// outside the destination. Returns the concrete target path on success. +fn safe_dest_join(dest: &Path, rel: &str) -> Option { + use std::path::Component; + let rel_path = Path::new(rel); + let mut out = dest.to_path_buf(); + for comp in rel_path.components() { + match comp { + Component::Normal(os) => out.push(os), + // Any non-plain component (root, prefix, `.`, `..`) could escape the destination. + _ => return None, + } + } + // The join must have added at least one component and must remain under `dest`. + if out == *dest || !out.starts_with(dest) { + return None; + } + Some(out) +} + /// Hex sha256 of a corpus file (reused by every corpus fetch, `xtask/pixls.sha256` included). fn file_hex(path: &std::path::Path) -> Option { std::fs::read(path).ok().map(|b| crate::sha256::hex(&b)) @@ -317,3 +406,75 @@ pub fn fetch_pixls(update: bool) -> Result<(), String> { println!("pixls: {} files verified in {}", crate::corpus_pins::PIXLS_FILES.len(), dest.display()); Ok(()) } + +#[cfg(test)] +mod tests { + use super::{rel_manifest_path, safe_dest_join}; + use std::path::{Path, PathBuf}; + + #[test] + fn rel_manifest_joins_normal_components_with_forward_slash() { + assert_eq!(rel_manifest_path(Path::new("a/b/c.psd")).as_deref(), Some("a/b/c.psd")); + assert_eq!(rel_manifest_path(Path::new("file.psd")).as_deref(), Some("file.psd")); + } + + #[test] + #[cfg(unix)] + fn rel_manifest_preserves_literal_backslash_in_unix_filename() { + // On Unix a backslash is an ordinary filename character, not a separator. The old code + // turned `weird\name.psd` into `weird/name.psd`, inventing a path component that the + // installer would then treat as a subdirectory. The component must be kept intact. + let rel = Path::new("weird\\name.psd"); + assert_eq!(rel_manifest_path(rel).as_deref(), Some("weird\\name.psd")); + } + + #[test] + fn rel_manifest_rejects_traversal_and_absolute() { + assert_eq!(rel_manifest_path(Path::new("../escape.psd")), None); + assert_eq!(rel_manifest_path(Path::new("a/../../escape.psd")), None); + assert_eq!(rel_manifest_path(Path::new("/abs/path.psd")), None); + assert_eq!(rel_manifest_path(Path::new("")), None); + } + + #[test] + fn safe_dest_join_contains_normal_paths() { + let dest = PathBuf::from("/corpus/psd"); + let got = safe_dest_join(&dest, "sub/dir/file.psd").expect("normal path allowed"); + assert!(got.starts_with(&dest)); + assert!(got.ends_with("sub/dir/file.psd")); + } + + #[test] + fn safe_dest_join_rejects_escapes() { + let dest = PathBuf::from("/corpus/psd"); + assert_eq!(safe_dest_join(&dest, "../../etc/passwd"), None); + assert_eq!(safe_dest_join(&dest, "/etc/passwd"), None); + assert_eq!(safe_dest_join(&dest, "a/../../b"), None); + // An empty relative path would resolve to `dest` itself, not a file within it. + assert_eq!(safe_dest_join(&dest, ""), None); + } + + #[test] + #[cfg(unix)] + fn collect_skips_directory_symlinks_and_cycles() { + use std::os::unix::fs::symlink; + + let tmp = std::env::temp_dir().join(format!("pinned-collect-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(tmp.join("real")).unwrap(); + std::fs::write(tmp.join("real/a.psd"), b"data").unwrap(); + // A symlink back to an ancestor: the old collector would follow it and recurse forever. + symlink(&tmp, tmp.join("real/loop")).unwrap(); + // A symlink to a sibling directory also must not be descended into. + symlink(tmp.join("real"), tmp.join("alias")).unwrap(); + + let corpus = + super::PinnedCorpus { name: "t", dest: "t", upstreams: &[], manifest: "t.sha256", exts: &["psd"], subset: false, sources_md: |_| String::new() }; + let mut out = Vec::new(); + corpus.collect(&tmp, &tmp, &mut out); + // Exactly one real file is found; symlinked directories are skipped (no cycle, no dupes). + assert_eq!(out, vec![PathBuf::from("real/a.psd")]); + + let _ = std::fs::remove_dir_all(&tmp); + } +}