From bf8e70c70f52e189eca295fc00eff077bf73b34f Mon Sep 17 00:00:00 2001
From: Damien Elmes <gpg@ankiweb.net>
Date: Thu, 17 Mar 2022 20:38:07 +1000
Subject: [PATCH] Ensure partial colpkg file removed if export fails

---
 .../import_export/package/colpkg/export.rs    |  8 ++++++--
 .../import_export/package/colpkg/import.rs    |  7 ++-----
 .../src/import_export/package/colpkg/tests.rs |  2 ++
 rslib/src/io.rs                               | 19 +++++++++++++++++++
 rslib/src/lib.rs                              |  1 +
 rslib/src/sync/mod.rs                         | 10 ++--------
 6 files changed, 32 insertions(+), 15 deletions(-)
 create mode 100644 rslib/src/io.rs

diff --git a/rslib/src/import_export/package/colpkg/export.rs b/rslib/src/import_export/package/colpkg/export.rs
index d839508b9..0e9c8561f 100644
--- a/rslib/src/import_export/package/colpkg/export.rs
+++ b/rslib/src/import_export/package/colpkg/export.rs
@@ -20,6 +20,7 @@ use zstd::{
 use super::super::{MediaEntries, MediaEntry, Meta, Version};
 use crate::{
     collection::CollectionBuilder,
+    io::atomic_rename,
     media::files::{filename_if_normalized, sha1_of_data},
     prelude::*,
 };
@@ -38,6 +39,7 @@ impl Collection {
         progress_fn: impl FnMut(usize),
     ) -> Result<()> {
         let colpkg_name = out_path.as_ref();
+        let temp_colpkg = NamedTempFile::new_in(colpkg_name.parent().ok_or(AnkiError::NotFound)?)?;
         let src_path = self.col_path.clone();
         let src_media_folder = if include_media {
             Some(self.media_folder.clone())
@@ -50,14 +52,16 @@ impl Collection {
         // exporting code should be downgrading to 18 instead of 11 (which will probably require
         // changing the boolean to an enum).
         self.close(true)?;
+
         export_collection_file(
-            &colpkg_name,
+            temp_colpkg.path(),
             &src_path,
             src_media_folder,
             legacy,
             &tr,
             progress_fn,
-        )
+        )?;
+        atomic_rename(temp_colpkg, colpkg_name)
     }
 }
 
diff --git a/rslib/src/import_export/package/colpkg/import.rs b/rslib/src/import_export/package/colpkg/import.rs
index 1350b56b4..e732353f5 100644
--- a/rslib/src/import_export/package/colpkg/import.rs
+++ b/rslib/src/import_export/package/colpkg/import.rs
@@ -22,6 +22,7 @@ use crate::{
         package::{MediaEntries, MediaEntry, Meta},
         ImportProgress,
     },
+    io::atomic_rename,
     media::files::normalize_filename,
     prelude::*,
 };
@@ -85,11 +86,7 @@ pub fn import_colpkg(
         });
 
     // Proceed with replacing collection, regardless of media import result
-    tempfile.as_file().sync_all()?;
-    tempfile.persist(&col_path).map_err(|err| err.error)?;
-    if !cfg!(windows) {
-        File::open(col_dir)?.sync_all()?;
-    }
+    atomic_rename(tempfile, &col_path)?;
 
     media_import_result
 }
diff --git a/rslib/src/import_export/package/colpkg/tests.rs b/rslib/src/import_export/package/colpkg/tests.rs
index b0642f5c2..f5ad95775 100644
--- a/rslib/src/import_export/package/colpkg/tests.rs
+++ b/rslib/src/import_export/package/colpkg/tests.rs
@@ -88,6 +88,8 @@ fn normalization_check_on_export() -> Result<()> {
             .unwrap_err(),
         AnkiError::MediaCheckRequired
     );
+    // file should have been cleaned up
+    assert!(!colpkg_name.exists());
 
     Ok(())
 }
diff --git a/rslib/src/io.rs b/rslib/src/io.rs
new file mode 100644
index 000000000..c9e549331
--- /dev/null
+++ b/rslib/src/io.rs
@@ -0,0 +1,19 @@
+use std::path::Path;
+
+use tempfile::NamedTempFile;
+
+use crate::prelude::*;
+
+pub(crate) fn atomic_rename(file: NamedTempFile, target: &Path) -> Result<()> {
+    file.as_file().sync_all()?;
+    file.persist(&target)
+        .map_err(|err| AnkiError::IoError(format!("write {target:?} failed: {err}")))?;
+    if !cfg!(windows) {
+        if let Some(parent) = target.parent() {
+            std::fs::File::open(parent)
+                .and_then(|file| file.sync_all())
+                .map_err(|err| AnkiError::IoError(format!("sync {parent:?} failed: {err}")))?;
+        }
+    }
+    Ok(())
+}
diff --git a/rslib/src/lib.rs b/rslib/src/lib.rs
index d911c3d3f..9e03edc38 100644
--- a/rslib/src/lib.rs
+++ b/rslib/src/lib.rs
@@ -19,6 +19,7 @@ pub mod error;
 pub mod findreplace;
 pub mod i18n;
 pub mod import_export;
+mod io;
 pub mod latex;
 pub mod links;
 pub mod log;
diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs
index 3d0a5054d..c8893fda3 100644
--- a/rslib/src/sync/mod.rs
+++ b/rslib/src/sync/mod.rs
@@ -21,6 +21,7 @@ use crate::{
     deckconfig::DeckConfSchema11,
     decks::DeckSchema11,
     error::{SyncError, SyncErrorKind},
+    io::atomic_rename,
     notes::Note,
     notetype::{Notetype, NotetypeSchema11},
     prelude::*,
@@ -679,14 +680,7 @@ impl Collection {
         let db = open_and_check_sqlite_file(out_file.path())?;
         db.execute_batch("update col set ls=mod")?;
         drop(db);
-        // overwrite existing collection atomically
-        out_file.as_file().sync_all()?;
-        out_file
-            .persist(&col_path)
-            .map_err(|e| AnkiError::IoError(format!("download save failed: {}", e)))?;
-        if !cfg!(windows) {
-            std::fs::File::open(col_folder)?.sync_all()?;
-        }
+        atomic_rename(out_file, &col_path)?;
 
         Ok(())
     }