Serial-ATA / Serial-ATA/lofty-rs
A stray APE footer discards an entire MPEG file, in every parsing mode
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 359
- Forks
- 76
- Avg merge
- 12h 54m
- Merged PRs (30d)
- 11
Description
Reproducer
I tried this code:
//! A minimal reproduction: an MP3 with a valid ID3v2 tag and a stray APE footer.
use lofty::config::{ParseOptions, ParsingMode};
use lofty::file::{AudioFile, TaggedFileExt};
use lofty::prelude::{Accessor, ItemKey, TagExt};
use lofty::probe::Probe;
use lofty::tag::{ItemValue, Tag, TagItem, TagType};
use std::fs;
use std::io::Write;
use std::path::Path;
/// Ten MPEG-1 Layer III frames: FF FB 90 00 is 128 kbit/s, 44.1 kHz, stereo, and a
/// frame is then 417 bytes. Padded with 0xAA rather than zeroes, which matters: see
/// the note by the footer below.
fn write_mp3(path: &Path) {
let mut frame = vec![0xAAu8; 417];
frame[..4].copy_from_slice(&[0xFF, 0xFB, 0x90, 0x00]);
let mut bytes = Vec::new();
for _ in 0..10 {
bytes.extend_from_slice(&frame);
}
fs::write(path, bytes).unwrap();
}
/// The last 32 bytes of the four real files, byte for byte: an APEv2 footer that
/// claims a 142-byte tag with 3 items, where what precedes it is audio.
///
/// The 0xAA padding is what those items are then read out of. Zero padding reads as
/// an item of no length and is accepted, so a file of silence does not reproduce this.
fn append_orphan_ape_footer(path: &Path) {
let mut footer = Vec::new();
footer.extend_from_slice(b"APETAGEX");
footer.extend_from_slice(&2000u32.to_le_bytes()); // version
footer.extend_from_slice(&142u32.to_le_bytes()); // size, including this footer
footer.extend_from_slice(&3u32.to_le_bytes()); // item count
footer.extend_from_slice(&0xA000_0000u32.to_le_bytes()); // footer, has header
footer.extend_from_slice(&[0u8; 8]); // reserved
assert_eq!(footer.len(), 32);
let mut file = fs::OpenOptions::new().append(true).open(path).unwrap();
file.write_all(&footer).unwrap();
}
fn read(path: &Path, mode: ParsingMode) -> String {
match Probe::open(path)
.unwrap()
.options(ParseOptions::new().parsing_mode(mode))
.read()
{
Ok(tagged) => {
let title = tagged
.primary_tag()
.or_else(|| tagged.first_tag())
.and_then(|tag| tag.title().map(|t| t.to_string()))
.unwrap_or_else(|| "<no title>".into());
format!(
"Ok — title {title:?}, {} ms",
tagged.properties().duration().as_millis()
)
}
Err(e) => format!("Err — {e}"),
}
}
fn main() {
let dir = std::env::temp_dir().join("lofty-orphan-ape-footer");
let _ = fs::remove_dir_all(&dir);
fs::create_dir_all(&dir).unwrap();
let path = dir.join("song.mp3");
write_mp3(&path);
let mut tag = Tag::new(TagType::Id3v2);
for (key, value) in [
(ItemKey::TrackTitle, "Winter"),
(ItemKey::TrackArtist, "Birdy"),
(ItemKey::AlbumTitle, "Beautiful Lies"),
] {
tag.insert(TagItem::new(key, ItemValue::Text(value.to_string())));
}
tag.save_to_path(&path, Default::default()).unwrap();
println!("lofty 0.24.0\n");
println!("without the APE footer:");
for mode in [ParsingMode::Strict, ParsingMode::BestAttempt, ParsingMode::Relaxed] {
println!(" {mode:?}: {}", read(&path, mode));
}
append_orphan_ape_footer(&path);
println!("\nwith 32 bytes of APE footer appended:");
for mode in [ParsingMode::Strict, ParsingMode::BestAttempt, ParsingMode::Relaxed] {
println!(" {mode:?}: {}", read(&path, mode));
}
}
Summary
An MP3 whose last 32 bytes are an APEv2 footer with no tag body behind it cannot be read at all: Probe::read returns SizeMismatch ("Encountered an invalid item size, either too big or too small to be valid") and everything else about the file is lost — a perfectly valid ID3v2 tag, the stream properties, the duration.
ParsingMode::Relaxed behaves exactly like Strict here, so there is no way to ask lofty to skip the tag it cannot parse and keep the ones it can.
What happens, and what we expected
Four files in a user's library recorded nothing at all. Each has a complete and valid ID3v2.3 tag — 38 frames, title, artist, album, ISRC, MusicBrainz ids, lyrics, a 128 KiB cover — and each ends in an APEv2 footer declaring a 142-byte tag with 3 items, where what precedes the footer is audio. The footer's tag is simply not there; something wrote the footer and not the body.
lofty reads that footer last, in mpeg/read.rs, after everything it has already read correctly:
match crate::ape::tag::read::read_ape_tag(reader, true, parse_options)? {
The ? there discards the whole TaggedFile.
What we expected, at least in BestAttempt and Relaxed, is what the documentation of those modes describes: the offending item is discarded and the parser moves on. A malformed optional trailing tag invalidating a file whose primary tag is valid is surprising — especially since the same file is missing nothing that lofty needs.
We are not suggesting the file is well-formed. It is not: the APE tag is corrupt. The question is whether one corrupt trailing tag should cost the caller everything else in the file.
Reproduction
See the code above. It writes a minimal MPEG file, tags it with lofty itself — so the ID3v2 tag is beyond
question — reads it in all three parsing modes, then appends the 32-byte footer and reads it again.
Two details of the fixture matter:
- The audio is padded with
0xAArather than zeroes. An orphan footer is read as items made out of whatever precedes it, and zero padding reads as an item of no length and is accepted — so a file of silence does not reproduce this. The real files are0xAApadded, which reads as an item of 2 863 311 530 bytes. - The footer is byte-for-byte the one those four files carry: version 2000, size 142, item count 3, flags
0xA0000000.
Output
lofty 0.24.0
without the APE footer:
Strict: Ok — title "Winter", 261 ms
BestAttempt: Ok — title "Winter", 261 ms
Relaxed: Ok — title "Winter", 261 ms
with 32 bytes of APE footer appended:
Strict: Err — Encountered an invalid item size, either too big or too small to be valid
BestAttempt: Err — Encountered an invalid item size, either too big or too small to be valid
Relaxed: Err — Encountered an invalid item size, either too big or too small to be valid
For comparison
Mutagen also refuses the APE tag on these files — APEBadItemError: 'w\x16h8' is not a valid APEv2 key — which we take as independent confirmation that the tag really is corrupt. The difference is that reading the ID3 tags of the same file with Mutagen succeeds: the broken APE tag only fails the caller who asks for the APE tag.
Context
Found while scanning a library of 11 291 files, of which these four were the only ones affected. Removing the 32-byte footer — nothing else — makes all four read completely in unmodified lofty, with identical ID3 frames and identical durations, so the audio and the ID3v2 tag were never in question.
Happy to test a patch against the real files.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in mpeg/read.rs at the read_ape_tag(reader, true, parse_options)? call, then follow the supplied reproducer and the APE reader behavior for the orphan footer. Done means BestAttempt and Relaxed preserve the valid ID3v2 tag and MPEG properties instead of returning SizeMismatch; use the reproducer to verify all parsing modes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 56/100