Panic on empty MP4 `AtomDataStorage::Multiple`

[hinto-janai]

Reproducer

No response

Summary

I'm reliably hitting a panic at: https://github.com/Serial-ATA/lofty-rs/blob/b324bfa478298d2b08931738612b6d13437f7e51/src/mp4/ilst/atom.rs#L18 while probing one of my audio files.

Stack trace:

[ ... panic unwind ... ]

   9: core::option::expect_failed
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/option.rs:2045:5
  10: core::option::Option<T>::expect
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/option.rs:905:21
  11: lofty::mp4::ilst::atom::AtomDataStorage::first_mut
             at ./git/festival/external/lofty-rs/src/mp4/ilst/atom.rs:18:39
  12: <lofty::mp4::ilst::Ilst as lofty::traits::SplitTag>::split_tag::{{closure}}
             at ./git/festival/external/lofty-rs/src/mp4/ilst/mod.rs:475:22
  13: alloc::vec::Vec<T,A>::retain_mut::process_loop
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/alloc/src/vec/mod.rs:1647:21
  14: alloc::vec::Vec<T,A>::retain_mut
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/alloc/src/vec/mod.rs:1676:9
  15: <lofty::mp4::ilst::Ilst as lofty::traits::SplitTag>::split_tag
             at ./git/festival/external/lofty-rs/src/mp4/ilst/mod.rs:473:3
  16: lofty::mp4::ilst::<impl core::convert::From<lofty::mp4::ilst::Ilst> for lofty::tag::Tag>::from
             at ./git/festival/external/lofty-rs/src/mp4/ilst/mod.rs:631:3
  17: <T as core::convert::Into<U>>::into
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/convert/mod.rs:726:9
  18: lofty::mp4::<impl core::convert::From<lofty::mp4::Mp4File> for lofty::file::TaggedFile>::from
             at ./git/festival/external/lofty-rs/src/mp4/mod.rs:31:10
  19: <T as core::convert::Into<U>>::into
             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/convert/mod.rs:726:9
  20: lofty::probe::Probe<R>::read
             at ./git/festival/external/lofty-rs/src/probe.rs:518:22

I am using a fork but it is the exact same as the current 0.13.0 commit https://github.com/Serial-ATA/lofty-rs/commit/b324bfa478298d2b08931738612b6d13437f7e51 with no changes.

I'd like to provide more info but I can't actually find which file is causing this. Debug printing before the call at: https://github.com/Serial-ATA/lofty-rs/blob/2b562c4a4b73b96d9bbc7e75268804ad12051a6e/src/mp4/ilst/mod.rs#L380-L386 only reveals Vec's that are .len() >= 1 so I'm not sure. Help on how to debug would be appreciated.

For now, would something like:

impl AtomDataStorage {
    pub(super) fn first_mut(&mut self) -> &mut AtomData {
        match self {
            AtomDataStorage::Single(val) => val,
-            AtomDataStorage::Multiple(data) => data.first_mut().expect("not empty"),
+            AtomDataStorage::Multiple(data) => {
+                if data.is_empty() {
+                    data.push(AtomData::Unknown {
+                        ..Default::default()
+                    });
+                }
+
+                data.first_mut().expect("not empty"),
+            }
        }
    }
}

be okay?

Expected behavior

No response

Assets

No response

[hinto-janai]

I found the culprit files and maybe the character causing the panic:

℗

This character was in the copyright field, alongside the company name. Removing it stops the panics.

Ideally, you would be able to reproduce this panic with the same files, although, uploading them here seems... dangerous. How should I proceed?

Other metadata programs (puddletag, id3info, music players) seem to work fine and parse the data correctly.

I'm not sure if that character is actually the problem. Re-formatting the copyright field without removing the symbol fixes it. My suspicion is that the tag data is being converted from some weird formatting to a valid one when doing that.

[Serial-ATA]

Thanks for the report!

I'm not able to replicate it with by simply adding ℗ to a copyright message.

For now, would something like:

...

be okay?

AtomDataStorage::Multiple shouldn't need any extra checks for fetching. For it to reach that point means there's a deeper issue here.

Ideally, you would be able to reproduce this panic with the same files, although, uploading them here seems... dangerous. How should I proceed?

You can email them to serial@[domain on my profile] :)

[hinto-janai]

I've sent an email from the one on my profile.

Sorry for all the misdirections but I'm not actually sure it's the copyright field anymore. Re-formatting any field in puddletag seems to fix it; something about the file must have invalid data, which puddletag fixes after saving tag changes.

[Serial-ATA]

Can you verify #202 fixes the issue? :)

[hinto-janai]

https://github.com/Serial-ATA/lofty-rs/pull/202 fixes the issue. Thanks.