ID3v2: Remove ID3v2ErrorKind::Other

CHANGELOG.md

@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Removed
+- **ID3v2**: All uses of `ID3v2ErrorKind::Other` have been replaced with concrete errors
+
 ## [0.12.1] - 2023-04-10
 
 ### Fixed

src/error.rs

@@ -67,50 +67,113 @@ pub enum ErrorKind {
 }
 
 /// The types of errors that can occur while interacting with ID3v2 tags
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 #[non_exhaustive]
 pub enum ID3v2ErrorKind {
-	/// Arises when an invalid picture format is parsed. Only applicable to [`ID3v2Version::V2`](crate::id3::v2::ID3v2Version::V2)
-	BadPictureFormat(String),
+	// Header
 	/// Arises when an invalid ID3v2 version is found
 	BadId3v2Version(u8, u8),
+	/// Arises when a compressed ID3v2.2 tag is encountered
+	///
+	/// At the time the ID3v2.2 specification was written, a compression scheme wasn't decided.
+	/// As such, it is recommended to ignore the tag entirely.
+	V2Compression,
+	/// Arises when an extended header has an invalid size (must be >= 6 bytes)
+	BadExtendedHeaderSize,
+
+	// Frame
 	/// Arises when a frame ID contains invalid characters (must be within `'A'..'Z'` or `'0'..'9'`)
 	BadFrameID,
 	/// Arises when a frame doesn't have enough data
 	BadFrameLength,
+	/// Arises when reading/writing a compressed or encrypted frame with no data length indicator
+	MissingDataLengthIndicator,
+	/// Arises when a frame or tag has its unsynchronisation flag set, but the content is not actually synchsafe
+	///
+	/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
+	InvalidUnsynchronisation,
+	/// Arises when a text encoding other than Latin-1 or UTF-16 appear in an ID3v2.2 tag
+	V2InvalidTextEncoding,
+	/// Arises when an invalid picture format is parsed. Only applicable to [`ID3v2Version::V2`](crate::id3::v2::ID3v2Version::V2)
+	BadPictureFormat(String),
 	/// Arises when invalid data is encountered while reading an ID3v2 synchronized text frame
 	BadSyncText,
+	/// Arises when decoding a [`UniqueFileIdentifierFrame`](crate::id3::v2::UniqueFileIdentifierFrame) with no owner
+	MissingUFIDOwner,
+
+	// Compression
+	#[cfg(feature = "id3v2_compression_support")]
+	/// Arises when a compressed frame is unable to be decompressed
+	Decompression(flate2::DecompressError),
+	#[cfg(not(feature = "id3v2_compression_support"))]
+	/// Arises when a compressed frame is encountered, but support is disabled
+	CompressedFrameEncountered,
+
+	// Writing
+	/// Arises when attempting to write an encrypted frame with an invalid encryption method symbol (must be <= 0x80)
+	InvalidEncryptionMethodSymbol(u8),
 	/// Arises when attempting to write an invalid Frame (Bad `FrameID`/`FrameValue` pairing)
 	BadFrame(String, &'static str),
-	/// A catch-all for all remaining errors
-	///
-	/// NOTE: This will likely be deprecated in the future
-	Other(&'static str),
+	/// Arises when attempting to write a [`LanguageFrame`](crate::id3::v2::LanguageFrame) with an invalid language
+	InvalidLanguage([u8; 3]),
 }
 
 impl Display for ID3v2ErrorKind {
 	fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
 		match self {
-			ID3v2ErrorKind::BadId3v2Version(major, minor) => write!(
+			// Header
+			Self::BadId3v2Version(major, minor) => write!(
 				f,
 				"Found an invalid version (v{major}.{minor}), expected any major revision in: (2, \
 				 3, 4)"
 			),
-			ID3v2ErrorKind::BadFrameID => write!(f, "Failed to parse a frame ID"),
-			ID3v2ErrorKind::BadFrameLength => write!(
+			Self::V2Compression => write!(f, "Encountered a compressed ID3v2.2 tag"),
+			Self::BadExtendedHeaderSize => {
+				write!(f, "Found an extended header with an invalid size (< 6)")
+			},
+
+			// Frame
+			Self::BadFrameID => write!(f, "Failed to parse a frame ID"),
+			Self::BadFrameLength => write!(
 				f,
 				"Frame isn't long enough to extract the necessary information"
 			),
-			ID3v2ErrorKind::BadSyncText => write!(f, "Encountered invalid data in SYLT frame"),
-			ID3v2ErrorKind::BadFrame(ref frame_id, frame_value) => write!(
+			Self::MissingDataLengthIndicator => write!(
 				f,
-				"Attempted to write an invalid frame. ID: \"{}\", Value: \"{}\"",
-				frame_id, frame_value
+				"Encountered an encrypted frame without a data length indicator"
 			),
-			ID3v2ErrorKind::BadPictureFormat(format) => {
+			Self::InvalidUnsynchronisation => write!(f, "Encountered an invalid unsynchronisation"),
+			Self::V2InvalidTextEncoding => {
+				write!(f, "ID3v2.2 only supports Latin-1 and UTF-16 encodings")
+			},
+			Self::BadPictureFormat(format) => {
 				write!(f, "Picture: Found unexpected format \"{format}\"")
 			},
-			ID3v2ErrorKind::Other(message) => write!(f, "{message}"),
+			Self::BadSyncText => write!(f, "Encountered invalid data in SYLT frame"),
+			Self::MissingUFIDOwner => write!(f, "Missing owner in UFID frame"),
+
+			// Compression
+			#[cfg(feature = "id3v2_compression_support")]
+			Self::Decompression(err) => write!(f, "Failed to decompress frame: {err}"),
+			#[cfg(not(feature = "id3v2_compression_support"))]
+			Self::CompressedFrameEncountered => write!(
+				f,
+				"Encountered a compressed ID3v2 frame, support is disabled"
+			),
+
+			// Writing
+			Self::InvalidEncryptionMethodSymbol(symbol) => write!(
+				f,
+				"Attempted to write an encrypted frame with an invalid method symbol ({symbol})"
+			),
+			Self::BadFrame(ref frame_id, frame_value) => write!(
+				f,
+				"Attempted to write an invalid frame. ID: \"{frame_id}\", Value: \"{frame_value}\"",
+			),
+			Self::InvalidLanguage(lang) => write!(
+				f,
+				"Invalid frame language found: {lang:?} (expected 3 ascii characters)"
+			),
 		}
 	}
 }
@@ -128,8 +191,8 @@ impl ID3v2Error {
 	}
 
 	/// Returns the [`ID3v2ErrorKind`]
-	pub fn kind(&self) -> ID3v2ErrorKind {
-		self.kind.clone()
+	pub fn kind(&self) -> &ID3v2ErrorKind {
+		&self.kind
 	}
 }
 

src/id3/v2/frame/content.rs

@@ -164,13 +164,8 @@ fn parse_link(content: &mut &[u8]) -> Result<Option<FrameValue>> {
 }
 
 fn verify_encoding(encoding: u8, version: ID3v2Version) -> Result<TextEncoding> {
-	if let ID3v2Version::V2 = version {
-		if encoding != 0 && encoding != 1 {
-			return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-				"ID3v2.2 only supports Latin-1 and UTF-16 encodings",
-			))
-			.into());
-		}
+	if version == ID3v2Version::V2 && (encoding != 0 && encoding != 1) {
+		return Err(ID3v2Error::new(ID3v2ErrorKind::V2InvalidTextEncoding).into());
 	}
 
 	match TextEncoding::from_u8(encoding) {

src/id3/v2/frame/read.rs

@@ -36,18 +36,14 @@ impl<'a> Frame<'a> {
 			let mut decompressed = Vec::new();
 			flate2::Decompress::new(true)
 				.decompress_vec(&content, &mut decompressed, flate2::FlushDecompress::None)
-				.map_err(|_| {
-					ID3v2Error::new(ID3v2ErrorKind::Other(
-						"Encountered a compressed frame, failed to decompress",
-					))
-				})?;
+				.map_err(|err| ID3v2Error::new(ID3v2ErrorKind::Decompression(err)))?;
 
 			content = decompressed
 		}
 
 		#[cfg(not(feature = "id3v2_compression_support"))]
 		if flags.compression {
-			crate::macros::decode_err!(@BAIL ID3v2, "Encountered a compressed ID3v2 frame, support is disabled")
+			return Err(ID3v2Error::new(ID3v2ErrorKind::CompressedFrameEncountered).into());
 		}
 
 		let mut content_reader = &*content;
@@ -69,10 +65,7 @@ impl<'a> Frame<'a> {
 
 		let value = if flags.encryption.is_some() {
 			if flags.data_length_indicator.is_none() {
-				return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-					"Encountered an encrypted frame without a data length indicator",
-				))
-				.into());
+				return Err(ID3v2Error::new(ID3v2ErrorKind::MissingDataLengthIndicator).into());
 			}
 
 			Some(FrameValue::Binary(content))

src/id3/v2/items/identifier.rs

@@ -36,9 +36,7 @@ impl UniqueFileIdentifierFrame {
 		}
 
 		let Some(owner) = decode_text(input, TextEncoding::Latin1, true)? else {
-			return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-				"Missing owner in UFID frame",
-			)).into());
+			return Err(ID3v2Error::new(ID3v2ErrorKind::MissingUFIDOwner).into());
 		};
 		let identifier = input.to_vec();
 

src/id3/v2/items/language_frame.rs

@@ -43,10 +43,7 @@ impl LanguageFrame {
 		let mut bytes = vec![self.encoding as u8];
 
 		if self.language.len() != 3 || self.language.iter().any(|c| !c.is_ascii_alphabetic()) {
-			return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-				"Invalid frame language found (expected 3 ascii characters)",
-			))
-			.into());
+			return Err(ID3v2Error::new(ID3v2ErrorKind::InvalidLanguage(self.language)).into());
 		}
 
 		bytes.extend(self.language);

src/id3/v2/mod.rs

@@ -93,10 +93,7 @@ where
 	// At the time the ID3v2.2 specification was written, a compression scheme wasn't decided.
 	// The spec recommends just ignoring the tag in this case.
 	if version == ID3v2Version::V2 && flags & 0x40 == 0x40 {
-		return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-			"Encountered a compressed ID3v2.2 tag",
-		))
-		.into());
+		return Err(ID3v2Error::new(ID3v2ErrorKind::V2Compression).into());
 	}
 
 	let mut flags_parsed = ID3v2TagFlags {
@@ -119,10 +116,7 @@ where
 		extended_size = unsynch_u32(bytes.read_u32::<BigEndian>()?);
 
 		if extended_size < 6 {
-			return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-				"Found an extended header with an invalid size (< 6)",
-			))
-			.into());
+			return Err(ID3v2Error::new(ID3v2ErrorKind::BadExtendedHeaderSize).into());
 		}
 
 		// Useless byte since there's only 1 byte for flags
@@ -148,10 +142,6 @@ where
 		}
 	}
 
-	if extended_size > 0 && extended_size >= size {
-		return Err(ID3v2Error::new(ID3v2ErrorKind::Other("Tag has an invalid size")).into());
-	}
-
 	Ok(ID3v2Header {
 		version,
 		flags: flags_parsed,

src/id3/v2/util/mod.rs

@@ -19,10 +19,7 @@ pub(in crate::id3::v2) fn unsynch_content(content: &[u8]) -> Result<Vec<u8>> {
 		// Then remove the next byte if it is a zero
 		if discard {
 			if content[next] >= 0xE0 {
-				return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-					"Encountered an invalid unsynchronisation",
-				))
-				.into());
+				return Err(ID3v2Error::new(ID3v2ErrorKind::InvalidUnsynchronisation).into());
 			}
 
 			if content[next] == 0 {

src/id3/v2/write/frame.rs

@@ -91,10 +91,9 @@ where
 	let method_symbol = flags.encryption.unwrap();
 
 	if method_symbol > 0x80 {
-		return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-			"Attempted to write an encrypted frame with an invalid method symbol (> 0x80)",
-		))
-		.into());
+		return Err(
+			ID3v2Error::new(ID3v2ErrorKind::InvalidEncryptionMethodSymbol(method_symbol)).into(),
+		);
 	}
 
 	if let Some(len) = flags.data_length_indicator {
@@ -108,10 +107,7 @@ where
 		}
 	}
 
-	Err(ID3v2Error::new(ID3v2ErrorKind::Other(
-		"Attempted to write an encrypted frame without a data length indicator",
-	))
-	.into())
+	Err(ID3v2Error::new(ID3v2ErrorKind::MissingDataLengthIndicator).into())
 }
 
 fn write_frame_header<W>(writer: &mut W, name: &str, len: u32, flags: FrameFlags) -> Result<()>