Verify ID3 Frame contents prior to writing

Author: Serial-ATA

src/error.rs

@@ -49,6 +49,9 @@ pub enum LoftyError {
 	#[cfg(feature = "id3v2")]
 	/// Arises when invalid data is encountered while reading an ID3v2 synchronized text frame
 	BadSyncText,
+	#[cfg(feature = "id3v2")]
+	/// Arises when attempting to write an invalid Frame (Bad `FrameID`/`FrameValue` pairing)
+	BadFrame(String, &'static str),
 	/// Arises when an atom contains invalid data
 	BadAtom(&'static str),
 
@@ -131,6 +134,12 @@ impl Display for LoftyError {
 			),
 			#[cfg(feature = "id3v2")]
 			LoftyError::BadSyncText => write!(f, "ID3v2: Encountered invalid data in SYLT frame"),
+			#[cfg(feature = "id3v2")]
+			LoftyError::BadFrame(frame_id, frame_value) => write!(
+				f,
+				"ID3v2: Attempted to write an invalid frame. ID: \"{}\", Value: \"{}\"",
+				frame_id, frame_value
+			),
 			LoftyError::BadAtom(message) => write!(f, "MP4 Atom: {}", message),
 
 			// Files

src/id3/v2/frame/content.rs

@@ -117,7 +117,7 @@ pub(super) fn parse_content(
 		"TXXX" => parse_user_defined(content, false)?,
 		"WXXX" => parse_user_defined(content, true)?,
 		"COMM" | "USLT" => parse_text_language(content, id)?,
-		_ if id.starts_with('T') || id == "WFED" => parse_text(content)?,
+		_ if id.starts_with('T') => parse_text(content)?,
 		// Apple proprietary frames
 		"WFED" | "GRP1" => parse_text(content)?,
 		_ if id.starts_with('W') => parse_link(content)?,

src/id3/v2/tag.rs

@@ -282,6 +282,7 @@ impl Id3v2Tag {
 	///
 	/// * Attempting to write the tag to a format that does not support it
 	/// * Attempting to write an encrypted frame without a valid method symbol or data length indicator
+	/// * Attempting to write an invalid [`FrameID`]/[`FrameValue`] pairing
 	pub fn write_to(&self, file: &mut File) -> Result<()> {
 		Id3v2TagRef::new(self.flags, self.frames.iter().filter_map(Frame::as_opt_ref))
 			.write_to(file)
@@ -576,6 +577,26 @@ mod tests {
 		crate::tag_utils::test_utils::verify_tag(&tag, true, true);
 	}
 
+	#[test]
+	fn fail_write_bad_frame() {
+		let mut tag = Id3v2Tag::default();
+		tag.insert(Frame {
+			id: FrameID::Valid(String::from("ABCD")),
+			value: FrameValue::URL(String::from("FOO URL")),
+			flags: Default::default(),
+		});
+
+		let res = tag.dump_to(&mut Vec::<u8>::new());
+
+		assert!(res.is_err());
+		assert_eq!(
+			res.unwrap_err().to_string(),
+			String::from(
+				"ID3v2: Attempted to write an invalid frame. ID: \"ABCD\", Value: \"URL\""
+			)
+		);
+	}
+
 	#[test]
 	fn tag_to_id3v2() {
 		fn verify_frame(tag: &Id3v2Tag, id: &str, value: &str) {

src/id3/v2/write/frame.rs

@@ -1,5 +1,5 @@
 use crate::error::{LoftyError, Result};
-use crate::id3::v2::frame::{FrameFlags, FrameRef};
+use crate::id3::v2::frame::{FrameFlags, FrameRef, FrameValue};
 use crate::id3::v2::synch_u32;
 
 use std::io::Write;
@@ -14,6 +14,7 @@ where
 	W: Write,
 {
 	for frame in frames {
+		verify_frame(&frame)?;
 		let value = frame.value.as_bytes()?;
 
 		write_frame(writer, frame.id, frame.flags, &value)?;
@@ -22,6 +23,33 @@ where
 	Ok(())
 }
 
+fn verify_frame(frame: &FrameRef) -> Result<()> {
+	match (frame.id, frame.value.as_ref()) {
+		("APIC", FrameValue::Picture { .. })
+		| ("USLT", FrameValue::UnSyncText(_))
+		| ("COMM", FrameValue::Comment(_))
+		| ("TXXX", FrameValue::UserText(_))
+		| ("WXXX", FrameValue::UserURL(_))
+		| (_, FrameValue::Binary(_))
+		| ("WFED" | "GRP1", FrameValue::Text { .. }) => Ok(()),
+		(id, FrameValue::Text { .. }) if id.starts_with('T') => Ok(()),
+		(id, FrameValue::URL(_)) if id.starts_with('W') => Ok(()),
+		(id, frame_value) => Err(LoftyError::BadFrame(
+			id.to_string(),
+			match frame_value {
+				FrameValue::Comment(_) => "Comment",
+				FrameValue::UnSyncText(_) => "UnSyncText",
+				FrameValue::Text { .. } => "Text",
+				FrameValue::UserText(_) => "UserText",
+				FrameValue::URL(_) => "URL",
+				FrameValue::UserURL(_) => "UserURL",
+				FrameValue::Picture { .. } => "Picture",
+				FrameValue::Binary(_) => "Binary",
+			},
+		)),
+	}
+}
+
 fn write_frame<W>(writer: &mut W, name: &str, flags: FrameFlags, value: &[u8]) -> Result<()>
 where
 	W: Write,