Panic reading mp3 tag

[milesegan]

Reproducer

I tried this code:

use std::path::Path;

use lofty::config::{ParseOptions, ParsingMode};
use lofty::file::{AudioFile, TaggedFileExt};
use lofty::probe::Probe;
use lofty::tag::Accessor;

fn main() {
    // the error seems to occur with strict or relaxed parsing mode
    let parsing_options = ParseOptions::new().parsing_mode(ParsingMode::Relaxed);
    let path = Path::new("sample.mp3");
    let file = Probe::open(&path)
        .expect("can't probe")
        .options(parsing_options)
        .read()
        .expect("can't read");
    let tag = file.primary_tag().expect("no tag");
    let properties = file.properties();
    println!("{:?}", tag.artist());
    println!("{:?}", tag.year());
}

Summary

This results in this panic:

thread 'main' panicked at src/main.rs:16:10:
can't read: TextDecode("UTF-16 string has an odd length")

Expected behavior

ffprobe and id3info both seem to be able to read this sample file ok. Are they just falling back to some kind of best-effort conversion for badly encoded UTF-16 strings perhaps?

Assets

sample.mp3.zip

[milesegan]

Perhaps related?

tinytag/tinytag@33ee413

[Serial-ATA]

Thanks for looking into it, that issue definitely looks related. I'll look into this ~2 weeks from now, I don't have any time to work on Lofty atm.

[Serial-ATA]

Turns out that wasn't the issue, the file is just broken. There's a second ID3v2 tag with broken UTF-16 strings immediately following the first. When the file was tagged, the correct tag was probably just prepended, rather than overwriting this invalid one.

Lofty tries to read all tags that appear before the audio data, and merge them into one:

lofty-rs/lofty/src/mpeg/read.rs

Lines 47 to 55 in dd4e926

	let id3v2 = parse_id3v2(reader, header, parse_options)?;
	if let Some(existing_tag) = &mut file.id3v2_tag {
	// https://github.com/Serial-ATA/lofty-rs/issues/87
	// Duplicate tags should have their frames appended to the previous
	for frame in id3v2.frames {
	existing_tag.insert(frame);
	}
	continue;
	}

I tested with ffprobe, and it only gives the information from the first ID3v2 tag, so I imagine it doesn't even try reading this second tag.

[milesegan]

That makes sense. Thanks for taking a look! I'll close this.