From 1474efa9a3f8d088ccf27c8626daf5f24c6b17aa Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Fri, 26 Apr 2024 14:10:14 -0400 Subject: [PATCH] Vorbis/Ape: Verify `FlagCompilation` item contents on merge --- CHANGELOG.md | 3 +++ lofty/src/ape/tag/mod.rs | 17 ++++++++++++++++- lofty/src/id3/v2/tag.rs | 9 +++++---- lofty/src/mp4/ilst/mod.rs | 11 ++--------- lofty/src/ogg/tag.rs | 14 ++++++++++++-- lofty/src/util/mod.rs | 8 ++++++++ 6 files changed, 46 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d80725f7..f6ce7c91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **VorbisComments**/**ApeTag**: Verify contents of `ItemKey::FlagCompilation` during `Tag` merge ([PR](https://github.com/Serial-ATA/lofty-rs/pull/387)) + ## [0.19.2] - 2024-04-26 ### Added diff --git a/lofty/src/ape/tag/mod.rs b/lofty/src/ape/tag/mod.rs index 934828d6..d25cab76 100644 --- a/lofty/src/ape/tag/mod.rs +++ b/lofty/src/ape/tag/mod.rs @@ -10,12 +10,13 @@ use crate::tag::item::ItemValueRef; use crate::tag::{ try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType, }; +use crate::util::flag_item; +use crate::util::io::{FileLike, Truncate}; use std::borrow::Cow; use std::io::Write; use std::ops::Deref; -use crate::util::io::{FileLike, Truncate}; use lofty_attr::tag; macro_rules! impl_accessor { @@ -163,6 +164,20 @@ impl ApeTag { ItemKey::TrackTotal => set_number(&item, |number| self.set_track_total(number)), ItemKey::DiscNumber => set_number(&item, |number| self.set_disk(number)), ItemKey::DiscTotal => set_number(&item, |number| self.set_disk_total(number)), + + // Normalize flag items + ItemKey::FlagCompilation => { + let Some(text) = item.item_value.text() else { + return; + }; + + let Some(flag) = flag_item(text) else { + return; + }; + + let value = u8::from(flag).to_string(); + self.insert(ApeItem::text("Compilation", value)); + }, _ => { if let Ok(item) = item.try_into() { self.insert(item); diff --git a/lofty/src/id3/v2/tag.rs b/lofty/src/id3/v2/tag.rs index 47e5f724..5f20d435 100644 --- a/lofty/src/id3/v2/tag.rs +++ b/lofty/src/id3/v2/tag.rs @@ -21,6 +21,7 @@ use crate::picture::{Picture, PictureType, TOMBSTONE_PICTURE}; use crate::tag::{ try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType, }; +use crate::util::flag_item; use crate::util::io::{FileLike, Length, Truncate}; use crate::util::text::{decode_text, TextDecodeOptions, TextEncoding}; @@ -1387,13 +1388,13 @@ impl MergeTag for SplitTagRemainder { // Flag items for item_key in [&ItemKey::FlagCompilation, &ItemKey::FlagPodcast] { - let Some(flag_value) = tag.take_strings(item_key).next() else { + let Some(text) = tag.take_strings(item_key).next() else { continue; }; - if flag_value != "1" && flag_value != "0" { + let Some(flag_value) = flag_item(&text) else { continue; - } + }; let frame_id = item_key .map_key(TagType::Id3v2, false) @@ -1401,7 +1402,7 @@ impl MergeTag for SplitTagRemainder { merged.frames.push(new_text_frame( FrameId::Valid(Cow::Borrowed(frame_id)), - flag_value, + u8::from(flag_value).to_string(), FrameFlags::default(), )); } diff --git a/lofty/src/mp4/ilst/mod.rs b/lofty/src/mp4/ilst/mod.rs index 682d0ab1..8690e38d 100644 --- a/lofty/src/mp4/ilst/mod.rs +++ b/lofty/src/mp4/ilst/mod.rs @@ -12,6 +12,7 @@ use crate::picture::{Picture, PictureType, TOMBSTONE_PICTURE}; use crate::tag::{ try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType, }; +use crate::util::flag_item; use crate::util::io::{FileLike, Length, Truncate}; use atom::{AdvisoryRating, Atom, AtomData}; @@ -705,18 +706,10 @@ impl MergeTag for SplitTagRemainder { ItemKey::DiscNumber => convert_to_uint(&mut discs.0, text.as_str()), ItemKey::DiscTotal => convert_to_uint(&mut discs.1, text.as_str()), ItemKey::FlagCompilation | ItemKey::FlagPodcast => { - let Ok(num) = text.as_str().parse::() else { + let Some(data) = flag_item(text.as_str()) else { continue; }; - let data = match num { - 0 => false, - 1 => true, - _ => { - // Ignore all other, unexpected values - continue; - }, - }; merged.atoms.push(Atom { ident: ident.into_owned(), data: AtomDataStorage::Single(AtomData::Bool(data)), diff --git a/lofty/src/ogg/tag.rs b/lofty/src/ogg/tag.rs index de27f7e9..ac3267c7 100644 --- a/lofty/src/ogg/tag.rs +++ b/lofty/src/ogg/tag.rs @@ -9,12 +9,13 @@ use crate::probe::Probe; use crate::tag::{ try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType, }; +use crate::util::flag_item; +use crate::util::io::{FileLike, Length, Truncate}; use std::borrow::Cow; use std::io::Write; use std::ops::Deref; -use crate::util::io::{FileLike, Length, Truncate}; use lofty_attr::tag; macro_rules! impl_accessor { @@ -575,10 +576,19 @@ impl MergeTag for SplitTagRemainder { let item_value = item.item_value; // Discard binary items, as they are not allowed in Vorbis comments - let (ItemValue::Text(val) | ItemValue::Locator(val)) = item_value else { + let (ItemValue::Text(mut val) | ItemValue::Locator(mut val)) = item_value else { continue; }; + // Normalize flag items + if item_key == ItemKey::FlagCompilation { + let Some(flag) = flag_item(&val) else { + continue; + }; + + val = u8::from(flag).to_string(); + } + let key; match item_key { ItemKey::Unknown(unknown) => { diff --git a/lofty/src/util/mod.rs b/lofty/src/util/mod.rs index 351aa415..99aeb906 100644 --- a/lofty/src/util/mod.rs +++ b/lofty/src/util/mod.rs @@ -2,3 +2,11 @@ pub(crate) mod alloc; pub mod io; pub(crate) mod math; pub(crate) mod text; + +pub(crate) fn flag_item(item: &str) -> Option { + match item { + "1" | "true" => Some(true), + "0" | "false" => Some(false), + _ => None, + } +}