fix: zip64 creation of exactly 4GB without large file should crash

.typos.toml

@@ -1,5 +1,6 @@
 [default.extend-identifiers]
 ZIP64_BYTES_THR = "ZIP64_BYTES_THR"
+ZIP64_BYTES_THR_U32 = "ZIP64_BYTES_THR_U32"
 ZIP64_ENTRY_THR = "ZIP64_ENTRY_THR"
 flate2 = "flate2"
 "00ba" = "00ba"

src/read/zipfile.rs

@@ -297,7 +297,7 @@ impl<'a, R: Read + ?Sized> ZipFile<'a, R> {
     /// a new zip archive.
     pub fn options(&self) -> SimpleFileOptions {
         let mut options = SimpleFileOptions::default()
-            .large_file(self.compressed_size().max(self.size()) > ZIP64_BYTES_THR)
+            .large_file(self.compressed_size().max(self.size()) >= ZIP64_BYTES_THR)
             .compression_method(self.compression())
             .unix_permissions(self.unix_mode().unwrap_or(0o644) | ffi::S_IFREG)
             .last_modified_time(

src/spec.rs

@@ -107,6 +107,7 @@ impl Magic {
 /// # }
 ///```
 pub const ZIP64_BYTES_THR: u64 = u32::MAX as u64;
+pub const ZIP64_BYTES_THR_U32: u32 = u32::MAX;
 /// The number of entries within a single zip necessary to allocate a zip64 central
 /// directory record.
 ///

src/types.rs

@@ -510,13 +510,14 @@
     }
     fn clamp_size_field(&self, field: u64) -> Result<u32, std::io::Error> {
         if self.large_file {
-            Ok(spec::ZIP64_BYTES_THR as u32)
+            Ok(spec::ZIP64_BYTES_THR_U32)
         } else {
-            field.min(spec::ZIP64_BYTES_THR).try_into().map_err(|_| {
+            let size: u32 = field.try_into().map_err(|_| {
                 std::io::Error::other(format!(
                     "File size {field} exceeds maximum size for non-ZIP64 files"
                 ))
-            })
+            })?;
+            Ok(size.min(spec::ZIP64_BYTES_THR_U32))
         }
     }
 
@@ -560,22 +561,8 @@
     }
 
     pub(crate) fn block(&self, file_name_raw: &[u8]) -> ZipResult<ZipCentralEntryBlock> {
-        let compressed_size = if self.large_file {
-            spec::ZIP64_BYTES_THR as u32
-        } else {
-            self.compressed_size
-                .min(spec::ZIP64_BYTES_THR)
-                .try_into()
-                .map_err(std::io::Error::other)?
-        };
-        let uncompressed_size = if self.large_file {
-            spec::ZIP64_BYTES_THR as u32
-        } else {
-            self.uncompressed_size
-                .min(spec::ZIP64_BYTES_THR)
-                .try_into()
-                .map_err(std::io::Error::other)?
-        };
+        let compressed_size = self.clamp_size_field(self.compressed_size)?;
+        let uncompressed_size = self.clamp_size_field(self.uncompressed_size)?;
         let offset = self
             .header_start
             .min(spec::ZIP64_BYTES_THR)
@@ -636,8 +623,8 @@
         if self.large_file {
             return self.zip64_data_descriptor_block().write(writer);
         }
-        if self.compressed_size > spec::ZIP64_BYTES_THR
-            || self.uncompressed_size > spec::ZIP64_BYTES_THR
+        if self.compressed_size >= spec::ZIP64_BYTES_THR
+            || self.uncompressed_size >= spec::ZIP64_BYTES_THR
         {
             if auto_large_file {
                 return self.zip64_data_descriptor_block().write(writer);

src/write.rs

@@ -191,6 +191,7 @@ pub(crate) mod zip_writer {
         pub(super) flush_on_finish_file: bool,
         pub(super) seek_possible: bool,
         pub(crate) auto_large_file: bool,
+        pub(crate) internal_error: bool,
     }
 
     impl<W: Write + Seek> Debug for ZipWriter<W> {
@@ -789,7 +790,7 @@ impl<W: Write + Seek> Write for ZipWriter<W> {
                 if let Ok(count) = write_result {
                     self.stats.update(&buf[..count]);
                     // Only perform the expensive large-file check when we first cross the threshold.
-                    if self.stats.bytes_written > spec::ZIP64_BYTES_THR {
+                    if self.stats.bytes_written >= spec::ZIP64_BYTES_THR {
                         let is_large_file = self
                             .files
                             .last()
@@ -798,6 +799,7 @@ impl<W: Write + Seek> Write for ZipWriter<W> {
                             .large_file;
                         if !is_large_file {
                             return Err(if let Err(e) = self.abort_file() {
+                                self.internal_error = true;
                                 let abort_io_err: io::Error = e.into();
                                 io::Error::new(
                                     abort_io_err.kind(),
@@ -806,6 +808,7 @@ impl<W: Write + Seek> Write for ZipWriter<W> {
                                     ),
                                 )
                             } else {
+                                self.internal_error = true;
                                 io::Error::other("Large file option has not been set")
                             });
                         }
@@ -863,6 +866,7 @@ impl<A: Read + Write + Seek> ZipWriter<A> {
             flush_on_finish_file: false,
             seek_possible: true,
             auto_large_file: false,
+            internal_error: false,
         })
     }
 
@@ -1031,6 +1035,7 @@ impl<W: Write + Seek> ZipWriter<W> {
             flush_on_finish_file: false,
             seek_possible: true,
             auto_large_file: false,
+            internal_error: false,
         }
     }
 
@@ -1401,8 +1406,12 @@ impl<W: Write + Seek> ZipWriter<W> {
             if file.is_using_data_descriptor() {
                 file.write_data_descriptor(writer, self.auto_large_file)?;
             } else {
-                file.update_local_file_header(writer, file_name_raw)?;
+                let res = file.update_local_file_header(writer, file_name_raw);
                 writer.seek(SeekFrom::Start(file_end))?;
+                if res.is_err() {
+                    self.internal_error = false;
+                    res?;
+                }
             }
         }
         if self.flush_on_finish_file {
@@ -1918,7 +1927,7 @@ impl<W: Write + Seek> ZipWriter<W> {
         }
 
         let central_directory_size = if is64 {
-            spec::ZIP64_BYTES_THR as u32
+            spec::ZIP64_BYTES_THR_U32
         } else {
             central_size.min(spec::ZIP64_BYTES_THR) as u32
         };
@@ -1992,12 +2001,16 @@ impl<W: Write> ZipWriter<StreamWriter<W>> {
             flush_on_finish_file: false,
             seek_possible: false,
             auto_large_file: false,
+            internal_error: false,
         }
     }
 }
 
 impl<W: Write + Seek> Drop for ZipWriter<W> {
     fn drop(&mut self) {
+        if self.internal_error {
+            return;
+        }
         if !self.inner.is_closed()
             && let Err(e) = self.finalize()
         {
@@ -2425,16 +2438,13 @@ impl ZipFileData {
         ))?;
         writer.write_u32_le(self.crc32)?;
         if self.large_file {
-            writer.write_u32_le(spec::ZIP64_BYTES_THR as u32)?;
-            writer.write_u32_le(spec::ZIP64_BYTES_THR as u32)?;
+            writer.write_u32_le(spec::ZIP64_BYTES_THR_U32)?;
+            writer.write_u32_le(spec::ZIP64_BYTES_THR_U32)?;
 
             self.update_local_zip64_extra_field(writer, file_name_raw)?;
-
-            // self.compressed_size = spec::ZIP64_BYTES_THR;
-            // self.uncompressed_size = spec::ZIP64_BYTES_THR;
         } else {
             // check compressed size as well as it can also be slightly larger than uncompressed size
-            if self.compressed_size > spec::ZIP64_BYTES_THR {
+            if self.compressed_size >= spec::ZIP64_BYTES_THR {
                 return Err(ZipError::Io(std::io::Error::other(
                     "large_file(true) option has not been set",
                 )));

tests/append_near_4gb.rs

@@ -1,14 +1,46 @@
+//! Tests related to big zip file
+
 /// Only on little endian because we cannot use fs with miri CI
 #[cfg(all(target_endian = "little", not(miri)))]
-fn write_data(w: &mut dyn std::io::Write, size: usize) {
+fn write_data(w: &mut dyn std::io::Write, size: usize) -> Result<(), std::io::Error> {
     let chunks = 1 << 20; // 1MB chunks
     let mut written = 0;
     let buf = vec![0x21; chunks];
     while written < size {
         let to_write = (size - written).min(chunks);
-        w.write_all(&buf[..to_write]).unwrap();
+        w.write_all(&buf[..to_write])?;
         written += to_write;
     }
+    Ok(())
+}
+
+/// Only on little endian because we cannot use fs with miri CI
+#[cfg(all(target_endian = "little", not(miri)))]
+#[test]
+fn test_append_4gb_without_large_file() {
+    use std::fs::File;
+    use tempfile::tempdir;
+    use zip::ZipWriter;
+    use zip::write::SimpleFileOptions;
+
+    let dir = tempdir().unwrap();
+    let path = dir
+        .path()
+        .join("debug_large_without_large_file_options.zip");
+    //let path = std::path::PathBuf::from("debug_large_without_large_file_options.zip");
+
+    let file = File::create(&path).unwrap();
+    let mut writer = ZipWriter::new(file);
+
+    let opts = SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
+
+    writer.start_file_from_path("close_to_4gb", opts).unwrap();
+
+    // Write a file that's 4GB
+    let size = u32::MAX;
+    let write_result = write_data(&mut writer, size as usize); // check is error
+
+    assert!(write_result.is_err());
 }
 
 /// Only on little endian because we cannot use fs with miri CI
@@ -33,12 +65,12 @@ fn test_append_near_4gb() {
         writer.start_file_from_path("close_to_4gb", opts).unwrap();
 
         // Write a file that's just under 4GB (4GB - 1 byte)
-        let size = u32::MAX;
-        write_data(&mut writer, size as usize);
+        let size = u32::MAX - 1;
+        write_data(&mut writer, size as usize).unwrap();
 
         // Add a small file
         writer.start_file_from_path("small_file", opts).unwrap();
-        write_data(&mut writer, 1024);
+        write_data(&mut writer, 1024).unwrap();
 
         writer.finish().unwrap();
     }
@@ -52,7 +84,7 @@ fn test_append_near_4gb() {
 
         // Add another small file
         writer.start_file_from_path("appended_file", opts).unwrap();
-        write_data(&mut writer, 1024);
+        write_data(&mut writer, 1024).unwrap();
 
         writer.finish().unwrap();
     }
@@ -108,12 +140,12 @@ fn test_append_near_4gb_with_1gb_files() {
 
             // Write a file that's 1 GB
             let size = 1u64 << 30;
-            write_data(&mut writer, size as usize);
+            write_data(&mut writer, size as usize).unwrap();
         }
 
         // Add a small file
         writer.start_file_from_path("small_file", opts).unwrap();
-        write_data(&mut writer, 1024);
+        write_data(&mut writer, 1024).unwrap();
 
         writer.finish().unwrap();
     }
@@ -127,7 +159,7 @@ fn test_append_near_4gb_with_1gb_files() {
 
         // Add another small file
         writer.start_file_from_path("appended_file", opts).unwrap();
-        write_data(&mut writer, 1024);
+        write_data(&mut writer, 1024).unwrap();
 
         writer.finish().unwrap();
     }
@@ -195,7 +227,7 @@ fn test_append_with_large_file_flag() {
             .large_file(true); // Force ZIP64 format
 
         writer.start_file_from_path("file1", opts).unwrap();
-        write_data(&mut writer, 1024);
+        write_data(&mut writer, 1024).unwrap();
 
         writer.finish().unwrap();
     }
@@ -209,7 +241,7 @@ fn test_append_with_large_file_flag() {
 
         // Add another file
         writer.start_file_from_path("file2", opts).unwrap();
-        write_data(&mut writer, 1024);
+        write_data(&mut writer, 1024).unwrap();
 
         writer.finish().unwrap();
     }