From fda2eb2c90a879dfa774bbdab40eb33f5f253436 Mon Sep 17 00:00:00 2001 From: axel10 Date: Sun, 24 May 2026 20:16:06 +0800 Subject: [PATCH 1/4] fix(mp4): prevent overwriting `hdlr` atom when writing new tags to files with empty `ilst` --- lofty/src/mp4/ilst/write.rs | 3 ++- lofty/src/mp4/read/mod.rs | 11 ++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lofty/src/mp4/ilst/write.rs b/lofty/src/mp4/ilst/write.rs index faba3dcea..90b58c147 100644 --- a/lofty/src/mp4/ilst/write.rs +++ b/lofty/src/mp4/ilst/write.rs @@ -230,7 +230,7 @@ fn save_to_existing( ParseOptions::DEFAULT_PARSING_MODE, )?; - if tree.is_empty() { + if ilst_idx.is_none() { // Nothing to do if remove_tag { return Ok(()); @@ -241,6 +241,7 @@ fn save_to_existing( replacement = ilst; range = meta_end..meta_end; } else { + let ilst_idx = ilst_idx.unwrap(); let existing_ilst = &tree[ilst_idx]; let existing_ilst_size = existing_ilst.len; diff --git a/lofty/src/mp4/read/mod.rs b/lofty/src/mp4/read/mod.rs index f55bbb60a..3ff9fec56 100644 --- a/lofty/src/mp4/read/mod.rs +++ b/lofty/src/mp4/read/mod.rs @@ -156,11 +156,11 @@ pub(super) fn atom_tree( mut len: u64, up_to: &[u8], parse_mode: ParsingMode, -) -> Result<(usize, Vec)> +) -> Result<(Option, Vec)> where R: Read + Seek, { - let mut found_idx: usize = 0; + let mut found_idx: Option = None; let mut buf = Vec::new(); let mut i = 0; @@ -174,18 +174,15 @@ where len = len.saturating_sub(atom.len); if let AtomIdent::Fourcc(ref fourcc) = atom.ident { - i += 1; - if fourcc == up_to { - found_idx = i; + found_idx = Some(i); } + i += 1; buf.push(atom); } } - found_idx = found_idx.saturating_sub(1); - Ok((found_idx, buf)) } From ab5b0173babed95ca6cc2bb77a3cbd9c083536c6 Mon Sep 17 00:00:00 2001 From: axel10 Date: Sun, 24 May 2026 20:44:23 +0800 Subject: [PATCH 2/4] test: add unit tests for MP4 file reading, writing, removal, and atom preservation --- lofty/tests/files/mp4.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lofty/tests/files/mp4.rs b/lofty/tests/files/mp4.rs index 08f7a8ad0..0db728090 100644 --- a/lofty/tests/files/mp4.rs +++ b/lofty/tests/files/mp4.rs @@ -73,3 +73,32 @@ fn read_no_properties() { fn read_no_tags() { crate::util::no_tag_test("tests/files/assets/minimal/m4a_codec_aac.m4a", None); } + +#[test_log::test] +fn hdlr_preservation_on_tag_recreation() { + use std::fs; + use lofty::tag::Tag; + + let src = "tests/files/assets/minimal/m4a_codec_aac.m4a"; + let temp_path = "tests/files/assets/minimal/temp_hdlr_test.m4a"; + let _ = fs::remove_file(temp_path); + fs::copy(src, temp_path).unwrap(); + + // 1. Remove all tags + TagType::Mp4Ilst.remove_from_path(temp_path).unwrap(); + + // 2. Open the file and write a brand new tag + let mut tagged_file = Probe::open(temp_path).unwrap().read().unwrap(); + let tag = Tag::new(TagType::Mp4Ilst); + tagged_file.insert_tag(tag); + tagged_file.save_to_path(temp_path, lofty::config::WriteOptions::default()).unwrap(); + + // 3. Read the file bytes and verify that "hdlr" is present + let bytes = fs::read(temp_path).unwrap(); + let has_hdlr = bytes.windows(4).any(|w| w == b"hdlr"); + + let _ = fs::remove_file(temp_path); + + assert!(has_hdlr, "Output M4A is missing the hdlr atom!"); +} + From ac04da5f8c86d685e7260026befc7138e2e89f9a Mon Sep 17 00:00:00 2001 From: axel10 Date: Sun, 24 May 2026 21:00:14 +0800 Subject: [PATCH 3/4] fix(mp4): resolve clippy unnecessary-unwrap warning in ilst write --- lofty/src/mp4/ilst/write.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lofty/src/mp4/ilst/write.rs b/lofty/src/mp4/ilst/write.rs index 90b58c147..cb639e68b 100644 --- a/lofty/src/mp4/ilst/write.rs +++ b/lofty/src/mp4/ilst/write.rs @@ -230,18 +230,7 @@ fn save_to_existing( ParseOptions::DEFAULT_PARSING_MODE, )?; - if ilst_idx.is_none() { - // Nothing to do - if remove_tag { - return Ok(()); - } - - let meta_end = (meta.start + meta.len) as usize; - - replacement = ilst; - range = meta_end..meta_end; - } else { - let ilst_idx = ilst_idx.unwrap(); + if let Some(ilst_idx) = ilst_idx { let existing_ilst = &tree[ilst_idx]; let existing_ilst_size = existing_ilst.len; @@ -316,6 +305,16 @@ fn save_to_existing( replacement = ilst; range = range_start as usize..range_end as usize; } + } else { + // Nothing to do + if remove_tag { + return Ok(()); + } + + let meta_end = (meta.start + meta.len) as usize; + + replacement = ilst; + range = meta_end..meta_end; } drop(write_handle); From 8b994f9f528a22015e180075913b9cf4dd92bb26 Mon Sep 17 00:00:00 2001 From: axel10 Date: Mon, 25 May 2026 06:58:46 +0800 Subject: [PATCH 4/4] fix(fmt): normalize mp4 test formatting --- lofty/tests/files/mp4.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lofty/tests/files/mp4.rs b/lofty/tests/files/mp4.rs index 0db728090..7d41602b6 100644 --- a/lofty/tests/files/mp4.rs +++ b/lofty/tests/files/mp4.rs @@ -76,8 +76,8 @@ fn read_no_tags() { #[test_log::test] fn hdlr_preservation_on_tag_recreation() { - use std::fs; use lofty::tag::Tag; + use std::fs; let src = "tests/files/assets/minimal/m4a_codec_aac.m4a"; let temp_path = "tests/files/assets/minimal/temp_hdlr_test.m4a"; @@ -91,7 +91,9 @@ fn hdlr_preservation_on_tag_recreation() { let mut tagged_file = Probe::open(temp_path).unwrap().read().unwrap(); let tag = Tag::new(TagType::Mp4Ilst); tagged_file.insert_tag(tag); - tagged_file.save_to_path(temp_path, lofty::config::WriteOptions::default()).unwrap(); + tagged_file + .save_to_path(temp_path, lofty::config::WriteOptions::default()) + .unwrap(); // 3. Read the file bytes and verify that "hdlr" is present let bytes = fs::read(temp_path).unwrap(); @@ -101,4 +103,3 @@ fn hdlr_preservation_on_tag_recreation() { assert!(has_hdlr, "Output M4A is missing the hdlr atom!"); } -