diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c8a3eb9a..e51cdf9b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **Properties**: Expose channel mask (only supported for WAV and MPEG for now) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/155)) - **ItemKey**: `InitialKey` mapping for Vorbis Comments ([PR](https://github.com/Serial-ATA/lofty-rs/pull/156)) +- **VorbisComments**: `VorbisComments::push` to allow for a non-replacing insertion ### Changed - **APE**/**ID3v1**/**ID3v2**/**Tag**: - Allow empty strings as values instead of removing the corresponding item when empty ([PR](https://github.com/Serial-ATA/lofty-rs/pull/134)) - Separated the trait `SplitAndMergeTag` into `SplitTag` and `MergeTag` to prevent any unexpected or undefined behavior at runtime ([#143](https://github.com/Serial-ATA/lofty-rs/pull/143)) +- **VorbisComments**: + - Keys will now be verified according to spec before insertion + - Getters will now case-insensitively search for keys + - `TRACKNUM` will now be considered in the `Accessor::*track` methods ### Fixed - **ID3v2**: diff --git a/src/ogg/tag.rs b/src/ogg/tag.rs index 0d168ddd8..e0550ec26 100644 --- a/src/ogg/tag.rs +++ b/src/ogg/tag.rs @@ -26,7 +26,7 @@ macro_rules! impl_accessor { } fn [](&mut self, value: String) { - self.insert(String::from($key), value, true) + self.insert(String::from($key), value) } fn [](&mut self) { @@ -78,19 +78,19 @@ impl VorbisComments { } /// Gets an item by key - /// - /// NOTE: This is case-sensitive pub fn get(&self, key: &str) -> Option<&str> { + if !verify_key(key) { + return None; + } + self.items .iter() - .find(|(k, _)| k == key) + .find(|(k, _)| k.eq_ignore_ascii_case(key)) .map(|(_, v)| v.as_str()) } /// Gets all items with the key /// - /// NOTE: This is case-sensitive - /// /// # Examples /// /// ```rust @@ -99,9 +99,9 @@ impl VorbisComments { /// let mut vorbis_comments = VorbisComments::default(); /// /// // Vorbis comments allows multiple fields with the same key, such as artist - /// vorbis_comments.insert(String::from("ARTIST"), String::from("Foo artist"), false); - /// vorbis_comments.insert(String::from("ARTIST"), String::from("Bar artist"), false); - /// vorbis_comments.insert(String::from("ARTIST"), String::from("Baz artist"), false); + /// vorbis_comments.push(String::from("ARTIST"), String::from("Foo artist")); + /// vorbis_comments.push(String::from("ARTIST"), String::from("Bar artist")); + /// vorbis_comments.push(String::from("ARTIST"), String::from("Baz artist")); /// /// let all_artists = vorbis_comments.get_all("ARTIST").collect::>(); /// assert_eq!(all_artists, vec!["Foo artist", "Bar artist", "Baz artist"]); @@ -109,29 +109,67 @@ impl VorbisComments { pub fn get_all<'a>(&'a self, key: &'a str) -> impl Iterator + Clone + '_ { self.items .iter() - .filter_map(move |(k, v)| (k == key).then_some(v.as_str())) + .filter_map(move |(k, v)| (k.eq_ignore_ascii_case(key)).then_some(v.as_str())) } /// Inserts an item /// - /// If `replace_all` is true, it will remove all items with the key before insertion - pub fn insert(&mut self, key: String, value: String, replace_all: bool) { - if replace_all { - self.items.retain(|(k, _)| k != &key); + /// This is the same as [`VorbisComments::push`], except it will remove any items with the same key. + /// + /// NOTE: This will do nothing if the key is invalid. This specification is available [here](https://xiph.org/vorbis/doc/v-comment.html#vectorformat). + /// + /// ```rust + /// use lofty::ogg::VorbisComments; + /// + /// let mut tag = VorbisComments::default(); + /// tag.insert(String::from("TITLE"), String::from("Title 1")); + /// tag.insert(String::from("TITLE"), String::from("Title 2")); + /// + /// // We only retain the last title inserted + /// let mut titles = tag.get_all("TITLE"); + /// assert_eq!(titles.next(), Some("Title 2")); + /// assert_eq!(titles.next(), None); + /// ``` + pub fn insert(&mut self, key: String, value: String) { + if !verify_key(&key) { + return; } + self.items.retain(|(k, _)| !k.eq_ignore_ascii_case(&key)); self.items.push((key, value)) } - /// Removes all items with a key, returning an iterator + /// Appends an item + /// + /// NOTE: This will do nothing if the key is invalid. This specification is available [here](https://xiph.org/vorbis/doc/v-comment.html#vectorformat). + /// + /// ```rust + /// use lofty::ogg::VorbisComments; + /// + /// let mut tag = VorbisComments::default(); + /// tag.push(String::from("TITLE"), String::from("Title 1")); + /// tag.push(String::from("TITLE"), String::from("Title 2")); /// - /// NOTE: This is case-sensitive + /// // We retain both titles + /// let mut titles = tag.get_all("TITLE"); + /// assert_eq!(titles.next(), Some("Title 1")); + /// assert_eq!(titles.next(), Some("Title 2")); + /// ``` + pub fn push(&mut self, key: String, value: String) { + if !verify_key(&key) { + return; + } + + self.items.push((key, value)) + } + + /// Removes all items with a key, returning an iterator pub fn remove(&mut self, key: &str) -> impl Iterator + '_ { // TODO: drain_filter let mut split_idx = 0_usize; for read_idx in 0..self.items.len() { - if self.items[read_idx].0 == key { + if self.items[read_idx].0.eq_ignore_ascii_case(key) { self.items.swap(split_idx, read_idx); split_idx += 1; } @@ -141,6 +179,17 @@ impl VorbisComments { } } +// A case-insensitive field name that may consist of ASCII 0x20 through 0x7D, 0x3D ('=') excluded. +// ASCII 0x41 through 0x5A inclusive (A-Z) is to be considered equivalent to ASCII 0x61 through 0x7A inclusive (a-z). +fn verify_key(key: &str) -> bool { + if key.is_empty() { + return false; + } + + key.bytes() + .all(|byte| (0x20..=0x7D).contains(&byte) && byte != 0x3D) +} + impl OggPictureStorage for VorbisComments { fn pictures(&self) -> &[(Picture, PictureInformation)] { &self.pictures @@ -157,7 +206,10 @@ impl Accessor for VorbisComments { ); fn track(&self) -> Option { - if let Some(item) = self.get("TRACKNUMBER") { + if let Some(item) = self + .get("TRACKNUMBER") + .map_or_else(|| self.get("TRACKNUM"), Some) + { return item.parse::().ok(); } @@ -165,11 +217,13 @@ impl Accessor for VorbisComments { } fn set_track(&mut self, value: u32) { - self.insert(String::from("TRACKNUMBER"), value.to_string(), true); + self.remove_track(); + self.insert(String::from("TRACKNUMBER"), value.to_string()); } fn remove_track(&mut self) { let _ = self.remove("TRACKNUMBER"); + let _ = self.remove("TRACKNUM"); } fn track_total(&self) -> Option { @@ -184,7 +238,7 @@ impl Accessor for VorbisComments { } fn set_track_total(&mut self, value: u32) { - self.insert(String::from("TRACKTOTAL"), value.to_string(), true); + self.insert(String::from("TRACKTOTAL"), value.to_string()); let _ = self.remove("TOTALTRACKS"); } @@ -202,7 +256,7 @@ impl Accessor for VorbisComments { } fn set_disk(&mut self, value: u32) { - self.insert(String::from("DISCNUMBER"), value.to_string(), true); + self.insert(String::from("DISCNUMBER"), value.to_string()); } fn remove_disk(&mut self) { @@ -221,7 +275,7 @@ impl Accessor for VorbisComments { } fn set_disk_total(&mut self, value: u32) { - self.insert(String::from("DISCTOTAL"), value.to_string(), true); + self.insert(String::from("DISCTOTAL"), value.to_string()); let _ = self.remove("TOTALDISCS"); } @@ -241,7 +295,7 @@ impl Accessor for VorbisComments { fn set_year(&mut self, value: u32) { // DATE is the preferred way of storing the year, but it is still possible we will // encounter YEAR - self.insert(String::from("DATE"), value.to_string(), true); + self.insert(String::from("DATE"), value.to_string()); let _ = self.remove("YEAR"); } @@ -524,13 +578,13 @@ mod tests { expected_tag.set_vendor(String::from("Lavf58.76.100")); - expected_tag.insert(String::from("ALBUM"), String::from("Baz album"), false); - expected_tag.insert(String::from("ARTIST"), String::from("Bar artist"), false); - expected_tag.insert(String::from("COMMENT"), String::from("Qux comment"), false); - expected_tag.insert(String::from("DATE"), String::from("1984"), false); - expected_tag.insert(String::from("GENRE"), String::from("Classical"), false); - expected_tag.insert(String::from("TITLE"), String::from("Foo title"), false); - expected_tag.insert(String::from("TRACKNUMBER"), String::from("1"), false); + expected_tag.push(String::from("ALBUM"), String::from("Baz album")); + expected_tag.push(String::from("ARTIST"), String::from("Bar artist")); + expected_tag.push(String::from("COMMENT"), String::from("Qux comment")); + expected_tag.push(String::from("DATE"), String::from("1984")); + expected_tag.push(String::from("GENRE"), String::from("Classical")); + expected_tag.push(String::from("TITLE"), String::from("Foo title")); + expected_tag.push(String::from("TRACKNUMBER"), String::from("1")); let file_cont = crate::tag::utils::test_utils::read_path("tests/tags/assets/test.vorbis"); let parsed_tag = read_tag(&file_cont);