From fcb54469229bd2d42485eb4ff7df5f8f0424f5f5 Mon Sep 17 00:00:00 2001
From: Serial <69764315+Serial-ATA@users.noreply.github.com>
Date: Thu, 4 Jul 2024 12:10:22 -0400
Subject: [PATCH] ID3v2: Ignore empty timestamp frames

I had a file with an empty timestamp frame that errored with `ParsingMode::BestAttempt`. Now that's only an error case with `ParsingMode::Strict`.
---
 lofty/src/id3/v2/items/timestamp_frame.rs |  7 ++++-
 lofty/src/tag/items/timestamp.rs          | 33 ++++++++++++++++++-----
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/lofty/src/id3/v2/items/timestamp_frame.rs b/lofty/src/id3/v2/items/timestamp_frame.rs
index 9e457a3b..d4997350 100644
--- a/lofty/src/id3/v2/items/timestamp_frame.rs
+++ b/lofty/src/id3/v2/items/timestamp_frame.rs
@@ -96,7 +96,12 @@ impl<'a> TimestampFrame<'a> {
 
 		let reader = &mut value.as_bytes();
 
-		frame.timestamp = Timestamp::parse(reader, parse_mode)?;
+		let Some(timestamp) = Timestamp::parse(reader, parse_mode)? else {
+			// Timestamp is empty
+			return Ok(None);
+		};
+
+		frame.timestamp = timestamp;
 		Ok(Some(frame))
 	}
 
diff --git a/lofty/src/tag/items/timestamp.rs b/lofty/src/tag/items/timestamp.rs
index efc131d8..d5ff8bb7 100644
--- a/lofty/src/tag/items/timestamp.rs
+++ b/lofty/src/tag/items/timestamp.rs
@@ -70,7 +70,8 @@ impl FromStr for Timestamp {
 	type Err = LoftyError;
 
 	fn from_str(s: &str) -> Result<Self> {
-		Timestamp::parse(&mut s.as_bytes(), ParsingMode::BestAttempt)
+		Timestamp::parse(&mut s.as_bytes(), ParsingMode::BestAttempt)?
+			.ok_or_else(|| LoftyError::new(ErrorKind::BadTimestamp("Timestamp frame is empty")))
 	}
 }
 
@@ -86,7 +87,7 @@ impl Timestamp {
 	///
 	/// * Failure to read from `reader`
 	/// * The timestamp is invalid
-	pub fn parse<R>(reader: &mut R, parse_mode: ParsingMode) -> Result<Self>
+	pub fn parse<R>(reader: &mut R, parse_mode: ParsingMode) -> Result<Option<Self>>
 	where
 		R: Read,
 	{
@@ -109,6 +110,14 @@ impl Timestamp {
 			.take(Self::MAX_LENGTH as u64)
 			.read_to_end(&mut content)?;
 
+		if content.is_empty() {
+			if parse_mode == ParsingMode::Strict {
+				err!(BadTimestamp("Timestamp frame is empty"))
+			}
+
+			return Ok(None);
+		}
+
 		let reader = &mut &content[..];
 
 		// We need to very that the year is exactly 4 bytes long. This doesn't matter for other segments.
@@ -131,7 +140,7 @@ impl Timestamp {
 			break;
 		}
 
-		Ok(timestamp)
+		Ok(Some(timestamp))
 	}
 
 	fn segment<const SIZE: usize>(
@@ -237,7 +246,7 @@ mod tests {
 		let parsed_timestamp =
 			Timestamp::parse(&mut content.as_bytes(), ParsingMode::Strict).unwrap();
 
-		assert_eq!(parsed_timestamp, expected());
+		assert_eq!(parsed_timestamp, Some(expected()));
 	}
 
 	#[test]
@@ -248,7 +257,7 @@ mod tests {
 		let parsed_timestamp =
 			Timestamp::parse(&mut content.as_bytes(), ParsingMode::BestAttempt).unwrap();
 
-		assert_eq!(parsed_timestamp, expected());
+		assert_eq!(parsed_timestamp, Some(expected()));
 	}
 
 	#[test]
@@ -259,7 +268,7 @@ mod tests {
 		let parsed_timestamp =
 			Timestamp::parse(&mut content.as_bytes(), ParsingMode::BestAttempt).unwrap();
 
-		assert_eq!(parsed_timestamp, expected());
+		assert_eq!(parsed_timestamp, Some(expected()));
 	}
 
 	#[test]
@@ -348,7 +357,17 @@ mod tests {
 
 		for (data, expected) in partial_timestamps {
 			let parsed_timestamp = Timestamp::parse(&mut &data[..], ParsingMode::Strict).unwrap();
-			assert_eq!(parsed_timestamp, expected);
+			assert_eq!(parsed_timestamp, Some(expected));
 		}
 	}
+
+	#[test]
+	fn empty_timestamp() {
+		let empty_timestamp =
+			Timestamp::parse(&mut "".as_bytes(), ParsingMode::BestAttempt).unwrap();
+		assert!(empty_timestamp.is_none());
+
+		let empty_timestamp_strict = Timestamp::parse(&mut "".as_bytes(), ParsingMode::Strict);
+		assert!(empty_timestamp_strict.is_err());
+	}
 }