Read partially broken files

Cargo.lock

@@ -12,6 +12,12 @@ version = "1.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "aae1277d39aeec15cb388266ecc24b11c80469deae6067e17a1a7aa9e5c1f234"
 
+[[package]]
+name = "anyhow"
+version = "1.0.38"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "afddf7f520a80dbf76e6f50a35bca42a2331ef227a28b3b6dc5c2e2338d114b1"
+
 [[package]]
 name = "autocfg"
 version = "1.0.1"
@@ -202,6 +208,7 @@ dependencies = [
 name = "id3-image"
 version = "0.2.0"
 dependencies = [
+ "anyhow",
  "id3",
  "image",
  "structopt",

Cargo.toml

@@ -15,6 +15,7 @@ categories = [ "command-line-utilities", "filesystem", "multimedia" ]
 travis-ci = { repository = "AndrewRadev/id3-image" }
 
 [dependencies]
+anyhow = "1.0.38"
 id3 = "0.6.2"
 image = "0.23.13"
 structopt = { version = "0.3.21", default-features = false }

src/lib.rs

@@ -1,5 +1,6 @@
 use std::path::Path;
-use std::error::Error;
+
+use anyhow::anyhow;
 
 /// Embed the image from `image_filename` into `music_filename`, in-place. Any errors reading ID3
 /// tags from the music file or parsing the image get propagated upwards.
@@ -7,12 +8,10 @@ use std::error::Error;
 /// The image is encoded as a JPEG with a 90% quality setting, and embedded as a "Front cover".
 /// Tags get written as ID3v2.3.
 ///
-pub fn embed_image(music_filename: &Path, image_filename: &Path) -> Result<(), Box<dyn Error>> {
+pub fn embed_image(music_filename: &Path, image_filename: &Path) -> anyhow::Result<()> {
-    let mut tag = id3::Tag::read_from_path(&music_filename).
+    let mut tag = read_tag(music_filename)?;
-        map_err(|e| format!("Error reading music file {:?}: {}", music_filename, e))?;
-
     let image = image::open(&image_filename).
-        map_err(|e| format!("Error reading image {:?}: {}", image_filename, e))?;
+        map_err(|e| anyhow!("Error reading image {:?}: {}", image_filename, e))?;
 
     let mut encoded_image_bytes = Vec::new();
     // Unwrap: Writing to a Vec should always succeed;
@@ -26,7 +25,7 @@ pub fn embed_image(music_filename: &Path, image_filename: &Path) -> Result<(), B
     });
 
     tag.write_to_path(music_filename, id3::Version::Id3v23).
-        map_err(|e| format!("Error writing image to music file {:?}: {}", music_filename, e))?;
+        map_err(|e| anyhow!("Error writing image to music file {:?}: {}", music_filename, e))?;
 
     Ok(())
 }
@@ -37,24 +36,22 @@ pub fn embed_image(music_filename: &Path, image_filename: &Path) -> Result<(), B
 /// Any errors from parsing id3 tags will be propagated. The function will also return an error if
 /// there's no embedded images in the mp3 file.
 ///
-pub fn extract_first_image(music_filename: &Path, image_filename: &Path) -> Result<(), Box<dyn Error>> {
+pub fn extract_first_image(music_filename: &Path, image_filename: &Path) -> anyhow::Result<()> {
-    let tag = id3::Tag::read_from_path(&music_filename).
+    let tag = read_tag(music_filename)?;
-        map_err(|e| format!("Error reading music file {:?}: {}", music_filename, e))?;
-
     let first_picture = tag.pictures().next();
 
     if let Some(p) = first_picture {
         match image::load_from_memory(&p.data) {
             Ok(image) => {
                 image.save(&image_filename).
-                    map_err(|e| format!("Couldn't write image file {:?}: {}", image_filename, e))?;
+                    map_err(|e| anyhow!("Couldn't write image file {:?}: {}", image_filename, e))?;
             },
-            Err(e) => return Err(format!("Couldn't load image: {}", e).into()),
+            Err(e) => return Err(anyhow!("Couldn't load image: {}", e)),
         };
 
         Ok(())
     } else {
-        Err("No image found in music file".into())
+        Err(anyhow!("No image found in music file"))
     }
 }
 
@@ -63,14 +60,19 @@ pub fn extract_first_image(music_filename: &Path, image_filename: &Path) -> Resu
 ///
 /// If the mp3 file's ID3 tags can't be parsed, the error will be propagated upwards.
 ///
-pub fn remove_images(music_filename: &Path) -> Result<(), Box<dyn Error>> {
+pub fn remove_images(music_filename: &Path) -> anyhow::Result<()> {
-    let mut tag = id3::Tag::read_from_path(&music_filename).
+    let mut tag = read_tag(music_filename)?;
-        map_err(|e| format!("Error reading music file {:?}: {}", music_filename, e))?;
-
     tag.remove("APIC");
 
     tag.write_to_path(music_filename, id3::Version::Id3v23).
-        map_err(|e| format!("Error updating music file {:?}: {}", music_filename, e))?;
+        map_err(|e| anyhow!("Error updating music file {:?}: {}", music_filename, e))?;
 
     Ok(())
 }
+
+fn read_tag(path: &Path) -> anyhow::Result<id3::Tag> {
+    id3::Tag::read_from_path(&path).or_else(|e| {
+        eprintln!("Warning: file metadata is corrupted, trying to read partial tag: {}", path.display());
+        e.partial_tag.clone().ok_or_else(|| anyhow!("Error reading music file {:?}: {}", path, e))
+    })
+}

tests/test_processing.rs

@@ -75,6 +75,17 @@ fn test_successful_jpeg_image_embedding() {
     assert!(tag.pictures().count() > 0);
 }
 
+#[test]
+fn test_successful_jpeg_image_embedding_with_a_broken_file() {
+    let song  = Fixture::copy("attempt_1_broken.mp3");
+    let image = Fixture::copy("attempt_1.jpg");
+
+    embed_image(&song, &image).unwrap();
+
+    let tag = read_tag(&song);
+    assert!(tag.pictures().count() > 0);
+}
+
 #[test]
 fn test_successful_png_image_embedding() {
     let song  = Fixture::copy("attempt_1_no_image.mp3");
@@ -122,6 +133,22 @@ fn test_removing_and_adding_an_image() {
     assert!(tag.pictures().count() > 0);
 }
 
+#[test]
+fn test_removing_and_adding_an_image_to_a_broken_file() {
+    let song  = Fixture::copy("attempt_1_broken.mp3");
+    let image = Fixture::copy("attempt_1.jpg");
+
+    remove_images(&song).unwrap();
+
+    let tag = read_tag(&song);
+    assert!(tag.pictures().count() == 0);
+
+    embed_image(&song, &image).unwrap();
+
+    let tag = read_tag(&song);
+    assert!(tag.pictures().count() > 0);
+}
+
 #[test]
 fn test_extracting_a_jpg_image() {
     let song  = Fixture::copy("attempt_1.mp3");
@@ -136,6 +163,16 @@ fn test_extracting_a_jpg_image() {
     assert!(image.exists());
 }
 
+#[test]
+fn test_extracting_a_jpg_image_from_a_broken_file() {
+    let song  = Fixture::copy("attempt_1_broken.mp3");
+    let image = Fixture::blank("attempt_1.jpg");
+
+    extract_first_image(&song, &image).unwrap();
+
+    assert!(image.exists());
+}
+
 #[test]
 fn test_extracting_a_png_image() {
     let song  = Fixture::copy("attempt_1.mp3");