probe: Add new default ParsingMode variant

CHANGELOG.md

@@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+- **ParsingMode**: A new variant, `BestAttempt` will attempt to fill holes in otherwise valid tag items ([PR](https://github.com/Serial-ATA/lofty-rs/pull/205))
+
+### Changed
+- **Probe**: The default `ParsingMode` is now `ParsingMode::BestAttempt` (It was previously `ParsingMode::Strict`)
+
 ### Fixed
 - **MP4**: Fixed potential panic with malformed `plID` atoms ([issue](https://github.com/Serial-ATA/lofty-rs/issues/201)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/202))
 

src/aac/read.rs

@@ -48,7 +48,7 @@ where
 
 				stream_len -= u64::from(header.size);
 
-				let id3v2 = parse_id3v2(reader, header)?;
+				let id3v2 = parse_id3v2(reader, header, parse_mode)?;
 				if let Some(existing_tag) = &mut file.id3v2_tag {
 					// https://github.com/Serial-ATA/lofty-rs/issues/87
 					// Duplicate tags should have their frames appended to the previous

src/ape/read.rs

@@ -40,7 +40,7 @@ where
 
 		let reader = &mut &*content;
 
-		let id3v2 = parse_id3v2(reader, header)?;
+		let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?;
 		id3v2_tag = Some(id3v2);
 	}
 

src/flac/read.rs

@@ -51,7 +51,7 @@ where
 	if let ID3FindResults(Some(header), Some(content)) = find_id3v2(data, true)? {
 		let reader = &mut &*content;
 
-		let id3v2 = parse_id3v2(reader, header)?;
+		let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?;
 		flac_file.id3v2_tag = Some(id3v2);
 	}
 
@@ -84,6 +84,7 @@ where
 				&mut &*block.content,
 				block.content.len() as u64,
 				&mut vorbis_comments,
+				parse_options.parsing_mode,
 			)?;
 
 			flac_file.vorbis_comments_tag = Some(vorbis_comments);

src/id3/v2/frame/content.rs

@@ -6,6 +6,7 @@ use crate::id3::v2::items::{
 };
 use crate::id3::v2::Id3v2Version;
 use crate::macros::err;
+use crate::probe::ParsingMode;
 use crate::util::text::TextEncoding;
 
 use std::io::Read;
@@ -15,6 +16,7 @@ pub(super) fn parse_content<R: Read>(
     reader: &mut R,
     id: &str,
     version: Id3v2Version,
+	parse_mode: ParsingMode,
 ) -> Result<Option<FrameValue>> {
 	Ok(match id {
 		// The ID was previously upgraded, but the content remains unchanged, so version is necessary
@@ -26,7 +28,7 @@ pub(super) fn parse_content<R: Read>(
 		"WXXX" => ExtendedUrlFrame::parse(reader, version)?.map(FrameValue::UserUrl),
 		"COMM" => CommentFrame::parse(reader, version)?.map(FrameValue::Comment),
 		"USLT" => UnsynchronizedTextFrame::parse(reader, version)?.map(FrameValue::UnsynchronizedText),
-		"UFID" => UniqueFileIdentifierFrame::parse(reader)?.map(FrameValue::UniqueFileIdentifier),
+		"UFID" => UniqueFileIdentifierFrame::parse(reader, parse_mode)?.map(FrameValue::UniqueFileIdentifier),
 		_ if id.starts_with('T') => TextInformationFrame::parse(reader, version)?.map(FrameValue::Text),
 		// Apple proprietary frames
 		// WFED (Podcast URL), GRP1 (Grouping), MVNM (Movement Name), MVIN (Movement Number)

src/id3/v2/frame/read.rs

@@ -5,13 +5,18 @@ use crate::id3::v2::frame::content::parse_content;
 use crate::id3::v2::util::synchsafe::{SynchsafeInteger, UnsynchronizedStream};
 use crate::id3::v2::{FrameFlags, FrameId, FrameValue, Id3v2Version};
 use crate::macros::try_vec;
+use crate::probe::ParsingMode;
 
 use std::io::Read;
 
 use byteorder::{BigEndian, ReadBytesExt};
 
 impl<'a> Frame<'a> {
-	pub(crate) fn read<R>(reader: &mut R, version: Id3v2Version) -> Result<(Option<Self>, bool)>
+	pub(crate) fn read<R>(
+		reader: &mut R,
+		version: Id3v2Version,
+		parse_mode: ParsingMode,
+	) -> Result<(Option<Self>, bool)>
 	where
 		R: Read,
 	{
@@ -91,14 +96,14 @@ impl<'a> Frame<'a> {
 						return handle_encryption(&mut compression_reader, size, id, flags);
 					}
 
-					return parse_frame(&mut compression_reader, id, flags, version);
+					return parse_frame(&mut compression_reader, id, flags, version, parse_mode);
 				}
 
 				if flags.encryption.is_some() {
 					return handle_encryption(&mut unsynchronized_reader, size, id, flags);
 				}
 
-				return parse_frame(&mut unsynchronized_reader, id, flags, version);
+				return parse_frame(&mut unsynchronized_reader, id, flags, version, parse_mode);
 			},
 			// Possible combinations:
 			//
@@ -113,7 +118,7 @@ impl<'a> Frame<'a> {
 					return handle_encryption(&mut compression_reader, size, id, flags);
 				}
 
-				return parse_frame(&mut compression_reader, id, flags, version);
+				return parse_frame(&mut compression_reader, id, flags, version, parse_mode);
 			},
 			// Possible combinations:
 			//
@@ -126,7 +131,7 @@ impl<'a> Frame<'a> {
 			},
 			// Everything else that doesn't have special flags
 			_ => {
-				return parse_frame(&mut reader, id, flags, version);
+				return parse_frame(&mut reader, id, flags, version, parse_mode);
 			},
 		}
 	}
@@ -172,8 +177,9 @@ fn parse_frame<R: Read>(
 	id: FrameId<'static>,
 	flags: FrameFlags,
 	version: Id3v2Version,
+	parse_mode: ParsingMode,
 ) -> Result<(Option<Frame<'static>>, bool)> {
-	match parse_content(reader, id.as_str(), version)? {
+	match parse_content(reader, id.as_str(), version, parse_mode)? {
 		Some(value) => Ok((Some(Frame { id, value, flags }), false)),
 		None => Ok((None, false)),
 	}

src/id3/v2/items/identifier.rs

@@ -1,6 +1,7 @@
-use crate::error::{Id3v2Error, Id3v2ErrorKind};
-use crate::util::text::{decode_text, encode_text};
-use crate::{Result, TextEncoding};
+use crate::error::{Id3v2Error, Id3v2ErrorKind, Result};
+use crate::macros::parse_mode_choice;
+use crate::probe::ParsingMode;
+use crate::util::text::{decode_text, encode_text, TextEncoding};
 
 use std::hash::{Hash, Hasher};
 use std::io::Read;
@@ -20,13 +21,23 @@ impl UniqueFileIdentifierFrame {
 	/// # Errors
 	///
 	/// Owner is missing or improperly encoded
-	pub fn parse<R>(reader: &mut R) -> Result<Option<Self>>
+	pub fn parse<R>(reader: &mut R, parse_mode: ParsingMode) -> Result<Option<Self>>
 	where
 		R: Read,
 	{
-		let Some(owner) = decode_text(reader, TextEncoding::Latin1, true)?.text_or_none() else {
-			return Err(Id3v2Error::new(Id3v2ErrorKind::MissingUfidOwner).into());
-		};
+		let owner_decode_result = decode_text(reader, TextEncoding::Latin1, true)?;
+
+		let owner;
+		match owner_decode_result.text_or_none() {
+			Some(valid) => owner = valid,
+			None => {
+				parse_mode_choice!(
+					parse_mode,
+					BESTATTEMPT: owner = String::new(),
+					DEFAULT: return Err(Id3v2Error::new(Id3v2ErrorKind::MissingUfidOwner).into())
+				);
+			},
+		}
 
 		let mut identifier = Vec::new();
 		reader.read_to_end(&mut identifier)?;
@@ -57,3 +68,24 @@ impl Hash for UniqueFileIdentifierFrame {
 		self.owner.hash(state);
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	#[test]
+	fn issue_204_invalid_ufid_parsing_mode_best_attempt() {
+		use crate::id3::v2::UniqueFileIdentifierFrame;
+		use crate::ParsingMode;
+
+		let ufid_no_owner = UniqueFileIdentifierFrame {
+			owner: String::new(),
+			identifier: vec![0],
+		};
+
+		let bytes = ufid_no_owner.as_bytes();
+
+		assert!(UniqueFileIdentifierFrame::parse(&mut &bytes[..], ParsingMode::Strict).is_err());
+		assert!(
+			UniqueFileIdentifierFrame::parse(&mut &bytes[..], ParsingMode::BestAttempt).is_ok()
+		);
+	}
+}

src/id3/v2/read.rs

@@ -3,10 +3,15 @@ use super::tag::Id3v2Tag;
 use super::Id3v2Header;
 use crate::error::Result;
 use crate::id3::v2::util::synchsafe::UnsynchronizedStream;
+use crate::probe::ParsingMode;
 
 use std::io::Read;
 
-pub(crate) fn parse_id3v2<R>(bytes: &mut R, header: Id3v2Header) -> Result<Id3v2Tag>
+pub(crate) fn parse_id3v2<R>(
+	bytes: &mut R,
+	header: Id3v2Header,
+	parse_mode: ParsingMode,
+) -> Result<Id3v2Tag>
 where
 	R: Read,
 {
@@ -16,20 +21,24 @@ where
 	if header.flags.unsynchronisation {
 		// Unsynchronize the entire tag
 		let mut unsyncronized_reader = UnsynchronizedStream::new(tag_bytes);
-		ret = read_all_frames_into_tag(&mut unsyncronized_reader, header)?;
+		ret = read_all_frames_into_tag(&mut unsyncronized_reader, header, parse_mode)?;
 
 		// Get the `Take` back from the `UnsynchronizedStream`
 		tag_bytes = unsyncronized_reader.into_inner();
 	} else {
-		ret = read_all_frames_into_tag(&mut tag_bytes, header)?;
+		ret = read_all_frames_into_tag(&mut tag_bytes, header, parse_mode)?;
 	};
 
 	// Throw away the rest of the tag (padding, bad frames)
 	std::io::copy(&mut tag_bytes, &mut std::io::sink())?;
 	Ok(ret)
 }
 
-fn read_all_frames_into_tag<R>(reader: &mut R, header: Id3v2Header) -> Result<Id3v2Tag>
+fn read_all_frames_into_tag<R>(
+	reader: &mut R,
+	header: Id3v2Header,
+	parse_mode: ParsingMode,
+) -> Result<Id3v2Tag>
 where
 	R: Read,
 {
@@ -38,7 +47,7 @@ where
 	tag.set_flags(header.flags);
 
 	loop {
-		match Frame::read(reader, header.version)? {
+		match Frame::read(reader, header.version, parse_mode)? {
 			// No frame content found, and we can expect there are no more frames
 			(None, true) => break,
 			(Some(f), false) => drop(tag.insert(f)),
@@ -53,9 +62,10 @@ where
 #[test]
 fn zero_size_id3v2() {
 	use crate::id3::v2::read_id3v2_header;
+	use crate::ParsingMode;
 	use std::io::Cursor;
 
 	let mut f = Cursor::new(std::fs::read("tests/tags/assets/id3v2/zero.id3v2").unwrap());
 	let header = read_id3v2_header(&mut f).unwrap();
-	assert!(parse_id3v2(&mut f, header).is_ok());
+	assert!(parse_id3v2(&mut f, header, ParsingMode::Strict).is_ok());
 }

src/id3/v2/tag.rs

@@ -1068,6 +1068,7 @@ impl<'a, I: Iterator<Item = FrameRef<'a>> + Clone + 'a> Id3v2TagRef<'a, I> {
 
 #[cfg(test)]
 mod tests {
+	use crate::ParsingMode;
 	use std::borrow::Cow;
 
 	use crate::id3::v2::frame::MUSICBRAINZ_UFID_OWNER;
@@ -1095,7 +1096,7 @@ mod tests {
 		let mut reader = std::io::Cursor::new(&tag_bytes[..]);
 
 		let header = read_id3v2_header(&mut reader).unwrap();
-		crate::id3::v2::read::parse_id3v2(&mut reader, header).unwrap()
+		crate::id3::v2::read::parse_id3v2(&mut reader, header, ParsingMode::Strict).unwrap()
 	}
 
 	#[test]
@@ -1206,7 +1207,9 @@ mod tests {
 		let temp_reader = &mut &*writer;
 
 		let temp_header = read_id3v2_header(temp_reader).unwrap();
-		let temp_parsed_tag = crate::id3::v2::read::parse_id3v2(temp_reader, temp_header).unwrap();
+		let temp_parsed_tag =
+			crate::id3::v2::read::parse_id3v2(temp_reader, temp_header, ParsingMode::Strict)
+				.unwrap();
 
 		assert_eq!(parsed_tag, temp_parsed_tag);
 	}
@@ -1456,7 +1459,7 @@ mod tests {
 		let mut reader = &mut &writer[..];
 
 		let header = read_id3v2_header(&mut reader).unwrap();
-		assert!(crate::id3::v2::read::parse_id3v2(reader, header).is_ok());
+		assert!(crate::id3::v2::read::parse_id3v2(reader, header, ParsingMode::Strict).is_ok());
 
 		assert_eq!(writer[3..10], writer[writer.len() - 7..])
 	}
@@ -1481,7 +1484,7 @@ mod tests {
 		let mut reader = &mut &writer[..];
 
 		let header = read_id3v2_header(&mut reader).unwrap();
-		let tag = crate::id3::v2::read::parse_id3v2(reader, header).unwrap();
+		let tag = crate::id3::v2::read::parse_id3v2(reader, header, ParsingMode::Strict).unwrap();
 
 		assert_eq!(tag.len(), 1);
 		assert_eq!(
@@ -1798,7 +1801,8 @@ mod tests {
 		let mut reader = std::io::Cursor::new(&content[..]);
 
 		let header = read_id3v2_header(&mut reader).unwrap();
-		let reparsed = crate::id3::v2::read::parse_id3v2(&mut reader, header).unwrap();
+		let reparsed =
+			crate::id3::v2::read::parse_id3v2(&mut reader, header, ParsingMode::Strict).unwrap();
 
 		assert_eq!(id3v2, reparsed);
 	}

src/iff/aiff/read.rs

@@ -52,7 +52,7 @@ where
 	while chunks.next(data).is_ok() {
 		match &chunks.fourcc {
 			b"ID3 " | b"id3 " => {
-				let tag = chunks.id3_chunk(data)?;
+				let tag = chunks.id3_chunk(data, parse_options.parsing_mode)?;
 				if let Some(existing_tag) = id3v2_tag.as_mut() {
 					// https://github.com/Serial-ATA/lofty-rs/issues/87
 					// Duplicate tags should have their frames appended to the previous

src/iff/chunk.rs

@@ -1,6 +1,7 @@
 use crate::error::Result;
 use crate::id3::v2::tag::Id3v2Tag;
 use crate::macros::{err, try_vec};
+use crate::probe::ParsingMode;
 
 use std::io::{Read, Seek, SeekFrom};
 use std::marker::PhantomData;
@@ -91,7 +92,7 @@ impl<B: ByteOrder> Chunks<B> {
 		Ok(content)
 	}
 
-	pub fn id3_chunk<R>(&mut self, data: &mut R) -> Result<Id3v2Tag>
+	pub fn id3_chunk<R>(&mut self, data: &mut R, parse_mode: ParsingMode) -> Result<Id3v2Tag>
 	where
 		R: Read + Seek,
 	{
@@ -103,7 +104,7 @@ impl<B: ByteOrder> Chunks<B> {
 		let reader = &mut &*content;
 
 		let header = read_id3v2_header(reader)?;
-		let id3v2 = parse_id3v2(reader, header)?;
+		let id3v2 = parse_id3v2(reader, header, parse_mode)?;
 
 		// Skip over the footer
 		if id3v2.flags().footer {

src/iff/wav/read.rs

@@ -88,7 +88,7 @@ where
 				}
 			},
 			b"ID3 " | b"id3 " => {
-				let tag = chunks.id3_chunk(data)?;
+				let tag = chunks.id3_chunk(data, parse_options.parsing_mode)?;
 				if let Some(existing_tag) = id3v2_tag.as_mut() {
 					// https://github.com/Serial-ATA/lofty-rs/issues/87
 					// Duplicate tags should have their frames appended to the previous

src/macros.rs

@@ -72,22 +72,26 @@ macro_rules! parse_mode_choice {
 	(
 		$parse_mode:ident,
 		$(STRICT: $strict_handler:expr,)?
+		$(BESTATTEMPT: $best_attempt_handler:expr,)?
 		$(RELAXED: $relaxed_handler:expr,)?
 		DEFAULT: $default:expr
 	) => {
 		match $parse_mode {
 			$(crate::probe::ParsingMode::Strict => { $strict_handler },)?
+			$(crate::probe::ParsingMode::BestAttempt => { $best_attempt_handler },)?
 			$(crate::probe::ParsingMode::Relaxed => { $relaxed_handler },)?
 			_ => { $default }
 		}
 	};
 	(
 		$parse_mode:ident,
 		$(STRICT: $strict_handler:expr,)?
+		$(BESTATTEMPT: $best_attempt_handler:expr,)?
 		$(RELAXED: $relaxed_handler:expr $(,)?)?
 	) => {
 		match $parse_mode {
 			$(crate::probe::ParsingMode::Strict => { $strict_handler },)?
+			$(crate::probe::ParsingMode::BestAttempt => { $best_attempt_handler },)?
 			$(crate::probe::ParsingMode::Relaxed => { $relaxed_handler },)?
 			#[allow(unreachable_patterns)]
 			_ => {}