Mirrors/lofty-rs
2
0
Fork 0
mirror of https://github.com/Serial-ATA/lofty-rs synced 2025-03-04 06:47:12 +00:00
Code Issues Projects Releases Packages Wiki Activity

ID3v2: Disallow 4 character TXXX descriptions as ItemKey

Browse source
This commit is contained in:
Serial 2024-05-03 11:48:51 -04:00 • committed by Alex
parent 833e34e03a
commit 32fb134e67
5 changed files with 58 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -49,6 +49,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Renamed `Popularimeter` -> `PopularimeterFrame`
- Renamed `SynchronizedText` -> `SynchronizedTextFrame`
### Fixed
- **ID3v2**: Disallow 4 character TXXX/WXXX frame descriptions from being converted to `ItemKey` ([issue](https://github.com/Serial-ATA/lofty-rs/issues/309)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/394))
## [0.19.2] - 2024-04-26
### Added

101
lofty/src/id3/v2/frame/conversion.rs
View file

@ -6,10 +6,9 @@ use crate::id3::v2::tag::{
};
use crate::id3::v2::{
ExtendedTextFrame, ExtendedUrlFrame, Frame, FrameFlags, FrameId, PopularimeterFrame,
UniqueFileIdentifierFrame, UnsynchronizedTextFrame,
UniqueFileIdentifierFrame,
};
use crate::macros::err;
use crate::tag::items::UNKNOWN_LANGUAGE;
use crate::tag::{ItemKey, ItemValue, TagItem, TagType};
use crate::TextEncoding;
@ -32,73 +31,41 @@ fn frame_from_unknown_item(id: FrameId<'_>, item_value: ItemValue) -> Result<Fra
impl From<TagItem> for Option<Frame<'static>> {
fn from(input: TagItem) -> Self {
let value;
match input.key().try_into().map(FrameId::into_owned) {
Ok(id) => {
match (&id, input.item_value) {
(FrameId::Valid(ref s), ItemValue::Text(text)) if s == "COMM" => {
value = new_comment_frame(text);
},
(FrameId::Valid(ref s), ItemValue::Text(text)) if s == "USLT" => {
value = Frame::UnsynchronizedText(UnsynchronizedTextFrame::new(
TextEncoding::UTF8,
UNKNOWN_LANGUAGE,
EMPTY_CONTENT_DESCRIPTOR,
text,
));
},
(FrameId::Valid(ref s), ItemValue::Locator(text) | ItemValue::Text(text))
if s == "WXXX" =>
{
value = Frame::UserUrl(ExtendedUrlFrame::new(
TextEncoding::UTF8,
EMPTY_CONTENT_DESCRIPTOR,
text,
));
},
(FrameId::Valid(ref s), ItemValue::Text(text)) if s == "TXXX" => {
value = new_user_text_frame(EMPTY_CONTENT_DESCRIPTOR, text);
},
(FrameId::Valid(ref s), ItemValue::Binary(text)) if s == "POPM" => {
value = Frame::Popularimeter(
PopularimeterFrame::parse(&mut &text[..], FrameFlags::default())
.ok()?,
);
},
(_, item_value) => value = frame_from_unknown_item(id, item_value).ok()?,
};
},
Err(_) => match input.item_key.map_key(TagType::Id3v2, true) {
Some(desc) => match input.item_value {
ItemValue::Text(text) => {
value = Frame::UserText(ExtendedTextFrame::new(
TextEncoding::UTF8,
String::from(desc),
text,
))
},
ItemValue::Locator(locator) => {
value = Frame::UserUrl(ExtendedUrlFrame::new(
TextEncoding::UTF8,
String::from(desc),
locator,
))
},
ItemValue::Binary(_) => return None,
if let Ok(id) = input.key().try_into().map(FrameId::into_owned) {
return frame_from_unknown_item(id, input.item_value).ok();
}
match input.item_key.map_key(TagType::Id3v2, true) {
Some(desc) => match input.item_value {
ItemValue::Text(text) => {
value = Frame::UserText(ExtendedTextFrame::new(
TextEncoding::UTF8,
String::from(desc),
text,
))
},
None => match (input.item_key, input.item_value) {
(ItemKey::MusicBrainzRecordingId, ItemValue::Text(recording_id)) => {
if !recording_id.is_ascii() {
return None;
}
let frame = UniqueFileIdentifierFrame::new(
MUSICBRAINZ_UFID_OWNER.to_owned(),
recording_id.into_bytes(),
);
value = Frame::UniqueFileIdentifier(frame);
},
_ => {
ItemValue::Locator(locator) => {
value = Frame::UserUrl(ExtendedUrlFrame::new(
TextEncoding::UTF8,
String::from(desc),
locator,
))
},
ItemValue::Binary(_) => return None,
},
None => match (input.item_key, input.item_value) {
(ItemKey::MusicBrainzRecordingId, ItemValue::Text(recording_id)) => {
if !recording_id.is_ascii() {
return None;
},
}
let frame = UniqueFileIdentifierFrame::new(
MUSICBRAINZ_UFID_OWNER.to_owned(),
recording_id.into_bytes(),
);
value = Frame::UniqueFileIdentifier(frame);
},
_ => {
return None;
},
},
}

8
lofty/src/id3/v2/tag.rs
View file

@ -1081,12 +1081,16 @@ fn handle_tag_split(tag: &mut Tag, frame: &mut Frame<'_>) -> bool {
!key_value_pairs.is_empty() // Frame is consumed if we consumed all items
},
// TODO: HACK!! We are specifically disallowing descriptions with a length of 4.
// This is due to use storing 4 character IDs as Frame::Text on tag merge.
// Maybe ItemKey could use a "TXXX:" prefix eventually, so we would store
// "TXXX:MusicBrainz Album Id" instead of "MusicBrainz Album Id".
// Store TXXX/WXXX frames by their descriptions, rather than their IDs
Frame::UserText(ExtendedTextFrame {
ref description,
ref content,
..
}) if !description.is_empty() => {
}) if !description.is_empty() && description.len() != 4 => {
let item_key = ItemKey::from_key(TagType::Id3v2, description);
for c in content.split(V4_MULTI_VALUE_SEPARATOR) {
tag.items.push(TagItem::new(
@ -1101,7 +1105,7 @@ fn handle_tag_split(tag: &mut Tag, frame: &mut Frame<'_>) -> bool {
ref description,
ref content,
..
}) if !description.is_empty() => {
}) if !description.is_empty() && description.len() != 4 => {
let item_key = ItemKey::from_key(TagType::Id3v2, description);
for c in content.split(V4_MULTI_VALUE_SEPARATOR) {
tag.items.push(TagItem::new(

45
lofty/src/id3/v2/tag/tests.rs
View file

@ -108,37 +108,6 @@ fn id3v2_to_tag() {
crate::tag::utils::test_utils::verify_tag(&tag, true, true);
}
#[test]
fn tag_to_id3v2_popm() {
let mut tag = Tag::new(TagType::Id3v2);
tag.insert(TagItem::new(
ItemKey::Popularimeter,
ItemValue::Binary(vec![
b'f', b'o', b'o', b'@', b'b', b'a', b'r', b'.', b'c', b'o', b'm', 0, 196, 0, 0, 255,
255,
]),
));
let expected = PopularimeterFrame::new(String::from("foo@bar.com"), 196, 65535);
let converted_tag: Id3v2Tag = tag.into();
assert_eq!(converted_tag.frames.len(), 1);
let actual_frame = converted_tag.frames.first().unwrap();
assert_eq!(actual_frame.id(), &FrameId::Valid(Cow::Borrowed("POPM")));
// Note: as POPM frames are considered equal by email alone, each field must
// be separately validated
match actual_frame {
Frame::Popularimeter(pop) => {
assert_eq!(pop.email, expected.email);
assert_eq!(pop.rating, expected.rating);
assert_eq!(pop.counter, expected.counter);
},
_ => unreachable!(),
}
}
#[test]
fn fail_write_bad_frame() {
let mut tag = Id3v2Tag::default();
@ -1322,3 +1291,17 @@ fn preserve_comment_lang_description_on_conversion() {
_ => panic!("Expected a CommentFrame"),
}
}
// TODO: Remove this once we have a better solution
#[test]
fn hold_back_4_character_txxx_description() {
let mut tag = Id3v2Tag::new();
let _ = tag.insert_user_text(String::from("MODE"), String::from("CBR"));
let tag: Tag = tag.into();
assert_eq!(tag.len(), 0);
let tag: Id3v2Tag = tag.into();
assert_eq!(tag.len(), 1);
}

2
lofty/src/tag/item.rs
View file

@ -1,3 +1,4 @@
use crate::tag::items::{Lang, UNKNOWN_LANGUAGE};
use crate::tag::TagType;
use std::borrow::Cow;
@ -9,7 +10,6 @@ macro_rules! first_key {
};
}
use crate::tag::items::{Lang, UNKNOWN_LANGUAGE};
pub(crate) use first_key;
// This is used to create the key/ItemKey maps