Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID3v2: Stop editing the string in Id3v2Tag::get_text() #337

Merged
merged 2 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **ID3v2**:
- Stop erroring on empty frames when not using `ParsingMode::Strict` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/299))
- Verify contents of flag items (`ItemKey::FlagCompilation`, `ItemKey::FlagPodcast`) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/336))
- `Id3v2Tag::get_text` will now return the raw, unedited string ([PR](https://github.com/Serial-ATA/lofty-rs/pull/336))
- Previously, all null separators were replaced with `"/"` to make the string easier to display.
Now, null separators are only replaced in [`Accessor`](https://docs.rs/lofty/latest/lofty/trait.Accessor.html) methods.
It is up to the caller to decide how to handle all other strings.
- **resolve**: Custom resolvers will now be checked before the default resolvers ([PR](https://github.com/Serial-ATA/lofty-rs/pull/319))
- **MPEG**: Up to `max_junk_bytes` will now be searched for tags between the start of the file and the first MPEG frame ([PR](https://github.com/Serial-ATA/lofty-rs/pull/320))
- This allows us to read and write ID3v2 tags that are preceeded by junk
Expand Down
62 changes: 50 additions & 12 deletions src/id3/v2/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,25 @@ const COMMENT_FRAME_ID: &str = "COMM";

const V4_MULTI_VALUE_SEPARATOR: char = '\0';

// Used exclusively for `Accessor` convenience methods
fn remove_separators_from_frame_text(value: &str, version: Id3v2Version) -> Cow<'_, str> {
if !value.contains(V4_MULTI_VALUE_SEPARATOR) || version != Id3v2Version::V4 {
return Cow::Borrowed(value);
}

return Cow::Owned(value.replace(V4_MULTI_VALUE_SEPARATOR, "/"));
}

macro_rules! impl_accessor {
($($name:ident => $id:literal;)+) => {
paste::paste! {
$(
fn $name(&self) -> Option<Cow<'_, str>> {
self.get_text(&[<$name:upper _ID>])
if let Some(value) = self.get_text(&[<$name:upper _ID>]) {
return Some(remove_separators_from_frame_text(value, self.original_version));
}

None
}

fn [<set_ $name>](&mut self, value: String) {
Expand All @@ -60,6 +73,13 @@ macro_rules! impl_accessor {
}
}

/// ## [`Accessor`] Methods
///
/// As ID3v2.4 allows for multiple values to exist in a single frame, the raw strings, as provided by [`Id3v2Tag::get_text`]
/// may contain null separators.
///
/// In the [`Accessor`] methods, these values have the separators (`\0`) replaced with `"/"` for convenience.
///
/// ## Conversions
///
/// ⚠ **Warnings** ⚠
Expand Down Expand Up @@ -171,24 +191,42 @@ impl Id3v2Tag {

/// Gets the text for a frame
///
/// NOTE: If the tag is [`Id3v2Version::V4`], there could be multiple values separated by null characters (`'\0'`).
/// Use [`Id3v2Tag::get_texts`] to conveniently split all of the values.
///
/// NOTE: This will not work for `TXXX` frames, use [`Id3v2Tag::get_user_text`] for that.
///
/// If the tag is [`Id3v2Version::V4`], this will allocate if the text contains any
/// null (`'\0'`) text separators to replace them with a slash (`'/'`).
pub fn get_text(&self, id: &FrameId<'_>) -> Option<Cow<'_, str>> {
/// # Examples
///
/// ```rust
/// use lofty::id3::v2::{FrameId, Id3v2Tag};
/// use lofty::Accessor;
/// use std::borrow::Cow;
///
/// const TITLE_ID: FrameId<'_> = FrameId::Valid(Cow::Borrowed("TIT2"));
///
/// let mut tag = Id3v2Tag::new();
///
/// tag.set_title(String::from("Foo"));
///
/// let title = tag.get_text(&TITLE_ID);
/// assert_eq!(title, Some("Foo"));
///
/// // Now we have a string with multiple values
/// tag.set_title(String::from("Foo\0Bar"));
///
/// // Null separator is retained! This case is better handled by `get_texts`.
/// let title = tag.get_text(&TITLE_ID);
/// assert_eq!(title, Some("Foo\0Bar"));
/// ```
pub fn get_text(&self, id: &FrameId<'_>) -> Option<&str> {
let frame = self.get(id);
if let Some(Frame {
value: FrameValue::Text(TextInformationFrame { value, .. }),
..
}) = frame
{
if !value.contains(V4_MULTI_VALUE_SEPARATOR)
|| self.original_version != Id3v2Version::V4
{
return Some(Cow::Borrowed(value.as_str()));
}

return Some(Cow::Owned(value.replace(V4_MULTI_VALUE_SEPARATOR, "/")));
return Some(value);
}

None
Expand Down Expand Up @@ -223,7 +261,7 @@ impl Id3v2Tag {
..
}) = self.get(id)
{
return Some(value.split('\0'));
return Some(value.split(V4_MULTI_VALUE_SEPARATOR));
}

None
Expand Down