Skip to content

ID3v2: fallback when BOM is not present in UTF-16 data #614

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.

Sign up for GitHub

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

Merged
Serial-ATA merged 1 commit into from
Feb 15, 2026
Merged

ID3v2: fallback when BOM is not present in UTF-16 data #614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- **ID3v2**: Don't error on empty UTF-16 descriptions ([issue](https://github.com/Serial-ATA/lofty-rs/issues/613)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/614))

## [0.23.2] - 2026-02-14

### Fixed

- **FLAC**:
- Fix duplicate `Last-metadata-block` flags in the presence of PADDING blocks ([issue](https://github.com/Serial-ATA/lofty-rs/issues/607)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/609))
- Ignore ID3v2 tags when not stripped during write ([issue](https://github.com/Serial-ATA/lofty-rs/issues/608)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/609))
Expand Down
31 changes: 18 additions & 13 deletions lofty/src/id3/v2/items/language_frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::error::{Id3v2Error, Id3v2ErrorKind, Result};
use crate::id3::v2::frame::content::verify_encoding;
use crate::id3::v2::header::Id3v2Version;
use crate::id3::v2::{FrameFlags, FrameHeader, FrameId};
use crate::macros::err;
use crate::tag::items::Lang;
use crate::util::text::{
DecodeTextResult, TextDecodeOptions, TextEncoding, decode_text,
Expand Down Expand Up @@ -49,20 +48,26 @@ impl LanguageFrame {
TextDecodeOptions::new().encoding(encoding).terminated(true),
)?;

let mut endianness: fn([u8; 2]) -> u16 = u16::from_le_bytes;

// It's possible for the description to be the only string with a BOM
// To be safe, we change the encoding to the concrete variant determined from the description
if encoding == TextEncoding::UTF16 {
endianness = match bom {
[0xFF, 0xFE] => u16::from_le_bytes,
[0xFE, 0xFF] => u16::from_be_bytes,
_ => err!(TextDecode("UTF-16 string missing a BOM")),
};
}
// There are 3 possibilities for UTF-16 encoded frames:
//
// * The description is the only string with a BOM
// * The description is empty (has no BOM)
// * All strings have a BOM
//
// To be safe, we change the encoding to the concrete variant determined from the description.
// Otherwise, we just have to hope that the other fields are encoded properly.
let endianness: Option<fn([u8; 2]) -> u16> = if encoding == TextEncoding::UTF16 {
match bom {
[0xFF, 0xFE] => Some(u16::from_le_bytes),
[0xFE, 0xFF] => Some(u16::from_be_bytes),
_ => None,
}
} else {
None
};
Comment on lines +59 to +67
Copy link
Owner

@Serial-ATA Serial-ATA Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example with a broken language frame? Not sure I understand what the solution here is. decode_text with an encoding of UTF16 should fail anyway if there isn't a valid BOM.

Copy link
Contributor Author

@aschey aschey Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, here's one that reproduces the problem. The frame does decode correctly with this change in place. I don't know much about id3 tags so maybe this is just a band-aid fix, but I was trying to preserve the behavior from 0.22 where this worked previously.

04 Blood on the Radio.zip - the lyrics tag is the one that fails here without this change

Copy link
Owner

@Serial-ATA Serial-ATA Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah. #535 broke the case of empty descriptions which shouldn't have a BOM. This change makes sense then, I'll just update the comment


let content;
if encoding == TextEncoding::UTF16 {
if let Some(endianness) = endianness {
(content, _) = utf16_decode_terminated_maybe_bom(reader, endianness)?;
} else {
content = decode_text(reader, TextDecodeOptions::new().encoding(encoding))?.content;
Expand Down
30 changes: 18 additions & 12 deletions lofty/src/id3/v2/items/sync_text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,23 @@ impl SynchronizedTextFrame<'_> {
)
.map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadSyncText))?;

let mut endianness: fn([u8; 2]) -> u16 = u16::from_le_bytes;

// It's possible for the description to be the only string with a BOM
// To be safe, we change the encoding to the concrete variant determined from the description
if encoding == TextEncoding::UTF16 {
endianness = match bom {
[0xFF, 0xFE] => u16::from_le_bytes,
[0xFE, 0xFF] => u16::from_be_bytes,
_ => err!(TextDecode("UTF-16 string missing a BOM")),
};
}
// There are 3 possibilities for UTF-16 encoded frames:
//
// * The description is the only string with a BOM
// * The description is empty (has no BOM)
// * All strings have a BOM
//
// To be safe, we change the encoding to the concrete variant determined from the description.
// Otherwise, we just have to hope that the other fields are encoded properly.
let endianness: Option<fn([u8; 2]) -> u16> = if encoding == TextEncoding::UTF16 {
match bom {
[0xFF, 0xFE] => Some(u16::from_le_bytes),
[0xFE, 0xFF] => Some(u16::from_be_bytes),
_ => None,
}
} else {
None
};

let mut pos = 0;
let total = (data.len() - 6) as u64 - cursor.stream_position()?;
Expand All @@ -178,7 +184,7 @@ impl SynchronizedTextFrame<'_> {

while pos < total {
let text;
if encoding == TextEncoding::UTF16 {
if let Some(endianness) = endianness {
let (decoded, bytes_read) =
utf16_decode_terminated_maybe_bom(&mut cursor, endianness)
.map_err(|_| Id3v2Error::new(Id3v2ErrorKind::BadSyncText))?;
Expand Down