From fa045ad4c54c2b82532217447d290f5b796952fc Mon Sep 17 00:00:00 2001
From: Chris Hennick <hennickc@amazon.com>
Date: Sun, 21 May 2023 11:26:33 -0700
Subject: [PATCH] Bug fix for abort_file when deleting an entry that isn't the
 last

---
 CHANGELOG.md                    |  6 ++++++
 fuzz/fuzz_targets/fuzz_write.rs |  4 ++++
 src/write.rs                    | 35 ++++++++++++++++++++++++++++++---
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0f46256d..a4f5f049 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -165,3 +165,9 @@
 ### Merged from upstream
 
  - Uses the `aes::cipher::KeyInit` trait from `aes` 0.8.2 where appropriate.
+
+### Fixed
+
+ - Calling `abort_file()` no longer corrupts the archive if called on a
+   shallow copy of a remaining file, or on an archive whose CDR entries are out
+   of sequence. However, it may leave an unused entry in the archive.
\ No newline at end of file
diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs
index 2e36069e..13b536b6 100644
--- a/fuzz/fuzz_targets/fuzz_write.rs
+++ b/fuzz/fuzz_targets/fuzz_write.rs
@@ -25,6 +25,7 @@ pub enum BasicFileOperation {
 pub struct FileOperation {
     basic: BasicFileOperation,
     name: String,
+    abort: bool,
     reopen: bool,
 }
 
@@ -70,6 +71,9 @@ fn do_operation<T>(writer: &mut RefCell<zip_next::ZipWriter<T>>,
             writer.borrow_mut().deep_copy_file(&base_name, &name)?;
         }
     }
+    if operation.abort {
+        writer.abort_file().unwrap();
+    }
     if operation.reopen {
         let new_writer = zip_next::ZipWriter::new_append(writer.borrow_mut().finish().unwrap()).unwrap();
         *writer = new_writer.into();
diff --git a/src/write.rs b/src/write.rs
index f7db7441..5557c185 100644
--- a/src/write.rs
+++ b/src/write.rs
@@ -713,9 +713,19 @@ impl<W: Write + Seek> ZipWriter<W> {
             .inner
             .prepare_next_writer(CompressionMethod::Stored, None)?;
         self.inner.switch_to(make_plain_writer)?;
-        self.inner
-            .get_plain()
-            .seek(SeekFrom::Start(last_file.header_start))?;
+
+        // Make sure this is the last file, and that no shallow copies of it remain; otherwise we'd
+        // overwrite a valid file and corrupt the archive
+        if !self.writing_to_file
+            && self
+                .files
+                .iter()
+                .all(|file| file.data_start.load() < last_file.data_start.load())
+        {
+            self.inner
+                .get_plain()
+                .seek(SeekFrom::Start(last_file.header_start))?;
+        }
         self.writing_to_file = false;
         Ok(())
     }
@@ -1774,6 +1784,25 @@ mod test {
         let _ = ZipArchive::new(zip).unwrap();
         Ok(())
     }
+
+    #[test]
+    fn remove_shallow_copy_keeps_original() -> ZipResult<()> {
+        let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()));
+        writer
+            .start_file("original", FileOptions::default())
+            .unwrap();
+        writer.write_all(RT_TEST_TEXT.as_bytes()).unwrap();
+        writer
+            .shallow_copy_file("original", "shallow_copy")
+            .unwrap();
+        writer.abort_file().unwrap();
+        let mut zip = ZipArchive::new(writer.finish().unwrap()).unwrap();
+        let mut file = zip.by_name("original").unwrap();
+        let mut contents = Vec::new();
+        file.read_to_end(&mut contents).unwrap();
+        assert_eq!(RT_TEST_TEXT.as_bytes(), contents);
+        Ok(())
+    }
 }
 
 #[cfg(not(feature = "unreserved"))]