Skip to content

Commit 45e7065

Browse files
committed
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`.
1 parent d4e58ea commit 45e7065

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

lofty/src/id3/v2/items/timestamp_frame.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,12 @@ impl<'a> TimestampFrame<'a> {
9696

9797
let reader = &mut value.as_bytes();
9898

99-
frame.timestamp = Timestamp::parse(reader, parse_mode)?;
99+
let Some(timestamp) = Timestamp::parse(reader, parse_mode)? else {
100+
// Timestamp is empty
101+
return Ok(None);
102+
};
103+
104+
frame.timestamp = timestamp;
100105
Ok(Some(frame))
101106
}
102107

lofty/src/tag/items/timestamp.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ impl FromStr for Timestamp {
7070
type Err = LoftyError;
7171

7272
fn from_str(s: &str) -> Result<Self> {
73-
Timestamp::parse(&mut s.as_bytes(), ParsingMode::BestAttempt)
73+
Timestamp::parse(&mut s.as_bytes(), ParsingMode::BestAttempt)?
74+
.ok_or_else(|| LoftyError::new(ErrorKind::BadTimestamp("Timestamp frame is empty")))
7475
}
7576
}
7677

@@ -86,7 +87,7 @@ impl Timestamp {
8687
///
8788
/// * Failure to read from `reader`
8889
/// * The timestamp is invalid
89-
pub fn parse<R>(reader: &mut R, parse_mode: ParsingMode) -> Result<Self>
90+
pub fn parse<R>(reader: &mut R, parse_mode: ParsingMode) -> Result<Option<Self>>
9091
where
9192
R: Read,
9293
{
@@ -109,6 +110,14 @@ impl Timestamp {
109110
.take(Self::MAX_LENGTH as u64)
110111
.read_to_end(&mut content)?;
111112

113+
if content.is_empty() {
114+
if parse_mode == ParsingMode::Strict {
115+
err!(BadTimestamp("Timestamp frame is empty"))
116+
}
117+
118+
return Ok(None);
119+
}
120+
112121
let reader = &mut &content[..];
113122

114123
// 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 {
131140
break;
132141
}
133142

134-
Ok(timestamp)
143+
Ok(Some(timestamp))
135144
}
136145

137146
fn segment<const SIZE: usize>(
@@ -237,7 +246,7 @@ mod tests {
237246
let parsed_timestamp =
238247
Timestamp::parse(&mut content.as_bytes(), ParsingMode::Strict).unwrap();
239248

240-
assert_eq!(parsed_timestamp, expected());
249+
assert_eq!(parsed_timestamp, Some(expected()));
241250
}
242251

243252
#[test]
@@ -248,7 +257,7 @@ mod tests {
248257
let parsed_timestamp =
249258
Timestamp::parse(&mut content.as_bytes(), ParsingMode::BestAttempt).unwrap();
250259

251-
assert_eq!(parsed_timestamp, expected());
260+
assert_eq!(parsed_timestamp, Some(expected()));
252261
}
253262

254263
#[test]
@@ -259,7 +268,7 @@ mod tests {
259268
let parsed_timestamp =
260269
Timestamp::parse(&mut content.as_bytes(), ParsingMode::BestAttempt).unwrap();
261270

262-
assert_eq!(parsed_timestamp, expected());
271+
assert_eq!(parsed_timestamp, Some(expected()));
263272
}
264273

265274
#[test]
@@ -348,7 +357,17 @@ mod tests {
348357

349358
for (data, expected) in partial_timestamps {
350359
let parsed_timestamp = Timestamp::parse(&mut &data[..], ParsingMode::Strict).unwrap();
351-
assert_eq!(parsed_timestamp, expected);
360+
assert_eq!(parsed_timestamp, Some(expected));
352361
}
353362
}
363+
364+
#[test]
365+
fn empty_timestamp() {
366+
let empty_timestamp =
367+
Timestamp::parse(&mut "".as_bytes(), ParsingMode::BestAttempt).unwrap();
368+
assert!(empty_timestamp.is_none());
369+
370+
let empty_timestamp_strict = Timestamp::parse(&mut "".as_bytes(), ParsingMode::Strict);
371+
assert!(empty_timestamp_strict.is_err());
372+
}
354373
}

0 commit comments

Comments
 (0)