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
This commit is contained in:
Damien Elmes 2023-05-18 11:33:01 +10:00
parent 63fd70ad34
commit a19cb75768
3 changed files with 27 additions and 9 deletions

View File

@ -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())?;

View File

@ -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();

View File

@ -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)?;