From ec47353047b76272da074ce3a372997d03a9852a Mon Sep 17 00:00:00 2001 From: hachispin Date: Sun, 9 Aug 2026 20:00:20 +0100 Subject: [PATCH 01/10] Remove unnecessary collect --- src/formats/ani.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index 4b09a40..3f71f3c 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -401,7 +401,7 @@ impl AniFile { if let Some(seq) = &ani.sequence && hdr.flags == Unsequenced - && seq.data != (0..hdr.num_steps).collect::>() + && seq.data.iter().copied().eq(0..hdr.num_steps) { warn!( "expected 'seq ' chunk to be None from flags={:?}, found the non \ From b53d3b22dfa37665622e5f505741b11b6f6c0582 Mon Sep 17 00:00:00 2001 From: hachispin Date: Sun, 9 Aug 2026 20:30:41 +0100 Subject: [PATCH 02/10] Track duplicate chunks more reliably --- src/formats/ani.rs | 80 +++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index 3f71f3c..6d57162 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -62,12 +62,11 @@ pub struct RiffChunkU8 { /// /// - `0`: no flags are set /// - `2`: frames are not ICO -#[derive(Debug, Default, PartialEq, BinRead)] +#[derive(Debug, PartialEq, BinRead)] #[br(little)] #[br(repr = u32)] enum AniFlags { /// Contains ICO frames that play in the order they're defined (no "seq " chunk). - #[default] Unsequenced = 1, /// Contains ICO frames with a custom "seq " chunk, /// which defines the order frames should be played. @@ -78,7 +77,7 @@ enum AniFlags { /// Models an ANI file's header (or the "anih" chunk). #[binread] -#[derive(Debug, Default, PartialEq)] +#[derive(Debug, PartialEq)] #[br(little)] pub struct AniHeader { #[br(temp)] @@ -111,7 +110,6 @@ pub struct AniHeader { } /// Models a parsed ANI file. -#[derive(Default)] pub struct AniFile { /// The header, i.e, the "anih" chunk. pub header: AniHeader, @@ -159,6 +157,40 @@ impl fmt::Debug for AniFile { } } +/// For tracking duplicates. +#[derive(Default)] +struct AniParserState { + pub header: Option, + pub title: Option, + pub author: Option, + pub rate: Option, + pub sequence: Option, + pub ico_frames: Option>, +} + +impl TryFrom for AniFile { + type Error = anyhow::Error; + + fn try_from(state: AniParserState) -> Result { + let Some(header) = state.header else { + bail!("AniHeader is required") + }; + + let Some(ico_frames) = state.ico_frames else { + bail!("ico_frames is required") + }; + + Ok(Self { + header, + title: state.title, + author: state.author, + rate: state.rate, + sequence: state.sequence, + ico_frames, + }) + } +} + impl AniFile { /// Max blob size for any (dynamic length) chunk. const MAX_CHUNK_SIZE: usize = 2_097_152; @@ -178,16 +210,9 @@ impl AniFile { /// /// - [gdgsoft](https://www.gdgsoft.com/anituner/help/aniformat.htm) pub fn from_blob(ani_blob: &[u8]) -> Result { - if ani_blob.len() > Self::MAX_CHUNK_SIZE { - bail!( - "ani_blob.len()={} unreasonably large (2MB+)", - ani_blob.len() - ) - } - // for sanity checks against read sizes let ani_blob_len_u64 = u64::try_from(ani_blob.len())?; - let mut ani = Self::default(); + let mut state = AniParserState::default(); let mut cursor = Cursor::new(ani_blob); let mut buf = [0_u8; 4]; cursor.read_exact(&mut buf)?; @@ -217,33 +242,34 @@ impl AniFile { cursor.read_exact(&mut buf)?; match &buf { - b"LIST" => Self::parse_list(&mut cursor, &mut ani)?, + b"LIST" => Self::parse_list(&mut cursor, &mut state)?, b"anih" => { - if ani.header != AniHeader::default() { + if state.header.is_some() { bail!("duplicate 'anih' chunk"); } - ani.header = - AniHeader::read_le(&mut cursor).context("failed to read 'anih' chunk")?; + state.header = Some( + AniHeader::read_le(&mut cursor).context("failed to read 'anih' chunk")?, + ); } b"rate" => { - if ani.rate.is_some() { + if state.rate.is_some() { bail!("duplicate 'rate' chunk"); } - ani.rate = Some( + state.rate = Some( RiffChunkU32::read_le(&mut cursor) .context("failed to read 'rate' chunk")?, ); } b"seq " => { - if ani.sequence.is_some() { + if state.sequence.is_some() { bail!("duplicate 'seq ' chunk"); } - ani.sequence = Some( + state.sequence = Some( RiffChunkU32::read_le(&mut cursor) .context("failed to read 'seq ' chunk")?, ); @@ -255,6 +281,8 @@ impl AniFile { } } + let ani = state.try_into()?; + Self::check_invariants(&ani)?; Ok(ani) @@ -266,7 +294,7 @@ impl AniFile { /// either be "INFO" (title/author) or "fram" (frame data). /// /// The "INFO" chunk isn't required. The "fram" chunk is. - fn parse_list(cursor: &mut Cursor<&[u8]>, ani: &mut Self) -> Result<()> { + fn parse_list(cursor: &mut Cursor<&[u8]>, state: &mut AniParserState) -> Result<()> { let ani_blob_size = cursor.get_ref().len(); let mut buf = [0_u8; 4]; let mut list_id = [0_u8; 4]; @@ -303,9 +331,9 @@ impl AniFile { cursor.read_exact(&mut buf)?; let field = if buf == *b"INAM" { - &mut ani.title + &mut state.title } else if buf == *b"IART" { - &mut ani.author + &mut state.author } else { bail!("expected 'INAM' or 'IART' subchunk in 'INFO', instead got {buf:?}"); }; @@ -321,11 +349,11 @@ impl AniFile { } b"fram" => { - if !ani.ico_frames.is_empty() { + if state.ico_frames.is_some() { bail!("duplicate 'fram' chunk"); } - let mut chunks = Vec::with_capacity(usize::try_from(ani.header.num_frames)?); + let mut chunks = Vec::new(); while cursor.position() < end { cursor.read_exact(&mut buf)?; @@ -344,7 +372,7 @@ impl AniFile { bail!("failed to parse any frames from 'fram' chunk"); } - ani.ico_frames = chunks; + state.ico_frames = Some(chunks); } _ => bail!("unexpected list_id={list_id:?}"), From a8a3386e10f9f035314580370f2fd1405c913127 Mon Sep 17 00:00:00 2001 From: hachispin Date: Sun, 9 Aug 2026 20:31:01 +0100 Subject: [PATCH 03/10] Fix condition on sequence linearity check --- src/formats/ani.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index 6d57162..bd2c839 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -429,7 +429,7 @@ impl AniFile { if let Some(seq) = &ani.sequence && hdr.flags == Unsequenced - && seq.data.iter().copied().eq(0..hdr.num_steps) + && !seq.data.iter().copied().eq(0..hdr.num_steps) { warn!( "expected 'seq ' chunk to be None from flags={:?}, found the non \ From 8508912b42ef4bd0c76ef3ab9338b3e7525bf0aa Mon Sep 17 00:00:00 2001 From: hachispin Date: Sun, 9 Aug 2026 20:35:15 +0100 Subject: [PATCH 04/10] Use RIFF terminology --- src/formats/ani.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index bd2c839..f00efd3 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -173,11 +173,11 @@ impl TryFrom for AniFile { fn try_from(state: AniParserState) -> Result { let Some(header) = state.header else { - bail!("AniHeader is required") + bail!("required 'anih' chunk is missing") }; let Some(ico_frames) = state.ico_frames else { - bail!("ico_frames is required") + bail!("required 'fram' chunk is missing") }; Ok(Self { From 2eae73e16c42916f29f88959c85d66e32b7b0098 Mon Sep 17 00:00:00 2001 From: hachispin Date: Mon, 10 Aug 2026 23:39:04 +0100 Subject: [PATCH 05/10] Refactor parsing of RIFF chunks Should be easier to use. --- src/cursors/generic_cursor.rs | 6 +- src/formats/ani.rs | 332 +++++++++++++++++----------------- 2 files changed, 168 insertions(+), 170 deletions(-) diff --git a/src/cursors/generic_cursor.rs b/src/cursors/generic_cursor.rs index 7682c90..3a35083 100644 --- a/src/cursors/generic_cursor.rs +++ b/src/cursors/generic_cursor.rs @@ -222,13 +222,13 @@ impl GenericCursor { let icos: Vec = ani_file .ico_frames .into_iter() - .map(|chunk| IconDir::read(&mut Cursor::new(&chunk.data))) + .map(|chunk| IconDir::read(&mut Cursor::new(&chunk))) .collect::>()?; // get display order as indices into icos let sequence: Option> = ani_file .sequence - .map(|chunk| chunk.data.into_iter().map(usize::try_from).collect()) + .map(|chunk| chunk.into_iter().map(usize::try_from).collect()) .transpose()?; // indices validated in-bounds in AniFile @@ -241,7 +241,7 @@ impl GenericCursor { let num_steps = usize::try_from(header.num_steps)?; let delays_jiffies = ani_file .rate - .map_or_else(|| vec![header.jiffy_rate; num_steps], |chunk| chunk.data); + .unwrap_or_else(|| vec![header.jiffy_rate; num_steps]); // jiffies are 1/60th of a second // diff --git a/src/formats/ani.rs b/src/formats/ani.rs index f00efd3..e4fbec4 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -7,43 +7,37 @@ use std::{ fmt, - io::{Cursor, Read}, + io::{Cursor, Read, Seek, SeekFrom}, + range::Range, }; -use anyhow::{Context, Result, bail}; -use binrw::{BinRead, NullString, binread}; +use anyhow::{Result, anyhow, bail}; +use binrw::{BinRead, binread}; use crate::warn; -/// RIFF chunk with [`Self::data`] as [`Vec`]. +/// Generic RIFF chunk structure. #[binread] #[derive(Debug)] #[br(little)] -pub struct RiffChunkU32 { - // temp because `data` stores its own length - #[br(temp)] - data_size: u32, - - #[br(try_calc = usize::try_from(data_size / 4), temp)] - data_length: usize, - - #[br(count = data_length)] - pub data: Vec, - // no padding needed, data is inherently even (u32) -} - -/// RIFF chunk with [`Self::data`] as [`Vec`]. -#[binread] -#[derive(Debug)] -#[br(little)] -pub struct RiffChunkU8 { - // size == length here since `data` is Vec - #[br(temp)] - data_size: u32, - - #[br(count = data_size, pad_after = data_size % 2)] - pub data: Vec, - // padding byte skipped with `pad_after` +#[br(stream = s)] +pub struct RiffChunk { + id: [u8; 4], + size: u32, + + // Just for calculations. + #[br(temp, try_calc = s.stream_position())] + start: u64, + #[br(temp, try_calc = start.checked_add(u64::from(size)).ok_or("overflow"))] + end: u64, + + /// Range of data, excludes padding. + #[br(calc = (start..end).into())] + data: Range, + + /// Where the next chunk should start and another [`RiffChunk`] can be read. + #[br(try_calc = end.checked_add(u64::from(size) & 1).ok_or("overflow"))] + next: u64, } // NOTE: this is storing the valid combinations of bitflags and are not meant to be composable. @@ -80,9 +74,7 @@ enum AniFlags { #[derive(Debug, PartialEq)] #[br(little)] pub struct AniHeader { - #[br(temp)] - anih_size: u32, - #[br(assert(anih_size == header_size && header_size == 36), temp)] + #[br(assert(header_size == 36), temp)] header_size: u32, /// Number of frames in "fram" LIST. Not to be confused with [`Self::num_steps`]: /// @@ -115,10 +107,10 @@ pub struct AniFile { pub header: AniHeader, /// The title stored in the "INFO" ("LIST" subtype) chunk, with /// the identifier: "INAM". Note that this is rarely present. - pub title: Option, + pub title: Option, /// The author stored in the "INFO" ("LIST" subtype) chunk, with /// the identifier: "IART". Note that this is rarely present. - pub author: Option, + pub author: Option, /// Per-frame timings. Usually [`None`]. /// /// rate: `[t_0, t_1, t_2, ...]`\ @@ -128,20 +120,20 @@ pub struct AniFile { /// /// The rate is applied **after sequencing**, so `frames` is /// better said as the "display order", see [`Self::sequence`]. - pub rate: Option, + pub rate: Option>, /// Stores frame indices to indicate the order in which /// frames are played. Frames can also be repeated. /// /// frames: `[f_0, f_1, f_2, f_3, ...]`\ /// sequence: `[2, 3, 0, 0, 1, ...]`\ /// display order: `[f_2, f_3, f_0, f_0, f_1, ...]` - pub sequence: Option, + pub sequence: Option>, /// ICO frames. Each frame should have a hotspot. /// /// Each ICO frame can contain multiple images, usually for supporting different sizes. /// /// _Although redundant, since Windows scales cursors already._ - pub ico_frames: Vec, + pub ico_frames: Vec>, } // skip ico_frames @@ -157,44 +149,102 @@ impl fmt::Debug for AniFile { } } -/// For tracking duplicates. #[derive(Default)] struct AniParserState { - pub header: Option, - pub title: Option, - pub author: Option, - pub rate: Option, - pub sequence: Option, - pub ico_frames: Option>, + pub header: Option>, + pub title: Option>, + pub author: Option>, + pub rate: Option>, + pub sequence: Option>, + pub ico_frames: Option>, } -impl TryFrom for AniFile { - type Error = anyhow::Error; +fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { + // helpers - fn try_from(state: AniParserState) -> Result { - let Some(header) = state.header else { - bail!("required 'anih' chunk is missing") - }; + let to_usize_range = |range: Range| -> Result<_> { + let start = usize::try_from(range.start)?; + let end = usize::try_from(range.end)?; + + Ok(start..end) + }; - let Some(ico_frames) = state.ico_frames else { - bail!("required 'fram' chunk is missing") + let bytes_to_string = |r: Range| { + let string = &blob[to_usize_range(r)?]; + + let string = if let Some(s) = string.strip_suffix(b"\0") { + s + } else { + warn!("INFO string not null-terminated"); + string }; - Ok(Self { - header, - title: state.title, - author: state.author, - rate: state.rate, - sequence: state.sequence, - ico_frames, - }) + str::from_utf8(string) + .map(ToString::to_string) + .map_err(Into::::into) + }; + + let to_u32_vec = |r: Range| { + let bytes = &blob[to_usize_range(r)?]; + + let (bytes, rem) = bytes.as_chunks::<4>(); + + if !rem.is_empty() { + bail!("u32 data not divisible by 4") + } + + anyhow::Ok( + bytes + .iter() + .map(|&b| u32::from_le_bytes(b)) + .collect::>(), + ) + }; + + // required stuff + + let Some(header) = state.header else { + bail!("'anih' chunk is required but is missing") + }; + + let Some(ico_frames) = state.ico_frames else { + bail!("'fram' chunk is required but is missing") + }; + + let header = AniHeader::read(&mut Cursor::new(&blob[to_usize_range(header)?]))?; + let fram = &blob[to_usize_range(ico_frames)?]; + let mut cursor = Cursor::new(fram); + let mut ico_frames = Vec::with_capacity(usize::try_from(header.num_frames)?); + + while cursor.position() < u64::try_from(fram.len())? { + let icon = RiffChunk::read(&mut cursor)?; + debug_assert_eq!(icon.id, *b"icon"); + + let mut bytes = vec![0; usize::try_from(icon.size)?]; + cursor.read_exact(&mut bytes)?; + ico_frames.push(bytes); + + cursor.seek(SeekFrom::Start(icon.next))?; } + + // optional things + + let title = state.title.map(bytes_to_string).transpose()?; + let author = state.author.map(bytes_to_string).transpose()?; + let rate = state.rate.map(to_u32_vec).transpose()?; + let sequence = state.sequence.map(to_u32_vec).transpose()?; + + Ok(AniFile { + header, + title, + author, + rate, + sequence, + ico_frames, + }) } impl AniFile { - /// Max blob size for any (dynamic length) chunk. - const MAX_CHUNK_SIZE: usize = 2_097_152; - /// Parses `ani_blob`. /// /// ## Errors @@ -210,47 +260,42 @@ impl AniFile { /// /// - [gdgsoft](https://www.gdgsoft.com/anituner/help/aniformat.htm) pub fn from_blob(ani_blob: &[u8]) -> Result { - // for sanity checks against read sizes let ani_blob_len_u64 = u64::try_from(ani_blob.len())?; let mut state = AniParserState::default(); let mut cursor = Cursor::new(ani_blob); - let mut buf = [0_u8; 4]; - cursor.read_exact(&mut buf)?; - if buf != *b"RIFF" { - bail!("expected 'RIFF' chunk, instead got {buf:?}"); - } + let riff = RiffChunk::read(&mut cursor)?; - cursor.read_exact(&mut buf)?; - let riff_size = u32::from_le_bytes(buf); + if riff.id != *b"RIFF" { + bail!("expected 'RIFF' chunk, instead got {:?}", riff.id); + } // NOTE: stricter checks like this fail on "valid" files // `riff_size == blob.len() - 8` // https://github.com/quantum5/win2xcur/commit/ac9552ce83d2955a96a4d7a5cfde7c113ec5a4c5 - if u64::from(riff_size) > ani_blob_len_u64 { - bail!("riff_size={riff_size} extends beyond blob") + if u64::from(riff.size) > ani_blob_len_u64 { + bail!("riff_size={} extends beyond blob", riff.size) } - cursor.read_exact(&mut buf)?; + let mut list_type = [0_u8; 4]; + cursor.read_exact(&mut list_type)?; - if buf != *b"ACON" { - bail!("expected 'ACON' as 'RIFF' subtype, instead got {buf:?}"); + if list_type != *b"ACON" { + bail!("expected 'ACON' as 'RIFF' subtype, instead got {list_type:?}"); } // read chunks and parse while cursor.position() < ani_blob.len().try_into()? { - cursor.read_exact(&mut buf)?; + let chunk = RiffChunk::read(&mut cursor)?; - match &buf { - b"LIST" => Self::parse_list(&mut cursor, &mut state)?, + match dbg!(&chunk.id) { + b"LIST" => Self::parse_list(&mut cursor, &mut state, &chunk)?, b"anih" => { if state.header.is_some() { bail!("duplicate 'anih' chunk"); } - state.header = Some( - AniHeader::read_le(&mut cursor).context("failed to read 'anih' chunk")?, - ); + state.header = Some(chunk.data); } b"rate" => { @@ -258,10 +303,7 @@ impl AniFile { bail!("duplicate 'rate' chunk"); } - state.rate = Some( - RiffChunkU32::read_le(&mut cursor) - .context("failed to read 'rate' chunk")?, - ); + state.rate = Some(chunk.data); } b"seq " => { @@ -269,20 +311,16 @@ impl AniFile { bail!("duplicate 'seq ' chunk"); } - state.sequence = Some( - RiffChunkU32::read_le(&mut cursor) - .context("failed to read 'seq ' chunk")?, - ); + state.sequence = Some(chunk.data); } - // consider attempting to read size and skipping - // for unknown chunks (but it's a bit unreliable) - _ => bail!("unexpected fourcc(?) buf={buf:?}"), + _ => (), } - } - let ani = state.try_into()?; + cursor.seek(SeekFrom::Start(chunk.next))?; + } + let ani = process_ranges(ani_blob, &state)?; Self::check_invariants(&ani)?; Ok(ani) @@ -294,57 +332,32 @@ impl AniFile { /// either be "INFO" (title/author) or "fram" (frame data). /// /// The "INFO" chunk isn't required. The "fram" chunk is. - fn parse_list(cursor: &mut Cursor<&[u8]>, state: &mut AniParserState) -> Result<()> { - let ani_blob_size = cursor.get_ref().len(); - let mut buf = [0_u8; 4]; - let mut list_id = [0_u8; 4]; - cursor.read_exact(&mut buf)?; // list size - cursor.read_exact(&mut list_id)?; - let list_size = u32::from_le_bytes(buf); - - // excluding subtype fourcc (and padding) - let list_data_size = list_size - .checked_sub(4) - .with_context(|| format!("underflow on list_size={list_size} - 4"))?; - - if usize::try_from(list_data_size)? > Self::MAX_CHUNK_SIZE { - bail!("list_data_size={list_data_size} unreasonably large (2MB+)"); - } - + fn parse_list( + cursor: &mut Cursor<&[u8]>, + state: &mut AniParserState, + list_chunk: &RiffChunk, + ) -> Result<()> { let end = cursor .position() - .checked_add(u64::from(list_data_size)) - .with_context(|| { - format!( - "overflow on cursor.position={} + list_data_size={list_data_size}", - cursor.position() - ) - })?; - - if end > ani_blob_size.try_into()? { - bail!("list_data_size={list_data_size} extends beyond blob"); - } + .checked_add(u64::from(list_chunk.size)) + .ok_or_else(|| anyhow!("overflow"))?; - match &list_id { + let mut list_type = [0_u8; 4]; + cursor.read_exact(&mut list_type)?; + + match &list_type { b"INFO" => { while cursor.position() < end { - cursor.read_exact(&mut buf)?; - - let field = if buf == *b"INAM" { - &mut state.title - } else if buf == *b"IART" { - &mut state.author - } else { - bail!("expected 'INAM' or 'IART' subchunk in 'INFO', instead got {buf:?}"); - }; - - if field.is_some() { - bail!("duplicate 'INAM' or 'IART' subchunk in 'INFO'"); + let subchunk = RiffChunk::read(cursor)?; + + // just let it be overwritten lool + if subchunk.id == *b"INAM" { + state.title = Some(subchunk.data); + } else if subchunk.id == *b"IART" { + state.author = Some(subchunk.data); } - // size of string - cursor.read_exact(&mut buf)?; - *field = Some(NullString::read_le(cursor)?); + cursor.seek(SeekFrom::Start(subchunk.next))?; } } @@ -353,29 +366,14 @@ impl AniFile { bail!("duplicate 'fram' chunk"); } - let mut chunks = Vec::new(); - - while cursor.position() < end { - cursor.read_exact(&mut buf)?; - - if buf != *b"icon" { - bail!("expected 'icon' subchunk, instead got {buf:?}"); - } - - let chunk = RiffChunkU8::read_le(cursor) - .context("failed to read 'icon' subchunk of 'fram'")?; - - chunks.push(chunk); - } - - if chunks.is_empty() { - bail!("failed to parse any frames from 'fram' chunk"); - } - - state.ico_frames = Some(chunks); + // exclude list type (fram) + state.ico_frames = Some((cursor.position()..end).into()); } - _ => bail!("unexpected list_id={list_id:?}"), + // skip + _ => { + cursor.seek(SeekFrom::Start(list_chunk.next))?; + } } Ok(()) @@ -401,11 +399,11 @@ impl AniFile { } if let Some(rate) = &ani.rate - && rate.data.len() != num_steps + && rate.len() != num_steps { bail!( "expected num_steps={num_steps}, instead got rate.len()={}", - rate.data.len(), + rate.len(), ) } @@ -414,7 +412,7 @@ impl AniFile { } if let Some(seq) = &ani.sequence - && seq.data.iter().max() >= Some(&hdr.num_frames) + && seq.iter().max() >= Some(&hdr.num_frames) { bail!("frame indices of 'seq ' chunk go out of bounds"); } @@ -429,7 +427,7 @@ impl AniFile { if let Some(seq) = &ani.sequence && hdr.flags == Unsequenced - && !seq.data.iter().copied().eq(0..hdr.num_steps) + && !seq.iter().copied().eq(0..hdr.num_steps) { warn!( "expected 'seq ' chunk to be None from flags={:?}, found the non \ @@ -474,7 +472,7 @@ mod tests { assert!(ani.rate.is_none()); assert_eq!( - ani.sequence.as_ref().unwrap().data, + ani.sequence.as_ref().unwrap(), &[ 0, 1, 2, 2, 3, 3, 3, 3, 4, 5, 6, 7, 3, 3, 3, 2, 2, 2, 3, 8, 9 ] @@ -487,13 +485,13 @@ mod tests { assert_eq!( usize::try_from(hdr.num_steps).unwrap(), - ani.sequence.as_ref().unwrap().data.len() + ani.sequence.as_ref().unwrap().len() ); let mut ani_frames = String::new(); for frame in ani.ico_frames { - writeln!(&mut ani_frames, "{:?}", frame.data).unwrap(); + writeln!(&mut ani_frames, "{frame:?}").unwrap(); } assert_eq!(ani_frames, ANI_FRAMES); From 1cf68d37267ca862ab6ac5a70f7de129dc01f900 Mon Sep 17 00:00:00 2001 From: hachispin Date: Mon, 10 Aug 2026 23:46:13 +0100 Subject: [PATCH 06/10] Remove unneeded pub and seek call --- src/formats/ani.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index e4fbec4..67f835b 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -151,12 +151,12 @@ impl fmt::Debug for AniFile { #[derive(Default)] struct AniParserState { - pub header: Option>, - pub title: Option>, - pub author: Option>, - pub rate: Option>, - pub sequence: Option>, - pub ico_frames: Option>, + header: Option>, + title: Option>, + author: Option>, + rate: Option>, + sequence: Option>, + ico_frames: Option>, } fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { @@ -370,10 +370,7 @@ impl AniFile { state.ico_frames = Some((cursor.position()..end).into()); } - // skip - _ => { - cursor.seek(SeekFrom::Start(list_chunk.next))?; - } + _ => (), } Ok(()) From 2e6dbe8687388a01314df809f04aa72db321bb65 Mon Sep 17 00:00:00 2001 From: hachispin Date: Tue, 11 Aug 2026 01:11:50 +0100 Subject: [PATCH 07/10] Improve error messages; Polish a bit --- src/formats/ani.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index 67f835b..c86f5b5 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -28,7 +28,9 @@ pub struct RiffChunk { // Just for calculations. #[br(temp, try_calc = s.stream_position())] start: u64, - #[br(temp, try_calc = start.checked_add(u64::from(size)).ok_or("overflow"))] + #[br(temp, try_calc = start.checked_add(u64::from(size)).ok_or_else(|| { + anyhow!("overflow when calculating RiffChunk end for id={id:?}") + }))] end: u64, /// Range of data, excludes padding. @@ -36,7 +38,9 @@ pub struct RiffChunk { data: Range, /// Where the next chunk should start and another [`RiffChunk`] can be read. - #[br(try_calc = end.checked_add(u64::from(size) & 1).ok_or("overflow"))] + #[br(try_calc = end.checked_add(u64::from(size) & 1).ok_or_else(|| { + anyhow!("overflow when calculating RiffChunk next for id={id:?}") + }))] next: u64, } @@ -175,7 +179,7 @@ fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { let string = if let Some(s) = string.strip_suffix(b"\0") { s } else { - warn!("INFO string not null-terminated"); + warn!("'INFO' string is not null-terminated"); string }; @@ -218,7 +222,10 @@ fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { while cursor.position() < u64::try_from(fram.len())? { let icon = RiffChunk::read(&mut cursor)?; - debug_assert_eq!(icon.id, *b"icon"); + + if icon.id != *b"icon" { + bail!("expected 'icon' subchunks, instead got {:?}", icon.id); + } let mut bytes = vec![0; usize::try_from(icon.size)?]; cursor.read_exact(&mut bytes)?; @@ -288,11 +295,11 @@ impl AniFile { while cursor.position() < ani_blob.len().try_into()? { let chunk = RiffChunk::read(&mut cursor)?; - match dbg!(&chunk.id) { + match &chunk.id { b"LIST" => Self::parse_list(&mut cursor, &mut state, &chunk)?, b"anih" => { if state.header.is_some() { - bail!("duplicate 'anih' chunk"); + bail!("read duplicate 'anih' chunk at {}", cursor.position()); } state.header = Some(chunk.data); @@ -300,7 +307,7 @@ impl AniFile { b"rate" => { if state.rate.is_some() { - bail!("duplicate 'rate' chunk"); + bail!("read duplicate 'rate' chunk at {}", cursor.position()); } state.rate = Some(chunk.data); @@ -308,7 +315,7 @@ impl AniFile { b"seq " => { if state.sequence.is_some() { - bail!("duplicate 'seq ' chunk"); + bail!("read duplicate 'seq ' chunk at {}", cursor.position()); } state.sequence = Some(chunk.data); @@ -340,7 +347,7 @@ impl AniFile { let end = cursor .position() .checked_add(u64::from(list_chunk.size)) - .ok_or_else(|| anyhow!("overflow"))?; + .ok_or_else(|| anyhow!("overflow when calculating end of list chunk"))?; let mut list_type = [0_u8; 4]; cursor.read_exact(&mut list_type)?; @@ -350,7 +357,7 @@ impl AniFile { while cursor.position() < end { let subchunk = RiffChunk::read(cursor)?; - // just let it be overwritten lool + // Let INFO be overridden as it's non-essential. if subchunk.id == *b"INAM" { state.title = Some(subchunk.data); } else if subchunk.id == *b"IART" { @@ -363,7 +370,7 @@ impl AniFile { b"fram" => { if state.ico_frames.is_some() { - bail!("duplicate 'fram' chunk"); + bail!("read duplicate 'fram' chunk at {}", cursor.position()); } // exclude list type (fram) @@ -451,13 +458,6 @@ mod tests { const ANI_FRAMES: &str = include_str!(from_root!("/testing/fixtures/neuro_alt_frames")); const ANI_BLOB: &[u8] = include_bytes!(from_root!("/testing/fixtures/neuro/Neuro alt.ani")); - const { - assert!( - size_of::() == 136, - "AniFile fields have changed, update tests and this number accordingly" - ); - } - let ani = AniFile::from_blob(ANI_BLOB).unwrap(); let hdr = &ani.header; From 56f09cecbcd6566fcb8ad76a526a2636005e251b Mon Sep 17 00:00:00 2001 From: hachispin Date: Tue, 11 Aug 2026 02:06:53 +0100 Subject: [PATCH 08/10] Add extra safety checks; Don't allocate arbitrary capacities --- src/formats/ani.rs | 50 +++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index c86f5b5..80dc614 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -163,18 +163,19 @@ struct AniParserState { ico_frames: Option>, } -fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { - // helpers +fn slice_blob(blob: &[u8], range: Range) -> Result<&[u8]> { + let start = usize::try_from(range.start)?; + let end = usize::try_from(range.end)?; - let to_usize_range = |range: Range| -> Result<_> { - let start = usize::try_from(range.start)?; - let end = usize::try_from(range.end)?; + blob.get(start..end) + .ok_or_else(|| anyhow!("range {start}..{end} outside of blob (len={})", blob.len())) +} - Ok(start..end) - }; +fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { + // helpers let bytes_to_string = |r: Range| { - let string = &blob[to_usize_range(r)?]; + let string = slice_blob(blob, r)?; let string = if let Some(s) = string.strip_suffix(b"\0") { s @@ -183,13 +184,11 @@ fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { string }; - str::from_utf8(string) - .map(ToString::to_string) - .map_err(Into::::into) + anyhow::Ok(String::from_utf8_lossy(string).to_string()) }; let to_u32_vec = |r: Range| { - let bytes = &blob[to_usize_range(r)?]; + let bytes = slice_blob(blob, r)?; let (bytes, rem) = bytes.as_chunks::<4>(); @@ -215,10 +214,12 @@ fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { bail!("'fram' chunk is required but is missing") }; - let header = AniHeader::read(&mut Cursor::new(&blob[to_usize_range(header)?]))?; - let fram = &blob[to_usize_range(ico_frames)?]; + let header = AniHeader::read(&mut Cursor::new(slice_blob(blob, header)?))?; + let fram = slice_blob(blob, ico_frames)?; let mut cursor = Cursor::new(fram); - let mut ico_frames = Vec::with_capacity(usize::try_from(header.num_frames)?); + + // don't reserve the non-validated num_frames + let mut ico_frames = Vec::new(); while cursor.position() < u64::try_from(fram.len())? { let icon = RiffChunk::read(&mut cursor)?; @@ -227,9 +228,8 @@ fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { bail!("expected 'icon' subchunks, instead got {:?}", icon.id); } - let mut bytes = vec![0; usize::try_from(icon.size)?]; - cursor.read_exact(&mut bytes)?; - ico_frames.push(bytes); + let bytes = slice_blob(fram, icon.data)?; + ico_frames.push(bytes.to_vec()); cursor.seek(SeekFrom::Start(icon.next))?; } @@ -302,6 +302,13 @@ impl AniFile { bail!("read duplicate 'anih' chunk at {}", cursor.position()); } + if chunk.size != 36 { + bail!( + "expected 'anih' chunk size to be 36, instead got {}", + chunk.size + ) + } + state.header = Some(chunk.data); } @@ -344,6 +351,13 @@ impl AniFile { state: &mut AniParserState, list_chunk: &RiffChunk, ) -> Result<()> { + if list_chunk.size < 4 { + bail!( + "expected 'LIST' chunk to have size four or greater, instead got {}", + list_chunk.size + ); + } + let end = cursor .position() .checked_add(u64::from(list_chunk.size)) From f10c1abf9767b3a6fe934e8d63c321f8e8284294 Mon Sep 17 00:00:00 2001 From: hachispin Date: Tue, 11 Aug 2026 02:13:20 +0100 Subject: [PATCH 09/10] Read ZSTR properly --- src/formats/ani.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index 80dc614..3986a8f 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -177,12 +177,10 @@ fn process_ranges(blob: &[u8], state: &AniParserState) -> Result { let bytes_to_string = |r: Range| { let string = slice_blob(blob, r)?; - let string = if let Some(s) = string.strip_suffix(b"\0") { - s - } else { + let string = &string[..(string.iter().position(|&b| b == 0).unwrap_or_else(|| { warn!("'INFO' string is not null-terminated"); - string - }; + string.len() + }))]; anyhow::Ok(String::from_utf8_lossy(string).to_string()) }; From e985e7f3bb9ea552ea9b836d0f9b4f75d6ee3426 Mon Sep 17 00:00:00 2001 From: hachispin Date: Tue, 11 Aug 2026 02:32:48 +0100 Subject: [PATCH 10/10] Add check validating `num_steps` --- src/formats/ani.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/formats/ani.rs b/src/formats/ani.rs index 3986a8f..bd69121 100644 --- a/src/formats/ani.rs +++ b/src/formats/ani.rs @@ -433,6 +433,14 @@ impl AniFile { bail!("frame indices of 'seq ' chunk go out of bounds"); } + let observed_steps = ani.sequence.as_ref().map_or(num_frames, Vec::len); + if observed_steps != num_steps { + bail!( + "num_steps={num_steps} does not match length of \ + sequence table (observed_steps={observed_steps})" + ); + } + if hdr.flags == Sequenced && ani.sequence.is_none() { warn!( "expected 'seq ' chunk from flags={:?}, found None. the \