mp4: Skip unexpected or empty "data" ilst atoms

src/mp4/ilst/mod.rs

@@ -782,7 +782,7 @@ mod tests {
 		let cursor = Cursor::new(tag);
 		let mut reader = AtomReader::new(cursor).unwrap();
 
-		super::read::parse_ilst(&mut reader, len as u64).unwrap()
+		super::read::parse_ilst(&mut reader, crate::ParsingMode::Strict, len as u64).unwrap()
 	}
 
 	fn verify_atom(ilst: &Ilst, ident: [u8; 4], data: &AtomData) {
@@ -850,7 +850,8 @@ mod tests {
 		let cursor = Cursor::new(tag);
 		let mut reader = AtomReader::new(cursor).unwrap();
 
-		let parsed_tag = super::read::parse_ilst(&mut reader, len as u64).unwrap();
+		let parsed_tag =
+			super::read::parse_ilst(&mut reader, crate::ParsingMode::Strict, len as u64).unwrap();
 
 		assert_eq!(expected_tag, parsed_tag);
 	}
@@ -866,8 +867,12 @@ mod tests {
 		let mut reader = AtomReader::new(cursor).unwrap();
 
 		// Remove the ilst identifier and size
-		let temp_parsed_tag =
+		let temp_parsed_tag = super::read::parse_ilst(
-			super::read::parse_ilst(&mut reader, (writer.len() - 8) as u64).unwrap();
+			&mut reader,
+			crate::ParsingMode::Strict,
+			(writer.len() - 8) as u64,
+		)
+		.unwrap();
 
 		assert_eq!(parsed_tag, temp_parsed_tag);
 	}
@@ -880,7 +885,8 @@ mod tests {
 		let cursor = Cursor::new(tag);
 		let mut reader = AtomReader::new(cursor).unwrap();
 
-		let ilst = super::read::parse_ilst(&mut reader, len as u64).unwrap();
+		let ilst =
+			super::read::parse_ilst(&mut reader, crate::ParsingMode::Strict, len as u64).unwrap();
 
 		let tag: Tag = ilst.into();
 
@@ -999,7 +1005,12 @@ mod tests {
 			let cursor = Cursor::new(ilst_bytes);
 			let mut reader = AtomReader::new(cursor).unwrap();
 
-			ilst = super::read::parse_ilst(&mut reader, ilst_bytes.len() as u64).unwrap();
+			ilst = super::read::parse_ilst(
+				&mut reader,
+				crate::ParsingMode::Strict,
+				ilst_bytes.len() as u64,
+			)
+			.unwrap();
 		}
 
 		let mut file = tempfile::tempfile().unwrap();

src/mp4/ilst/read.rs

@@ -10,11 +10,16 @@ use crate::mp4::ilst::atom::AtomDataStorage;
 use crate::mp4::read::{skip_unneeded, AtomReader};
 use crate::picture::{MimeType, Picture, PictureType};
 use crate::util::text::utf16_decode;
+use crate::ParsingMode;
 
 use std::borrow::Cow;
 use std::io::{Cursor, Read, Seek, SeekFrom};
 
-pub(in crate::mp4) fn parse_ilst<R>(reader: &mut AtomReader<R>, len: u64) -> Result<Ilst>
+pub(in crate::mp4) fn parse_ilst<R>(
+	reader: &mut AtomReader<R>,
+	parsing_mode: ParsingMode,
+	len: u64,
+) -> Result<Ilst>
 where
 	R: Read + Seek,
 {
@@ -35,12 +40,14 @@ where
 					continue;
 				},
 				b"covr" => {
-					handle_covr(&mut ilst_reader, &mut tag, &atom)?;
+					handle_covr(&mut ilst_reader, parsing_mode, &mut tag, &atom)?;
 					continue;
 				},
 				// Upgrade this to a \xa9gen atom
 				b"gnre" => {
-					if let Some(atom_data) = parse_data_inner(&mut ilst_reader, &atom)? {
+					if let Some(atom_data) =
+						parse_data_inner(&mut ilst_reader, parsing_mode, &atom)?
+					{
 						let mut data = Vec::new();
 
 						for (_, content) in atom_data {
@@ -70,7 +77,9 @@ where
 				// Special case the "Album ID", as it has the code "BE signed integer" (21), but
 				// must be interpreted as a "BE 64-bit Signed Integer" (74)
 				b"plID" => {
-					if let Some(atom_data) = parse_data_inner(&mut ilst_reader, &atom)? {
+					if let Some(atom_data) =
+						parse_data_inner(&mut ilst_reader, parsing_mode, &atom)?
+					{
 						let mut data = Vec::new();
 
 						for (code, content) in atom_data {
@@ -98,7 +107,9 @@ where
 					continue;
 				},
 				b"cpil" | b"hdvd" | b"pcst" | b"pgap" | b"shwm" => {
-					if let Some(atom_data) = parse_data_inner(&mut ilst_reader, &atom)? {
+					if let Some(atom_data) =
+						parse_data_inner(&mut ilst_reader, parsing_mode, &atom)?
+					{
 						if let Some((_, content)) = atom_data.first() {
 							let data = match content[..] {
 								[0, ..] => AtomData::Bool(false),
@@ -118,17 +129,22 @@ where
 			}
 		}
 
-		parse_data(&mut ilst_reader, &mut tag, atom)?;
+		parse_data(&mut ilst_reader, parsing_mode, &mut tag, atom)?;
 	}
 
 	Ok(tag)
 }
 
-fn parse_data<R>(reader: &mut AtomReader<R>, tag: &mut Ilst, atom_info: AtomInfo) -> Result<()>
+fn parse_data<R>(
+	reader: &mut AtomReader<R>,
+	parsing_mode: ParsingMode,
+	tag: &mut Ilst,
+	atom_info: AtomInfo,
+) -> Result<()>
 where
 	R: Read + Seek,
 {
-	if let Some(mut atom_data) = parse_data_inner(reader, &atom_info)? {
+	if let Some(mut atom_data) = parse_data_inner(reader, parsing_mode, &atom_info)? {
 		// Most atoms we encounter are only going to have 1 value, so store them as such
 		if atom_data.len() == 1 {
 			let (flags, content) = atom_data.remove(0);
@@ -157,8 +173,11 @@ where
 	Ok(())
 }
 
+const DATA_ATOM_IDENT: AtomIdent<'static> = AtomIdent::Fourcc(*b"data");
+
 fn parse_data_inner<R>(
 	reader: &mut AtomReader<R>,
+	parsing_mode: ParsingMode,
 	atom_info: &AtomInfo,
 ) -> Result<Option<Vec<(u32, Vec<u8>)>>>
 where
@@ -170,11 +189,7 @@ where
 	let to_read = (atom_info.start + atom_info.len) - reader.stream_position()?;
 	let mut pos = 0;
 	while pos < to_read {
-		let data_atom = reader.next()?;
+		let next_atom = reader.next()?;
-		match data_atom.ident {
-			AtomIdent::Fourcc(ref name) if name == b"data" => {},
-			_ => err!(BadAtom("Expected atom \"data\" to follow name")),
-		}
 
 		// We don't care about the version
 		let _version = reader.read_u8()?;
@@ -187,17 +202,32 @@ where
 		// We don't care about the locale
 		reader.seek(SeekFrom::Current(4))?;
 
-		let content_len = (data_atom.len - 16) as usize;
+		match next_atom.ident {
-		if content_len == 0 {
+			DATA_ATOM_IDENT => {
-			// We won't add empty atoms
+				debug_assert!(next_atom.len >= 16);
-			return Ok(None);
+				let content_len = (next_atom.len - 16) as usize;
-		}
+				if content_len > 0 {
-
 					let mut content = try_vec![0; content_len];
 					reader.read_exact(&mut content)?;
-
-		pos += data_atom.len;
 					ret.push((flags, content));
+				} else {
+					log::warn!("Skipping empty \"data\" atom");
+				}
+			},
+			_ => match parsing_mode {
+				ParsingMode::Strict => {
+					err!(BadAtom("Expected atom \"data\" to follow name"))
+				},
+				ParsingMode::BestAttempt | ParsingMode::Relaxed => {
+					log::warn!(
+						"Skipping unexpected atom {actual_ident:?}, expected {expected_ident:?}",
+						actual_ident = next_atom.ident,
+						expected_ident = DATA_ATOM_IDENT
+					)
+				},
+			},
+		}
+		pos += next_atom.len;
 	}
 
 	let ret = if ret.is_empty() { None } else { Some(ret) };
@@ -228,11 +258,16 @@ fn parse_int(bytes: &[u8]) -> Result<i32> {
 	})
 }
 
-fn handle_covr<R>(reader: &mut AtomReader<R>, tag: &mut Ilst, atom_info: &AtomInfo) -> Result<()>
+fn handle_covr<R>(
+	reader: &mut AtomReader<R>,
+	parsing_mode: ParsingMode,
+	tag: &mut Ilst,
+	atom_info: &AtomInfo,
+) -> Result<()>
 where
 	R: Read + Seek,
 {
-	if let Some(atom_data) = parse_data_inner(reader, atom_info)? {
+	if let Some(atom_data) = parse_data_inner(reader, parsing_mode, atom_info)? {
 		let mut data = Vec::new();
 
 		let len = atom_data.len();

src/mp4/moov.rs

@@ -4,6 +4,7 @@ use super::ilst::Ilst;
 use super::read::{meta_is_full, nested_atom, skip_unneeded, AtomReader};
 use crate::error::Result;
 use crate::macros::decode_err;
+use crate::ParsingMode;
 
 use std::io::{Read, Seek};
 
@@ -33,7 +34,11 @@ impl Moov {
 		moov.ok_or_else(|| decode_err!(Mp4, "No \"moov\" atom found"))
 	}
 
-	pub(super) fn parse<R>(reader: &mut AtomReader<R>, read_properties: bool) -> Result<Self>
+	pub(super) fn parse<R>(
+		reader: &mut AtomReader<R>,
+		parsing_mode: ParsingMode,
+		read_properties: bool,
+	) -> Result<Self>
 	where
 		R: Read + Seek,
 	{
@@ -51,7 +56,7 @@ impl Moov {
 						}
 					},
 					b"udta" => {
-						meta = meta_from_udta(reader, atom.len - 8)?;
+						meta = meta_from_udta(reader, parsing_mode, atom.len - 8)?;
 					},
 					_ => skip_unneeded(reader, atom.extended, atom.len)?,
 				}
@@ -66,7 +71,11 @@ impl Moov {
 	}
 }
 
-fn meta_from_udta<R>(reader: &mut AtomReader<R>, len: u64) -> Result<Option<Ilst>>
+fn meta_from_udta<R>(
+	reader: &mut AtomReader<R>,
+	parsing_mode: ParsingMode,
+	len: u64,
+) -> Result<Option<Ilst>>
 where
 	R: Read + Seek,
 {
@@ -118,7 +127,7 @@ where
 	}
 
 	if found_ilst {
-		return parse_ilst(reader, ilst_atom_size - 8).map(Some);
+		return parse_ilst(reader, parsing_mode, ilst_atom_size - 8).map(Some);
 	}
 
 	Ok(None)

src/mp4/read.rs

@@ -178,7 +178,11 @@ where
 	let moov_info = Moov::find(&mut reader)?;
 	reader.reset_bounds(moov_info.start + 8, moov_info.len - 8);
 
-	let moov = Moov::parse(&mut reader, parse_options.read_properties)?;
+	let moov = Moov::parse(
+		&mut reader,
+		parse_options.parsing_mode,
+		parse_options.read_properties,
+	)?;
 
 	Ok(Mp4File {
 		ftyp,