ID3v2: Allow creation of more synchsafe integer types

src/file.rs

@@ -913,7 +913,7 @@ impl FileType {
 
 	// TODO: APE tags in the beginning of the file
 	pub(crate) fn from_buffer_inner(buf: &[u8]) -> (Option<Self>, Option<u32>) {
-		use crate::id3::v2::util::unsynch_u32;
+		use crate::id3::v2::util::synchsafe::SynchsafeInteger;
 
 		// Start out with an empty return: (File type, id3 size)
 		// Only one can be set
@@ -931,7 +931,7 @@ impl FileType {
 				// This is infallible, but preferable to an unwrap
 				if let Ok(arr) = buf[6..10].try_into() {
 					// Set the ID3v2 size
-					ret.1 = Some(unsynch_u32(u32::from_be_bytes(arr)));
+					ret.1 = Some(u32::from_be_bytes(arr).unsynch());
 				}
 			},
 			// We aren't able to determine a format

src/id3/v2/frame/header.rs

@@ -1,5 +1,6 @@
 use super::FrameFlags;
 use crate::error::{ID3v2Error, ID3v2ErrorKind, Result};
+use crate::id3::v2::util::synchsafe::SynchsafeInteger;
 use crate::id3::v2::util::upgrade::{upgrade_v2, upgrade_v3};
 use crate::id3::v2::FrameID;
 
@@ -88,7 +89,7 @@ where
 
 	// unsynch the frame size if necessary
 	if synchsafe {
-		size = crate::id3::v2::util::unsynch_u32(size);
+		size = size.unsynch();
 	}
 
 	let flags = u16::from_be_bytes([frame_header[8], frame_header[9]]);

src/id3/v2/frame/read.rs

@@ -28,7 +28,7 @@ impl<'a> Frame<'a> {
 		reader.read_exact(&mut content)?;
 
 		if flags.unsynchronisation {
-			content = crate::id3::v2::util::unsynch_content(content.as_slice())?;
+			content = crate::id3::v2::util::synchsafe::unsynch_content(content.as_slice())?;
 		}
 
 		#[cfg(feature = "id3v2_compression_support")]

src/id3/v2/mod.rs

@@ -18,7 +18,7 @@ pub(crate) mod write;
 
 use crate::error::{ID3v2Error, ID3v2ErrorKind, Result};
 use crate::macros::err;
-use util::unsynch_u32;
+use util::synchsafe::SynchsafeInteger;
 
 use std::io::Read;
 
@@ -106,14 +106,14 @@ where
 		restrictions: None, // Retrieved later if applicable
 	};
 
-	let size = unsynch_u32(BigEndian::read_u32(&header[6..]));
+	let size = BigEndian::read_u32(&header[6..]).unsynch();
 	let mut extended_size = 0;
 
 	let extended_header =
 		(version == ID3v2Version::V4 || version == ID3v2Version::V3) && flags & 0x40 == 0x40;
 
 	if extended_header {
-		extended_size = unsynch_u32(bytes.read_u32::<BigEndian>()?);
+		extended_size = bytes.read_u32::<BigEndian>()?.unsynch();
 
 		if extended_size < 6 {
 			return Err(ID3v2Error::new(ID3v2ErrorKind::BadExtendedHeaderSize).into());

src/id3/v2/read.rs

@@ -15,7 +15,7 @@ where
 
 	// Unsynchronize the entire tag
 	if header.flags.unsynchronisation {
-		tag_bytes = super::util::unsynch_content(&tag_bytes)?;
+		tag_bytes = super::util::synchsafe::unsynch_content(&tag_bytes)?;
 	}
 
 	let mut tag = ID3v2Tag::default();

src/id3/v2/util/mod.rs

@@ -1,92 +1,4 @@
 //! Utilities for working with ID3v2 tags
 
-pub(crate) mod upgrade;
-
-use crate::error::{ID3v2Error, ID3v2ErrorKind, Result};
-
-pub(in crate::id3::v2) fn unsynch_content(content: &[u8]) -> Result<Vec<u8>> {
-	let mut unsynch_content = Vec::new();
-
-	let mut discard = false;
-
-	let mut i = 0;
-	let mut next = 0;
-	let content_len = content.len();
-
-	// Check for (0xFF, 0x00, 0x00), replace with (0xFF, 0x00)
-	while i < content_len && next < content_len {
-		// Verify the next byte is less than 0xE0 (0b111xxxxx)
-		// Then remove the next byte if it is a zero
-		if discard {
-			if content[next] >= 0xE0 {
-				return Err(ID3v2Error::new(ID3v2ErrorKind::InvalidUnsynchronisation).into());
-			}
-
-			if content[next] == 0 {
-				discard = false;
-				next += 1;
-
-				continue;
-			}
-		}
-
-		discard = false;
-
-		unsynch_content.push(content[next]);
-
-		if content[next] == 0xFF {
-			discard = true
-		}
-
-		i += 1;
-		next += 1;
-	}
-
-	Ok(unsynch_content)
-}
-
-/// Create a synchsafe integer
-///
-/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
-///
-/// # Errors
-///
-/// `n` doesn't fit in 28 bits
-// https://github.com/polyfloyd/rust-id3/blob/e142ec656bf70a8153f6e5b34a37f26df144c3c1/src/stream/unsynch.rs#L9-L15
-pub fn synch_u32(n: u32) -> Result<u32> {
-	if n > 0x1000_0000 {
-		crate::macros::err!(TooMuchData);
-	}
-
-	let mut x: u32 = n & 0x7F | (n & 0xFFFF_FF80) << 1;
-	x = x & 0x7FFF | (x & 0xFFFF_8000) << 1;
-	x = x & 0x7F_FFFF | (x & 0xFF80_0000) << 1;
-	Ok(x)
-}
-
-/// Unsynchronise a synchsafe integer
-///
-/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
-// https://github.com/polyfloyd/rust-id3/blob/e142ec656bf70a8153f6e5b34a37f26df144c3c1/src/stream/unsynch.rs#L18-L20
-pub fn unsynch_u32(n: u32) -> u32 {
-	n & 0xFF | (n & 0xFF00) >> 1 | (n & 0xFF_0000) >> 2 | (n & 0xFF00_0000) >> 3
-}
-
-#[cfg(test)]
-mod tests {
-	#[test]
-	fn unsynchronisation() {
-		let valid_unsynch = vec![0xFF, 0x00, 0x00, 0xFF, 0x12, 0xB0, 0x05, 0xFF, 0x00, 0x00];
-
-		assert_eq!(
-			super::unsynch_content(valid_unsynch.as_slice()).unwrap(),
-			vec![0xFF, 0x00, 0xFF, 0x12, 0xB0, 0x05, 0xFF, 0x00]
-		);
-
-		let invalid_unsynch = vec![
-			0xFF, 0xE0, 0x00, 0xFF, 0x12, 0xB0, 0x05, 0xFF, 0x00, 0x50, 0x01,
-		];
-
-		assert!(super::unsynch_content(invalid_unsynch.as_slice()).is_err());
-	}
-}
+pub mod synchsafe;
+pub mod upgrade;

src/id3/v2/util/synchsafe.rs

@@ -0,0 +1,288 @@
+//! Utilities for working with unsynchronized ID3v2 content
+//!
+//! See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
+
+use crate::error::{ID3v2Error, ID3v2ErrorKind, Result};
+
+/// Unsynchronise a syncsafe buffer
+///
+/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
+///
+/// # Errors
+///
+/// The content is not properly unsynchronized
+pub fn unsynch_content(content: &[u8]) -> Result<Vec<u8>> {
+	let mut unsync_content = Vec::new();
+
+	let mut discard = false;
+
+	let mut i = 0;
+	let mut next = 0;
+	let content_len = content.len();
+
+	// Check for (0xFF, 0x00, 0x00), replace with (0xFF, 0x00)
+	while i < content_len && next < content_len {
+		// Verify the next byte is less than 0xE0 (0b111xxxxx)
+		// Then remove the next byte if it is a zero
+		if discard {
+			if content[next] >= 0xE0 {
+				return Err(ID3v2Error::new(ID3v2ErrorKind::InvalidUnsynchronisation).into());
+			}
+
+			if content[next] == 0 {
+				discard = false;
+				next += 1;
+
+				continue;
+			}
+		}
+
+		discard = false;
+
+		unsync_content.push(content[next]);
+
+		if content[next] == 0xFF {
+			discard = true
+		}
+
+		i += 1;
+		next += 1;
+	}
+
+	Ok(unsync_content)
+}
+
+/// An integer that can be converted to and from synchsafe variants
+pub trait SynchsafeInteger: Sized {
+	/// The integer type that this can be widened to for use in [`SynchsafeInteger::widening_synch`]
+	type WideningType;
+
+	/// Create a synchsafe integer
+	///
+	/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
+	///
+	/// # Errors
+	///
+	/// `self` doesn't fit in <`INTEGER_TYPE::BITS - size_of::<INTEGER_TYPE>()`> bits
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::id3::v2::util::synchsafe::SynchsafeInteger;
+	///
+	/// # fn main() -> lofty::error::Result<()> {
+	/// // Maximum value we can represent in a synchsafe u32
+	/// let unsynch_number = 0xFFF_FFFF_u32;
+	/// let synch_number = unsynch_number.synch()?;
+	///
+	/// // Our synchronized number should be something completely different
+	/// assert_ne!(synch_number, unsynch_number);
+	///
+	/// // Each byte should have 7 set bits and an MSB of 0
+	/// assert_eq!(synch_number, 0b01111111_01111111_01111111_01111111_u32);
+	/// # Ok(()) }
+	/// ```
+	fn synch(self) -> Result<Self>;
+
+	/// Create a synchsafe integer, widening to the next available integer type
+	///
+	/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::id3::v2::util::synchsafe::SynchsafeInteger;
+	///
+	/// // 0b11111111
+	/// let large_number = u8::MAX;
+	///
+	/// // Widened to a u16
+	/// // 0b00000001_01111111
+	/// let large_number_synchsafe = large_number.widening_synch();
+	///
+	/// // Unsynchronizing the number will get us back to 255
+	/// assert_eq!(large_number_synchsafe.unsynch(), large_number as u16);
+	/// ```
+	fn widening_synch(self) -> Self::WideningType;
+
+	/// Unsynchronise a synchsafe integer
+	///
+	/// See [`FrameFlags::unsynchronisation`](crate::id3::v2::FrameFlags::unsynchronisation) for an explanation.
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::id3::v2::util::synchsafe::SynchsafeInteger;
+	///
+	/// # fn main() -> lofty::error::Result<()> {
+	/// let unsynch_number = 0xFFF_FFFF_u32;
+	/// let synch_number = unsynch_number.synch()?;
+	///
+	/// // Our synchronized number should be something completely different
+	/// assert_ne!(synch_number, unsynch_number);
+	///
+	/// // Now, our re-unsynchronized number should match our original
+	/// let re_unsynch_number = synch_number.unsynch();
+	/// assert_eq!(re_unsynch_number, unsynch_number);
+	/// # Ok(()) }
+	/// ```
+	fn unsynch(self) -> Self;
+}
+
+macro_rules! impl_synchsafe {
+	(
+		$ty:ty, $widening_ty:ty,
+		synch($n:ident) $body:block;
+		widening_synch($w:ident) $widening_body:block;
+		unsynch($u:ident) $unsynch_body:block
+	) => {
+		#[allow(unused_parens)]
+		impl SynchsafeInteger for $ty {
+			type WideningType = $widening_ty;
+
+			fn synch(self) -> Result<Self> {
+				const MAXIMUM_INTEGER: $ty = {
+					let num_bytes = core::mem::size_of::<$ty>();
+					// 7 bits are available per byte, shave off 1 bit per byte
+					<$ty>::MAX >> num_bytes
+				};
+
+				if self > MAXIMUM_INTEGER {
+					crate::macros::err!(TooMuchData);
+				}
+
+				let $n = self;
+				Ok($body)
+			}
+
+			fn widening_synch(self) -> Self::WideningType {
+				let mut $w = <$widening_ty>::MIN;
+				let $n = self;
+				$widening_body;
+				$w
+			}
+
+			fn unsynch(self) -> Self {
+				let $u = self;
+				$unsynch_body
+			}
+		}
+	};
+}
+
+impl_synchsafe! {
+	u8, u16,
+	synch(n) {
+		(n & 0x7F)
+	};
+	widening_synch(w) {
+		w |= u16::from(n & 0x7F);
+		w |= u16::from(n & 0x80) << 1;
+	};
+	unsynch(u) {
+		(u & 0x7F)
+	}
+}
+
+impl_synchsafe! {
+	u16, u32,
+	synch(n) {
+		(n & 0x7F) |
+		((n & (0x7F << 7)) << 1)
+	};
+	widening_synch(w) {
+		w |= u32::from(n & 0x7F);
+		w |= u32::from((n & (0x7F << 7)) << 1);
+		w |= u32::from(n & (0x03 << 14)) << 2;
+	};
+	unsynch(u) {
+		((u & 0x7F00) >> 1) | (u & 0x7F)
+	}
+}
+
+impl_synchsafe! {
+	u32, u64,
+	synch(n) {
+		(n & 0x7F) |
+		((n & (0x7F << 7)) << 1) |
+		((n & (0x7F << 14)) << 2) |
+		((n & (0x7F << 21)) << 3)
+	};
+	widening_synch(w) {
+		w |= u64::from(n & 0x7F);
+		w |= u64::from(n & (0x7F << 7)) << 1;
+		w |= u64::from(n & (0x7F << 14)) << 2;
+		w |= u64::from(n & (0x7F << 21)) << 3;
+		w |= u64::from(n & (0x0F << 28)) << 4;
+	};
+	unsynch(u) {
+		((u & 0x7F00_0000) >> 3) | ((u & 0x7F_0000) >> 2) | ((u & 0x7F00) >> 1) | (u & 0x7F)
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	#[test]
+	fn unsynchronisation() {
+		let valid_unsync = vec![0xFF, 0x00, 0x00, 0xFF, 0x12, 0xB0, 0x05, 0xFF, 0x00, 0x00];
+
+		assert_eq!(
+			super::unsynch_content(valid_unsync.as_slice()).unwrap(),
+			vec![0xFF, 0x00, 0xFF, 0x12, 0xB0, 0x05, 0xFF, 0x00]
+		);
+
+		let invalid_unsync = vec![
+			0xFF, 0xE0, 0x00, 0xFF, 0x12, 0xB0, 0x05, 0xFF, 0x00, 0x50, 0x01,
+		];
+
+		assert!(super::unsynch_content(invalid_unsync.as_slice()).is_err());
+	}
+
+	use crate::id3::v2::util::synchsafe::SynchsafeInteger;
+	macro_rules! synchsafe_integer_tests {
+		(
+			$($int:ty => {
+				synch: $original:literal, $new:literal;
+				unsynch: $original_unsync:literal, $new_unsynch:literal;
+				widen: $original_widen:literal, $new_widen:literal;
+			});+
+		) => {
+			$(
+				paste::paste! {
+					#[test]
+					fn [<$int _synch>]() {
+						assert_eq!($original.synch().unwrap(), $new);
+					}
+
+					#[test]
+					fn [<$int _unsynch>]() {
+						assert_eq!($original_unsync.unsynch(), $new_unsynch);
+					}
+
+					#[test]
+					fn [<$int _widen>]() {
+						assert_eq!($original_widen.widening_synch(), $new_widen);
+					}
+				}
+			)+
+		};
+	}
+
+	synchsafe_integer_tests! {
+		u8 => {
+			synch:   0x7F_u8, 0x7F_u8;
+			unsynch: 0x7F_u8, 0x7F_u8;
+			widen:   0xFF_u8, 0x017F_u16;
+		};
+		u16 => {
+			synch:   0x3FFF_u16, 0x7F7F_u16;
+			unsynch: 0x7F7F_u16, 0x3FFF_u16;
+			widen:   0xFFFF_u16, 0x0003_7F7F_u32;
+		};
+		u32 => {
+			synch:   0xFFF_FFFF_u32, 0x7F7F_7F7F_u32;
+			unsynch: 0x7F7F_7F7F_u32, 0xFFF_FFFF_u32;
+			widen:   0xFFFF_FFFF_u32, 0x000F_7F7F_7F7F_u64;
+		}
+	}
+}

src/id3/v2/util/upgrade.rs

@@ -1,3 +1,5 @@
+//! Utilities for upgrading old ID3v2 frame IDs
+
 use std::collections::HashMap;
 
 macro_rules! gen_upgrades {

src/id3/v2/write/frame.rs

@@ -1,6 +1,6 @@
 use crate::error::{ID3v2Error, ID3v2ErrorKind, Result};
 use crate::id3::v2::frame::{FrameFlags, FrameRef, FrameValue};
-use crate::id3::v2::util::synch_u32;
+use crate::id3::v2::util::synchsafe::SynchsafeInteger;
 
 use std::io::Write;
 
@@ -99,7 +99,7 @@ where
 	if let Some(len) = flags.data_length_indicator {
 		if len > 0 {
 			write_frame_header(writer, name, (value.len() + 1) as u32, flags)?;
-			writer.write_u32::<BigEndian>(synch_u32(len)?)?;
+			writer.write_u32::<BigEndian>(len.synch()?)?;
 			writer.write_u8(method_symbol)?;
 			writer.write_all(value)?;
 
@@ -115,7 +115,7 @@ where
 	W: Write,
 {
 	writer.write_all(name.as_bytes())?;
-	writer.write_u32::<BigEndian>(synch_u32(len)?)?;
+	writer.write_u32::<BigEndian>(len.synch()?)?;
 	writer.write_u16::<BigEndian>(get_flags(flags))?;
 
 	Ok(())

src/id3/v2/write/mod.rs

@@ -7,7 +7,7 @@ use crate::file::FileType;
 use crate::id3::find_id3v2;
 use crate::id3::v2::frame::FrameRef;
 use crate::id3::v2::tag::Id3v2TagRef;
-use crate::id3::v2::util::synch_u32;
+use crate::id3::v2::util::synchsafe::SynchsafeInteger;
 use crate::id3::v2::ID3v2Tag;
 use crate::macros::err;
 use crate::probe::Probe;
@@ -117,7 +117,7 @@ pub(super) fn create_tag<'a, I: Iterator<Item = FrameRef<'a>> + 'a>(
 
 	// Go back to the start and write the final size
 	id3v2.seek(SeekFrom::Start(6))?;
-	id3v2.write_u32::<BigEndian>(synch_u32(extended_header_len + len as u32)?)?;
+	id3v2.write_u32::<BigEndian>((extended_header_len + len as u32).synch()?)?;
 
 	if needs_crc {
 		// The CRC is calculated on all the data between the header and footer
@@ -217,7 +217,7 @@ fn create_tag_header(flags: ID3v2TagFlags) -> Result<(Cursor<Vec<u8>>, u32)> {
 		header.seek(SeekFrom::Start(10))?;
 
 		// Seek back and write the actual values
-		header.write_u32::<BigEndian>(synch_u32(extended_header_size)?)?;
+		header.write_u32::<BigEndian>(extended_header_size.synch()?)?;
 		header.write_u8(1)?;
 		header.write_u8(ext_flags)?;