From 003848c087dbc8c58f3b23ca8468da8392e53d1c Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 2 Jul 2023 12:15:51 +0200 Subject: [PATCH] id3v2: Report bad or unsupported frame IDs --- src/error.rs | 12 ++++++++++-- src/id3/v2/frame/header.rs | 8 +++++--- src/id3/v2/frame/id.rs | 11 ++++++++--- src/id3/v2/frame/mod.rs | 22 ++++++++++++++++++---- src/id3/v2/tag.rs | 8 ++++++-- 5 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/error.rs b/src/error.rs index 7c35d73a..cdb2289f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,6 +4,7 @@ //! which can be extended at any time. use crate::file::FileType; +use crate::ItemKey; use std::collections::TryReserveError; use std::fmt::{Debug, Display, Formatter}; @@ -83,7 +84,11 @@ pub enum Id3v2ErrorKind { // Frame /// Arises when a frame ID contains invalid characters (must be within `'A'..'Z'` or `'0'..'9'`) - BadFrameId, + /// or if the ID is too short/long. + BadFrameId(Vec), + /// Arises when no frame ID is available in the ID3v2 specification for an item key + /// and the associated value type. + UnsupportedFrameId(ItemKey), /// Arises when a frame doesn't have enough data BadFrameLength, /// Arises when reading/writing a compressed or encrypted frame with no data length indicator @@ -133,7 +138,10 @@ impl Display for Id3v2ErrorKind { }, // Frame - Self::BadFrameId => write!(f, "Failed to parse a frame ID"), + Self::BadFrameId(frame_id) => write!(f, "Failed to parse a frame ID: 0x{frame_id:x?}"), + Self::UnsupportedFrameId(item_key) => { + write!(f, "Unsupported frame ID for item key {item_key:?}") + }, Self::BadFrameLength => write!( f, "Frame isn't long enough to extract the necessary information" diff --git a/src/id3/v2/frame/header.rs b/src/id3/v2/frame/header.rs index 72812292..a2fd19e5 100644 --- a/src/id3/v2/frame/header.rs +++ b/src/id3/v2/frame/header.rs @@ -24,8 +24,9 @@ where return Ok(None); } - let id_str = std::str::from_utf8(&frame_header[..3]) - .map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadFrameId))?; + let frame_id = &frame_header[..3]; + let id_str = std::str::from_utf8(frame_id) + .map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadFrameId(frame_id.to_vec())))?; let id = upgrade_v2(id_str).map_or_else(|| Cow::Owned(id_str.to_owned()), Cow::Borrowed); let frame_id = FrameId::new_cow(id)?; @@ -63,8 +64,9 @@ where frame_id_end = 3; } + let frame_id = &frame_header[..frame_id_end]; let id_str = std::str::from_utf8(&frame_header[..frame_id_end]) - .map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadFrameId))?; + .map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadFrameId(frame_id.to_vec())))?; let mut size = u32::from_be_bytes([ frame_header[4], diff --git a/src/id3/v2/frame/id.rs b/src/id3/v2/frame/id.rs index 28f00991..ef9b9b82 100644 --- a/src/id3/v2/frame/id.rs +++ b/src/id3/v2/frame/id.rs @@ -38,7 +38,9 @@ impl<'a> FrameId<'a> { match id.len() { 3 => Ok(FrameId::Outdated(id)), 4 => Ok(FrameId::Valid(id)), - _ => Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId).into()), + _ => Err( + Id3v2Error::new(Id3v2ErrorKind::BadFrameId(id.into_owned().into_bytes())).into(), + ), } } @@ -52,7 +54,10 @@ impl<'a> FrameId<'a> { pub(super) fn verify_id(id_str: &str) -> Result<()> { for c in id_str.chars() { if !c.is_ascii_uppercase() && !c.is_ascii_digit() { - return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId).into()); + return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId( + id_str.as_bytes().to_vec(), + )) + .into()); } } @@ -93,7 +98,7 @@ impl<'a> TryFrom<&'a ItemKey> for FrameId<'a> { } } - Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId).into()) + Err(Id3v2Error::new(Id3v2ErrorKind::UnsupportedFrameId(k.clone())).into()) }, } } diff --git a/src/id3/v2/frame/mod.rs b/src/id3/v2/frame/mod.rs index af63954f..64bf0e38 100644 --- a/src/id3/v2/frame/mod.rs +++ b/src/id3/v2/frame/mod.rs @@ -112,7 +112,12 @@ impl<'a> Frame<'a> { None => id, Some(upgraded) => Cow::Borrowed(upgraded), }, - _ => return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId).into()), + _ => { + return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId( + id.into_owned().into_bytes(), + )) + .into()) + }, }; let id = FrameId::new_cow(id_upgraded)?; @@ -504,8 +509,12 @@ impl<'a> TryFrom<&'a TagItem> for FrameRef<'a> { frame_id = id; }, Err(_) => { - let Some(desc) = tag_item.key().map_key(TagType::Id3v2, true) else { - return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId).into()); + let item_key = tag_item.key(); + let Some(desc) = item_key.map_key(TagType::Id3v2, true) else { + return Err(Id3v2Error::new(Id3v2ErrorKind::UnsupportedFrameId( + item_key.clone(), + )) + .into()); }; match tag_item.value() { @@ -525,7 +534,12 @@ impl<'a> TryFrom<&'a TagItem> for FrameRef<'a> { content: locator.clone(), }) }, - _ => return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId).into()), + _ => { + return Err(Id3v2Error::new(Id3v2ErrorKind::UnsupportedFrameId( + item_key.clone(), + )) + .into()) + }, } }, } diff --git a/src/id3/v2/tag.rs b/src/id3/v2/tag.rs index a848c49b..8768d6d3 100644 --- a/src/id3/v2/tag.rs +++ b/src/id3/v2/tag.rs @@ -533,7 +533,9 @@ impl Accessor for Id3v2Tag { fn set_comment(&mut self, value: String) { let mut value = Some(value); self.frames.retain_mut(|frame| { - let Some(CommentFrame { content, .. }) = filter_comment_frame_by_description_mut(frame, &EMPTY_CONTENT_DESCRIPTOR) else { + let Some(CommentFrame { content, .. }) = + filter_comment_frame_by_description_mut(frame, &EMPTY_CONTENT_DESCRIPTOR) + else { return true; }; if let Some(value) = value.take() { @@ -759,7 +761,9 @@ impl SplitTag for Id3v2Tag { ) => { if owner == MUSICBRAINZ_UFID_OWNER { let mut identifier = Cursor::new(identifier); - let Ok(recording_id) = decode_text(&mut identifier, TextEncoding::Latin1, false) else { + let Ok(recording_id) = + decode_text(&mut identifier, TextEncoding::Latin1, false) + else { return true; // Keep frame }; tag.items.push(TagItem::new(