polyfloyd / rust-id3

A rust library for reading and writing ID3 metadata
MIT License
245 stars 47 forks source link

The slash (`/`) characters in tags are not correctly parsed #103

Open tranxuanthang opened 2 years ago

tranxuanthang commented 2 years ago

Looks like slash characters in tags are not parsed correctly by rust-id3 and are converted to null terminator character.

Some examples are:

EDIT: Update example file. I'm not quite sure yet but it seems that the issue only happens in ID3 v2.3 version.

let mut tag = id3::Tag::new();
tag.add_frame(id3::Frame::text("TPE1", "Derek Duke/Russell Brower"));
tag.write_to_path("test.id3", id3::Version::Id3v23).unwrap();
let tag = id3::Tag::read_from_path("test.id3").unwrap();
println!("{:?}", tag.artist()); // Some("Derek Duke\0Russell Brower")

test.zip

tranxuanthang commented 2 years ago

After seeing these:

I understand the issue better now. But still I think assuming the slash / character as separator for multiple entries for ID3 v2.2 or v2.3 might end up messing with a few artist names. One of these cases is my first example, calling tag.artists() will get you Some(["Alan Walker, Au", "Ra, Tomine Harket"]).

At least, I think the null/slash substitute definitely should not apply for all text fields, for example album name and track name. And tag.artist() should return the expected result, without null terminator character.

polyfloyd commented 2 years ago

ID3 has support for split values in text frames. In v2.2 and v2.3 the separator is a slash while in v2.4 it is a null byte.

I intended the library to have a uniform ID3v2.4 interface, so everything that it reads is normalized to use null bytes. But I must admit that this leaves much to desire...

Splitting happens at a very low level, just after decoding at src/stream/frame/content.rs:449

It might be worth considering making the split functions of TagLike version aware so at least the underlying content remains untouched unless desired