From a19cb7576871c6f28c0097da54413c28e47bc2f6 Mon Sep 17 00:00:00 2001
From: Damien Elmes <gpg@ankiweb.net>
Date: Thu, 18 May 2023 11:33:01 +1000
Subject: [PATCH] Fix broken media importing in non-legacy apkg path

- Ensure our unit tests cover the non-legacy path, and check file
contents and not just existence.
- Pass `compressed` into copy_and_ensure_sha1_set(). We can't just
test for self.sha1.is_none(), as that fails in the case where an existing
media file was found in the legacy path.

https://forums.ankiweb.net/t/anki-media-file-corruption-when-exporting-png-files/30315
---
 .../package/apkg/import/media.rs              |  1 +
 rslib/src/import_export/package/apkg/tests.rs | 28 +++++++++++++++----
 rslib/src/import_export/package/media.rs      |  7 +++--
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/rslib/src/import_export/package/apkg/import/media.rs b/rslib/src/import_export/package/apkg/import/media.rs
index 35f405713..c1d0bfad3 100644
--- a/rslib/src/import_export/package/apkg/import/media.rs
+++ b/rslib/src/import_export/package/apkg/import/media.rs
@@ -61,6 +61,7 @@ impl Context<'_> {
                     &mut self.archive,
                     &self.target_col.media_folder,
                     &mut copier,
+                    self.meta.zstd_compressed(),
                 )?;
                 self.media_manager
                     .add_entry(&entry.name, entry.sha1.unwrap())?;
diff --git a/rslib/src/import_export/package/apkg/tests.rs b/rslib/src/import_export/package/apkg/tests.rs
index 4bf484a34..22aa10e5d 100644
--- a/rslib/src/import_export/package/apkg/tests.rs
+++ b/rslib/src/import_export/package/apkg/tests.rs
@@ -7,6 +7,7 @@ use std::collections::HashSet;
 use std::fs::File;
 use std::io::Write;
 
+use crate::io::read_file;
 use crate::media::files::sha1_of_data;
 use crate::media::MediaManager;
 use crate::prelude::*;
@@ -19,10 +20,15 @@ const SAMPLE_JS: &str = "_sample.js";
 const JPG_DATA: &[u8] = b"1";
 const MP3_DATA: &[u8] = b"2";
 const JS_DATA: &[u8] = b"3";
-const OTHER_MP3_DATA: &[u8] = b"4";
+const EXISTING_MP3_DATA: &[u8] = b"4";
 
 #[test]
 fn roundtrip() {
+    roundtrip_inner(true);
+    roundtrip_inner(false);
+}
+
+fn roundtrip_inner(legacy: bool) {
     let (mut src_col, src_tempdir) = open_fs_test_collection("src");
     let (mut target_col, _target_tempdir) = open_fs_test_collection("target");
     let apkg_path = src_tempdir.path().join("test.apkg");
@@ -39,7 +45,7 @@ fn roundtrip() {
             SearchNode::from_deck_name("parent::sample"),
             true,
             true,
-            true,
+            legacy,
             None,
             |_, _| true,
         )
@@ -113,7 +119,7 @@ impl Collection {
 
     fn add_conflicting_media(&mut self) {
         let mut file = File::create(self.media_folder.join(SAMPLE_MP3)).unwrap();
-        file.write_all(OTHER_MP3_DATA).unwrap();
+        file.write_all(EXISTING_MP3_DATA).unwrap();
     }
 
     fn assert_decks(&mut self) {
@@ -140,9 +146,19 @@ impl Collection {
             .unwrap()
             .all_checksums_as_is();
 
-        for file in [SAMPLE_JPG, SAMPLE_JS, &new_mp3_name] {
-            assert!(self.media_folder.join(file).exists());
-            assert_ne!(*csums.get(file).unwrap(), [0; 20]);
+        for (fname, orig_data) in [
+            (SAMPLE_JPG, JPG_DATA),
+            (SAMPLE_MP3, EXISTING_MP3_DATA),
+            (new_mp3_name.as_str(), MP3_DATA),
+            (SAMPLE_JS, JS_DATA),
+        ] {
+            // data should have been copied correctly
+            assert_eq!(
+                read_file(&self.media_folder.join(fname)).unwrap(),
+                orig_data
+            );
+            // and checksums in media db should be valid
+            assert_eq!(*csums.get(fname).unwrap(), sha1_of_data(orig_data));
         }
 
         let imported_note = self.storage.get_note(note.id).unwrap().unwrap();
diff --git a/rslib/src/import_export/package/media.rs b/rslib/src/import_export/package/media.rs
index c67ea46b0..1f403165b 100644
--- a/rslib/src/import_export/package/media.rs
+++ b/rslib/src/import_export/package/media.rs
@@ -121,14 +121,15 @@ impl SafeMediaEntry {
         archive: &mut ZipArchive<File>,
         target_folder: &Path,
         copier: &mut MediaCopier,
+        compressed: bool,
     ) -> Result<()> {
         let mut file = self.fetch_file(archive)?;
         let mut tempfile = new_tempfile_in(target_folder)?;
-        if self.sha1.is_none() {
+        if compressed {
+            copy_decode(&mut file, &mut tempfile)?
+        } else {
             let (_, sha1) = copier.copy(&mut file, &mut tempfile)?;
             self.sha1 = Some(sha1);
-        } else {
-            io::copy(&mut file, &mut tempfile)?;
         }
         atomic_rename(tempfile, &self.file_path(target_folder), false)?;