Strip trailing null characters from decoded text

uklotzde · Serial-ATA · commit 7398602e7854 · 2023-10-25T11:52:31.000-04:00
diff --git a/src/ape/tag/read.rs b/src/ape/tag/read.rs
@@ -5,6 +5,7 @@ use crate::ape::header::{self, ApeHeader};
 use crate::error::Result;
 use crate::macros::{decode_err, err, try_vec};
 use crate::tag::item::ItemValue;
+use crate::util::text::utf8_decode;
 
 use std::io::{Read, Seek, SeekFrom};
 
@@ -38,7 +39,7 @@ where
 			key_char = data.read_u8()?;
 		}
 
-		let key = String::from_utf8(key)
+		let key = utf8_decode(key)
 			.map_err(|_| decode_err!(Ape, "APE tag item contains a non UTF-8 key"))?;
 
 		if INVALID_KEYS.contains(&&*key.to_uppercase()) {
@@ -57,11 +58,11 @@ where
 		data.read_exact(&mut value)?;
 
 		let parsed_value = match item_type {
-			0 => ItemValue::Text(String::from_utf8(value).map_err(|_| {
+			0 => ItemValue::Text(utf8_decode(value).map_err(|_| {
 				decode_err!(Ape, "Failed to convert text item into a UTF-8 string")
 			})?),
 			1 => ItemValue::Binary(value),
-			2 => ItemValue::Locator(String::from_utf8(value).map_err(|_| {
+			2 => ItemValue::Locator(utf8_decode(value).map_err(|_| {
 				decode_err!(Ape, "Failed to convert locator item into a UTF-8 string")
 			})?),
 			_ => decode_err!(@BAIL Ape, "APE tag item contains an invalid item type"),
diff --git a/src/error.rs b/src/error.rs
@@ -388,7 +388,7 @@ impl Display for FileEncodingError {
 
 /// Errors that could occur within Lofty
 pub struct LoftyError {
-	kind: ErrorKind,
+	pub(crate) kind: ErrorKind,
 }
 
 impl LoftyError {
diff --git a/src/id3/v2/items/extended_text_frame.rs b/src/id3/v2/items/extended_text_frame.rs
@@ -2,7 +2,7 @@ use crate::error::{Id3v2Error, Id3v2ErrorKind, LoftyError, Result};
 use crate::id3::v2::frame::content::verify_encoding;
 use crate::id3::v2::header::Id3v2Version;
 use crate::util::text::{
-	decode_text, encode_text, read_to_terminator, trim_end_nulls, utf16_decode, TextEncoding,
+	decode_text, encode_text, read_to_terminator, utf16_decode_bytes, TextEncoding,
 };
 
 use std::hash::{Hash, Hasher};
@@ -61,10 +61,9 @@ impl ExtendedTextFrame {
 		let encoding = verify_encoding(encoding_byte, version)?;
 		let description = decode_text(reader, encoding, true)?;
 
-		let mut frame_content;
+		let frame_content;
 		if encoding != TextEncoding::UTF16 {
 			frame_content = decode_text(reader, encoding, false)?.content;
-			trim_end_nulls(&mut frame_content);
 
 			return Ok(Some(ExtendedTextFrame {
 				encoding,
@@ -95,7 +94,7 @@ impl ExtendedTextFrame {
 				_ => unreachable!(),
 			};
 
-			frame_content = utf16_decode(&raw_text, endianness).map_err(|_| {
+			frame_content = utf16_decode_bytes(&raw_text, endianness).map_err(|_| {
 				Into::<LoftyError>::into(Id3v2Error::new(Id3v2ErrorKind::BadSyncText))
 			})?;
 		}
diff --git a/src/id3/v2/items/extended_url_frame.rs b/src/id3/v2/items/extended_url_frame.rs
@@ -1,7 +1,7 @@
 use crate::error::Result;
 use crate::id3::v2::frame::content::verify_encoding;
 use crate::id3::v2::header::Id3v2Version;
-use crate::util::text::{decode_text, encode_text, trim_end_nulls, TextEncoding};
+use crate::util::text::{decode_text, encode_text, TextEncoding};
 
 use std::hash::{Hash, Hasher};
 use std::io::Read;
@@ -58,8 +58,7 @@ impl ExtendedUrlFrame {
 
 		let encoding = verify_encoding(encoding_byte, version)?;
 		let description = decode_text(reader, encoding, true)?.content;
-		let mut content = decode_text(reader, TextEncoding::Latin1, false)?.content;
-		trim_end_nulls(&mut content);
+		let content = decode_text(reader, TextEncoding::Latin1, false)?.content;
 
 		Ok(Some(ExtendedUrlFrame {
 			encoding,
diff --git a/src/id3/v2/items/language_frame.rs b/src/id3/v2/items/language_frame.rs
@@ -1,7 +1,7 @@
 use crate::error::{Id3v2Error, Id3v2ErrorKind, Result};
 use crate::id3::v2::frame::content::verify_encoding;
 use crate::id3::v2::header::Id3v2Version;
-use crate::util::text::{decode_text, encode_text, trim_end_nulls, TextEncoding};
+use crate::util::text::{decode_text, encode_text, TextEncoding};
 
 use std::hash::{Hash, Hasher};
 use std::io::Read;
@@ -33,8 +33,7 @@ impl LanguageFrame {
 		reader.read_exact(&mut language)?;
 
 		let description = decode_text(reader, encoding, true)?.content;
-		let mut content = decode_text(reader, encoding, false)?.content;
-		trim_end_nulls(&mut content);
+		let content = decode_text(reader, encoding, false)?.content;
 
 		Ok(Some(Self {
 			encoding,
diff --git a/src/id3/v2/items/ownership_frame.rs b/src/id3/v2/items/ownership_frame.rs
@@ -1,5 +1,5 @@
 use crate::error::{ErrorKind, Id3v2Error, Id3v2ErrorKind, LoftyError, Result};
-use crate::util::text::{decode_text, encode_text, TextEncoding};
+use crate::util::text::{decode_text, encode_text, utf8_decode, TextEncoding};
 
 use std::hash::Hash;
 use std::io::Read;
@@ -50,7 +50,7 @@ impl OwnershipFrame {
 		let mut date_bytes = vec![0u8; 8];
 		reader.read_exact(&mut date_bytes)?;
 
-		let date_of_purchase = String::from_utf8(date_bytes)?;
+		let date_of_purchase = utf8_decode(date_bytes)?;
 
 		let seller = decode_text(reader, encoding, false)?.content;
 
diff --git a/src/id3/v2/items/sync_text.rs b/src/id3/v2/items/sync_text.rs
@@ -1,6 +1,8 @@
 use crate::error::{ErrorKind, Id3v2Error, Id3v2ErrorKind, LoftyError, Result};
 use crate::macros::err;
-use crate::util::text::{decode_text, encode_text, read_to_terminator, utf16_decode, TextEncoding};
+use crate::util::text::{
+	decode_text, encode_text, read_to_terminator, utf16_decode_bytes, TextEncoding,
+};
 
 use std::io::{Cursor, Read, Seek, SeekFrom, Write};
 
@@ -144,7 +146,7 @@ impl SynchronizedText {
 							// text + null terminator
 							pos += (raw_text.len() + 2) as u64;
 
-							return utf16_decode(&raw_text, endianness)
+							return utf16_decode_bytes(&raw_text, endianness)
 								.map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadSyncText).into());
 						}
 
diff --git a/src/id3/v2/items/text_information_frame.rs b/src/id3/v2/items/text_information_frame.rs
@@ -1,7 +1,7 @@
 use crate::error::Result;
 use crate::id3::v2::frame::content::verify_encoding;
 use crate::id3::v2::header::Id3v2Version;
-use crate::util::text::{decode_text, encode_text, trim_end_nulls, TextEncoding};
+use crate::util::text::{decode_text, encode_text, TextEncoding};
 
 use byteorder::ReadBytesExt;
 
@@ -37,8 +37,7 @@ impl TextInformationFrame {
 		};
 
 		let encoding = verify_encoding(encoding_byte, version)?;
-		let mut value = decode_text(reader, encoding, false)?.content;
-		trim_end_nulls(&mut value);
+		let value = decode_text(reader, encoding, false)?.content;
 
 		Ok(Some(TextInformationFrame { encoding, value }))
 	}
diff --git a/src/iff/aiff/properties.rs b/src/iff/aiff/properties.rs
@@ -2,6 +2,7 @@ use super::read::CompressionPresent;
 use crate::error::Result;
 use crate::macros::{decode_err, try_vec};
 use crate::properties::FileProperties;
+use crate::util::text::utf8_decode;
 
 use std::borrow::Cow;
 use std::io::Read;
@@ -234,7 +235,7 @@ pub(super) fn read_properties(
 					let mut compression_name_bytes = try_vec![0u8; compression_name_size as usize];
 					comm.read_exact(&mut compression_name_bytes)?;
 
-					compression_name = String::from_utf8(compression_name_bytes)?;
+					compression_name = utf8_decode(compression_name_bytes)?;
 				}
 
 				AiffCompressionType::Other {
diff --git a/src/iff/chunk.rs b/src/iff/chunk.rs
@@ -2,6 +2,7 @@ use crate::error::Result;
 use crate::id3::v2::tag::Id3v2Tag;
 use crate::macros::{err, try_vec};
 use crate::probe::ParsingMode;
+use crate::util::text::utf8_decode;
 
 use std::io::{Read, Seek, SeekFrom};
 use std::marker::PhantomData;
@@ -67,7 +68,7 @@ impl<B: ByteOrder> Chunks<B> {
 			data.seek(SeekFrom::Current(1))?;
 		}
 
-		Ok(String::from_utf8(cont)?)
+		utf8_decode(cont)
 	}
 
 	pub fn content<R>(&mut self, data: &mut R) -> Result<Vec<u8>>
diff --git a/src/mp4/ilst/read.rs b/src/mp4/ilst/read.rs
@@ -9,7 +9,7 @@ use crate::mp4::atom_info::AtomInfo;
 use crate::mp4::ilst::atom::AtomDataStorage;
 use crate::mp4::read::{skip_unneeded, AtomReader};
 use crate::picture::{MimeType, Picture, PictureType};
-use crate::util::text::utf16_decode;
+use crate::util::text::{utf16_decode_bytes, utf8_decode};
 use crate::ParsingMode;
 
 use std::borrow::Cow;
@@ -317,8 +317,8 @@ where
 fn interpret_atom_content(flags: u32, content: Vec<u8>) -> Result<AtomData> {
 	// https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/Metadata/Metadata.html#//apple_ref/doc/uid/TP40000939-CH1-SW35
 	Ok(match flags {
-		UTF8 => AtomData::UTF8(String::from_utf8(content)?),
-		UTF16 => AtomData::UTF16(utf16_decode(&content, u16::from_be_bytes)?),
+		UTF8 => AtomData::UTF8(utf8_decode(content)?),
+		UTF16 => AtomData::UTF16(utf16_decode_bytes(&content, u16::from_be_bytes)?),
 		BE_SIGNED_INTEGER => AtomData::SignedInteger(parse_int(&content)?),
 		BE_UNSIGNED_INTEGER => AtomData::UnsignedInteger(parse_uint(&content)?),
 		code => AtomData::Unknown {
diff --git a/src/ogg/read.rs b/src/ogg/read.rs
@@ -4,6 +4,7 @@ use crate::error::{ErrorKind, LoftyError, Result};
 use crate::macros::{decode_err, err, parse_mode_choice};
 use crate::picture::{MimeType, Picture, PictureInformation, PictureType};
 use crate::probe::ParsingMode;
+use crate::util::text::{utf16_decode, utf8_decode};
 
 use std::borrow::Cow;
 use std::io::{Read, Seek, SeekFrom};
@@ -34,25 +35,31 @@ where
 
 	len -= u64::from(vendor_len);
 
-	let vendor = match String::from_utf8(vendor) {
+	let vendor = match utf8_decode(vendor) {
 		Ok(v) => v,
 		Err(e) => {
 			// The actions following this are not spec-compliant in the slightest, so
 			// we need to short circuit if strict.
 			if parse_mode == ParsingMode::Strict {
-				return Err(LoftyError::new(ErrorKind::StringFromUtf8(e)));
+				return Err(e);
 			}
 
 			// Some vendor strings have invalid mixed UTF-8 and UTF-16 encodings.
 			// This seems to work, while preserving the string opposed to using
 			// the replacement character
+			let LoftyError {
+				kind: ErrorKind::StringFromUtf8(e),
+			} = e
+			else {
+				return Err(e);
+			};
 			let s = e
 				.as_bytes()
 				.iter()
 				.map(|c| u16::from(*c))
 				.collect::<Vec<_>>();
 
-			match String::from_utf16(&s) {
+			match utf16_decode(&s) {
 				Ok(vendor) => vendor,
 				Err(_) => decode_err!(@BAIL "OGG: File has an invalid vendor string"),
 			}
@@ -142,14 +149,14 @@ where
 				// SAFETY: We just verified that all of the bytes fall within the subset of ASCII
 				let key = unsafe { String::from_utf8_unchecked(k.to_vec()) };
 
-				match String::from_utf8(value.to_vec()) {
+				match utf8_decode(value.to_vec()) {
 					Ok(value) => tag.items.push((key, value)),
 					Err(e) => {
 						if parse_mode == ParsingMode::Strict {
-							return Err(LoftyError::new(ErrorKind::StringFromUtf8(e)));
+							return Err(e);
 						}
 
-						log::warn!("Non UTF-8 value found, discarding field");
+						log::warn!("Non UTF-8 value found, discarding field {key:?}");
 						continue;
 					},
 				}
diff --git a/src/picture.rs b/src/picture.rs
@@ -1,6 +1,7 @@
 use crate::error::{ErrorKind, LoftyError, Result};
 use crate::macros::err;
 use crate::probe::ParsingMode;
+use crate::util::text::utf8_decode;
 
 use std::borrow::Cow;
 use std::fmt::{Debug, Formatter};
@@ -683,8 +684,8 @@ impl Picture {
 		if desc_len > 0 && desc_len < size {
 			let pos = 12 + mime_len;
 
-			if let Ok(desc) = std::str::from_utf8(&content[pos..pos + desc_len]) {
-				description = Some(Cow::from(desc.to_string()));
+			if let Ok(desc) = utf8_decode(content[pos..pos + desc_len].to_vec()) {
+				description = Some(desc.into());
 			}
 
 			size -= desc_len;
diff --git a/src/util/text.rs b/src/util/text.rs

Original file line number	Diff line number	Diff line change
`@@ -388,7 +388,7 @@ impl Display for FileEncodingError {`
`388`	`388`
`389`	`389`	`/// Errors that could occur within Lofty`
`390`	`390`	`pub struct LoftyError {`
`391`		`- kind: ErrorKind,`
	`391`	`+ pub(crate) kind: ErrorKind,`
`392`	`392`	`}`
`393`	`393`
`394`	`394`	`impl LoftyError {`
Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ use crate::error::Result;`
`2`	`2`	`use crate::id3::v2::tag::Id3v2Tag;`
`3`	`3`	`use crate::macros::{err, try_vec};`
`4`	`4`	`use crate::probe::ParsingMode;`
	`5`	`+use crate::util::text::utf8_decode;`
`5`	`6`
`6`	`7`	`use std::io::{Read, Seek, SeekFrom};`
`7`	`8`	`use std::marker::PhantomData;`
`@@ -67,7 +68,7 @@ impl<B: ByteOrder> Chunks<B> {`
`67`	`68`	`data.seek(SeekFrom::Current(1))?;`
`68`	`69`	`}`
`69`	`70`
`70`		`- Ok(String::from_utf8(cont)?)`
	`71`	`+ utf8_decode(cont)`
`71`	`72`	`}`
`72`	`73`
`73`	`74`	`pub fn content<R>(&mut self, data: &mut R) -> Result<Vec<u8>>`