Compare commits

..
Author SHA1 Message Date
aiongg 15645fbec4 Apply review fixes
- crates/media/src/probe.rs
- crates/media/src/mp4guard.rs
- crates/media/src/audio.rs
2026-11-04 20:07:37 +00:00
Dylan M. Taylor 1138860d5d address review: run MP4 table guard only for MP4 or unknown containers 2026-11-03 19:44:37 +00:00
Dylan M. Taylor 006d27fd1c media: check an MP4 for oversized sample tables where symphonia will start reading it 2026-11-03 11:12:55 +00:00
8 changed files with 63 additions and 69 deletions
-32
View File
@@ -5023,38 +5023,6 @@ fn pen_pressure_is_kept_and_a_mouse_draws_at_full_width() {
assert_eq!(got, vec![vec![0.2, 1.0, 0.4], vec![1.0, 1.0]]);
}
#[test]
fn a_long_ink_stroke_is_extended_in_place_and_capped() {
let mut s = session();
s.set_tool(tools::ToolKind::Ink { mode: "pen".into(), color: deckcraft_model::Rgba::rgb(20, 20, 20), width: 4.0 });
let ev = |kind, x: f64| tools::PointerEvent { kind, x, y: 80.0, mods: Mods::default(), tol: 3.0, pressure: Some(0.5) };
s.pointer(ev(PointerKind::Down, 0.0)).unwrap();
let n = tools::MAX_GESTURE_POINTS + 5_000;
for i in 1..n {
s.pointer(ev(PointerKind::Drag, (i % 500) as f64)).unwrap();
}
assert_eq!(s.tool.ink_preview.len(), tools::MAX_GESTURE_POINTS);
s.pointer(ev(PointerKind::Up, 0.0)).unwrap();
let kept: usize = ink_pressures(&s).iter().map(Vec::len).sum();
assert!((2..=tools::MAX_GESTURE_POINTS).contains(&kept), "{kept}");
}
#[test]
fn double_clicking_a_capped_freeform_keeps_its_vertices() {
let mut s = session();
let ev = |kind, x: f64, y: f64| tools::PointerEvent { kind, x, y, mods: Default::default(), tol: 3.0, pressure: None };
s.set_tool(tools::ToolKind::Shape { preset: "freeform".into() });
s.pointer(ev(tools::PointerKind::Down, 100.0, 100.0)).unwrap();
// Densely clicked vertices, each within the double-click tolerance of the last.
for i in 0..tools::MAX_GESTURE_POINTS + 10 {
s.pointer(ev(tools::PointerKind::Down, 300.0 + i as f64 * 0.001, 100.0)).unwrap();
}
s.pointer(ev(tools::PointerKind::DoubleClick, 300.0, 100.0)).unwrap();
let st = s.doc().unwrap();
let deckcraft_model::Geom::Custom { paths } = &st.shapes().last().unwrap().geom else { panic!("no freeform") };
assert_eq!(paths[0].d.matches(" L ").count(), tools::MAX_GESTURE_POINTS - 1);
}
fn ink_pressures(s: &Session) -> Vec<Vec<f32>> {
let st = s.doc().unwrap();
let Some(sh) = st.shapes().iter().rev().find(|sh| matches!(sh.kind, deckcraft_model::ShapeKind::Ink { .. })) else {
-3
View File
@@ -17,7 +17,6 @@ mod geometry;
mod hit;
mod pointer;
pub(crate) use drag::MAX_GESTURE_POINTS;
pub use hit::{line_xfrm, resize_box, table_rows, text_pos};
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
@@ -276,8 +275,6 @@ pub struct ToolState {
pub path_preview: Vec<Point>,
pub path_smooth: bool,
pub path_closing: bool,
/// Whether the last freeform press added a vertex (it adds none at the point cap).
pub press_added_vertex: bool,
/// The shape in Edit Points: its vertices are handles instead of the box's.
pub edit_points: Option<ShapeId>,
/// The vertex (path, index) whose control handles show in Edit Points.
+2 -6
View File
@@ -4,12 +4,8 @@ use super::{Drag, Hit, Point, Result, Session, ShapeId, ShapeKind, TextSel, Tool
impl Session {
pub(super) fn double_click(&mut self, p: Point, tol: f64) -> Result<Value> {
if let Some(Drag::Freeform { mode, mut points, .. }) = self.tool.drag.clone() {
// The double-click's own click added a duplicate vertex (none at the point cap, where
// the existing vertices are all kept).
while self.tool.press_added_vertex
&& points.len() > 2
&& points.last().zip(points.get(points.len() - 2)).is_some_and(|(a, b)| (*a - *b).hypot() <= tol)
{
// The double-click's own click added a duplicate vertex.
while points.len() > 2 && points.last().zip(points.get(points.len() - 2)).is_some_and(|(a, b)| (*a - *b).hypot() <= tol) {
points.pop();
}
self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false });
+6 -22
View File
@@ -12,16 +12,9 @@ struct MoveDrag {
enter_text: Option<(ShapeId, Point)>,
}
/// Samples one ink or freeform gesture keeps; later pointer moves extend it no further.
pub(crate) const MAX_GESTURE_POINTS: usize = 20_000;
impl Session {
/// Update the active drag interaction using the pointer position and modifiers.
pub(super) fn drag(&mut self, p: Point, mods: Mods, tol: f64) -> Result<Value> {
if matches!(self.tool.drag, Some(Drag::Ink { .. })) {
self.extend_ink(p);
return Ok(Value::Null);
}
let Some(drag) = self.tool.drag.clone() else { return Ok(Value::Null) };
match drag {
Drag::Guide { index } => {
@@ -188,7 +181,7 @@ impl Session {
}
Drag::Freeform { mode, mut points, pressed } => {
// Dragging scribbles (except for curves, which only take clicked points).
if pressed && mode != "curve" && points.len() < MAX_GESTURE_POINTS && points.last().is_none_or(|l| (*l - p).hypot() >= 1.5) {
if pressed && mode != "curve" && points.last().is_none_or(|l| (*l - p).hypot() >= 1.5) {
points.push(p);
}
let mut v = points.clone();
@@ -198,24 +191,15 @@ impl Session {
self.tool.path_preview = v;
self.tool.drag = Some(Drag::Freeform { mode, points, pressed });
}
// Handled above.
Drag::Ink { .. } => {}
Drag::Ink { mut points } => {
points.push(self.ink_sample(p));
self.tool.ink_preview = points.clone();
self.tool.drag = Some(Drag::Ink { points });
}
}
Ok(Value::Null)
}
/// Ink grows by one sample per move: extend it (and its preview) in place rather than
/// copying the whole stroke twice per event, up to [`MAX_GESTURE_POINTS`].
fn extend_ink(&mut self, p: Point) {
let sample = self.ink_sample(p);
if let Some(Drag::Ink { points }) = self.tool.drag.as_mut()
&& points.len() < MAX_GESTURE_POINTS
{
points.push(sample);
self.tool.ink_preview.push(sample);
}
}
fn drag_move(&mut self, drag: MoveDrag, p: Point, mods: Mods, tol: f64) -> Result<Value> {
let MoveDrag { ids, start, origs, moved, duplicate, enter_text } = drag;
let mut d = p - start;
+1 -4
View File
@@ -228,10 +228,7 @@ impl Session {
self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false });
return self.finish_freeform(true);
}
self.tool.press_added_vertex = points.len() < super::MAX_GESTURE_POINTS;
if self.tool.press_added_vertex {
points.push(p);
}
points.push(p);
self.tool.path_preview = points.clone();
self.tool.drag = Some(Drag::Freeform { mode, points, pressed: true });
return Ok(Value::Null);
+3 -1
View File
@@ -81,7 +81,9 @@ pub fn decode(bytes: &Bytes) -> Result<Pcm> {
Some(r) => r,
None => contained(|| decode_symphonia(bytes, "mkv")),
},
Container::Mp4 if !crate::mp4guard::tables_fit(bytes) => Err(MediaError::Corrupt("the MP4's sample tables do not fit the file".into())),
Container::Mp4 | Container::Unknown | Container::Mp3 if !crate::mp4guard::probe_fits(bytes) => {
Err(MediaError::Corrupt("the MP4's sample tables do not fit the file".into()))
}
c => contained(|| decode_symphonia(bytes, c.name())),
}
}
+49
View File
@@ -12,6 +12,29 @@ pub(crate) fn tables_fit(b: &[u8]) -> bool {
boxes_fit(b, b.len() as u64, 0)
}
/// Bytes symphonia's probe scans for a format marker before giving up.
const PROBE_SEARCH_LIMIT: usize = 1024 * 1024;
/// Whether symphonia can be given `b`, whatever the sniffer made of it: its probe takes the first
/// `ftyp` within its search window as the start of an MP4 (the size field 4 bytes before it), so
/// the table check must start where that parser will, not at offset 0.
pub(crate) fn probe_fits(b: &[u8]) -> bool {
if crate::Container::sniff(b) == crate::Container::Mp4 && !tables_fit(b) {
return false;
}
// The probe reads leading ID3v2 tags as metadata and searches again after the last one.
let mut rest = b;
while rest.starts_with(b"ID3") {
let size = rest.get(6..10).map(|s| s.iter().fold(0usize, |a, &x| (a << 7) | (x & 0x7f) as usize)).unwrap_or(0);
rest = rest.get(10 + size..).unwrap_or(&[]);
}
let head = &rest[..rest.len().min(PROBE_SEARCH_LIMIT + 16)];
match head.windows(4).position(|w| w == b"ftyp") {
Some(p) => tables_fit(&rest[p.saturating_sub(4)..]),
None => true,
}
}
fn be32(b: &[u8], at: usize) -> Option<u64> {
b.get(at..at.checked_add(4)?).map(|s| u32::from_be_bytes([s[0], s[1], s[2], s[3]]) as u64)
}
@@ -74,3 +97,29 @@ fn table_fits(kind: &[u8], body: &[u8], file_len: u64) -> bool {
};
be32(body, 4).is_some_and(|count| count <= body.len().saturating_sub(8) as u64 / entry)
}
#[cfg(test)]
mod tests {
use super::*;
/// An MP4 whose `stsc` count asks for far more entries than the file holds.
fn mp4_with_huge_stsc() -> Vec<u8> {
let stsc = [&24u32.to_be_bytes()[..], b"stsc", &[0, 0, 0, 0], &u32::MAX.to_be_bytes(), &[0; 8]].concat();
let stbl = [&(stsc.len() as u32 + 8).to_be_bytes()[..], b"stbl", &stsc].concat();
let moov = [&(stbl.len() as u32 + 8).to_be_bytes()[..], b"moov", &stbl].concat();
[&16u32.to_be_bytes()[..], b"ftyp", b"isom", &[0; 4], &moov].concat()
}
#[test]
fn a_prefixed_mp4_is_checked_where_symphonia_starts_it() {
let mp4 = mp4_with_huge_stsc();
assert!(!tables_fit(&mp4));
assert!(!probe_fits(&mp4));
let prefixed = [&b"junk before the file "[..], &mp4].concat();
assert_eq!(crate::Container::sniff(&prefixed), crate::Container::Unknown);
assert!(tables_fit(&prefixed), "offset 0 sees no MP4 boxes");
assert!(!probe_fits(&prefixed));
let far = [&vec![0u8; PROBE_SEARCH_LIMIT + 16][..], &mp4].concat();
assert!(probe_fits(&far), "past the probe window symphonia never reads it as an MP4");
}
}
+2 -1
View File
@@ -71,7 +71,8 @@ pub fn probe_headers(bytes: &Bytes) -> Result<MediaInfo> {
};
if info.audio.is_none() && info.video.is_none() || info.audio.as_ref().is_some_and(|a| a.sample_rate == 0) {
// Plain audio files (and anything our demuxers didn't describe): symphonia's probe.
if c == Container::Mp4 && !crate::mp4guard::tables_fit(bytes) {
// Symphonia opens an MP4 wherever its probe finds an `ftyp` (after leading ID3v2 tags too); other known containers (a WAV's audio bytes can spell an `ftyp`) skip the check.
if matches!(c, Container::Mp4 | Container::Unknown | Container::Mp3) && !crate::mp4guard::probe_fits(bytes) {
return Err(MediaError::Corrupt("the MP4's sample tables do not fit the file".into()));
}
let (codec, rate, channels, frames) = std::panic::catch_unwind(|| crate::audio::probe_symphonia(bytes, c.name()))