Merge pull request #106 from hidez8891/dev/ziped

fix: UTF-8 flag could be set for non-UTF8 filenames

src/write.rs

@@ -13,6 +13,7 @@ use crate::types::{ffi, AesVendorVersion, DateTime, System, ZipFileData, DEFAULT
 use core::num::NonZeroU64;
 use crc32fast::Hasher;
 use indexmap::IndexMap;
+use std::borrow::ToOwned;
 use std::default::Default;
 use std::io;
 use std::io::prelude::*;
@@ -723,14 +724,15 @@ impl<W: Write + Seek> ZipWriter<W> {
     }
 
     /// Start a new file for with the requested options.
-    fn start_entry<S, T: FileOptionExtension>(
+    fn start_entry<S, SToOwned, T: FileOptionExtension>(
         &mut self,
         name: S,
         options: FileOptions<T>,
         raw_values: Option<ZipRawValues>,
     ) -> ZipResult<()>
     where
-        S: Into<Box<str>>,
+        S: Into<Box<str>> + ToOwned<Owned = SToOwned>,
+        SToOwned: Into<Box<str>>,
     {
         self.finish_file()?;
 
@@ -796,8 +798,8 @@ impl<W: Write + Seek> ZipWriter<W> {
                 crc32: raw_values.crc32,
                 compressed_size: raw_values.compressed_size,
                 uncompressed_size: raw_values.uncompressed_size,
-                file_name: name.into(),
+                file_name: name.to_owned().into(), // Never used for saving, but used as map key in insert_file_data()
-                file_name_raw: vec![].into_boxed_slice(), // Never used for saving
+                file_name_raw: name.into().bytes().collect(),
                 extra_field,
                 central_extra_field: options.extended_options.central_extra_data().cloned(),
                 file_comment: String::with_capacity(0).into_boxed_str(),
@@ -815,15 +817,15 @@ impl<W: Write + Seek> ZipWriter<W> {
             let index = self.insert_file_data(file)?;
             let file = &mut self.files[index];
             let writer = self.inner.get_plain();
+            // local file header signature
             writer.write_u32_le(spec::LOCAL_FILE_HEADER_SIGNATURE)?;
             // version needed to extract
             writer.write_u16_le(file.version_needed())?;
             // general purpose bit flag
-            let flag = if !file.file_name.is_ascii() {
+            let is_utf8 = std::str::from_utf8(&file.file_name_raw).is_ok();
-                1u16 << 11
+            let is_ascii = file.file_name_raw.is_ascii();
-            } else {
+            let flag = if is_utf8 && !is_ascii { 1u16 << 11 } else { 0 }
-                0
+                | if file.encrypted { 1u16 << 0 } else { 0 };
-            } | if file.encrypted { 1u16 << 0 } else { 0 };
             writer.write_u16_le(flag)?;
             // Compression method
             #[allow(deprecated)]
@@ -842,7 +844,7 @@ impl<W: Write + Seek> ZipWriter<W> {
                 writer.write_u32_le(file.uncompressed_size as u32)?;
             }
             // file name length
-            writer.write_u16_le(file.file_name.as_bytes().len() as u16)?;
+            writer.write_u16_le(file.file_name_raw.len() as u16)?;
             // extra field length
             let mut extra_field_length = file.extra_field_len();
             if file.large_file {
@@ -855,7 +857,7 @@ impl<W: Write + Seek> ZipWriter<W> {
             let extra_field_length = extra_field_length as u16;
             writer.write_u16_le(extra_field_length)?;
             // file name
-            writer.write_all(file.file_name.as_bytes())?;
+            writer.write_all(&file.file_name_raw)?;
             // zip64 extra field
             if file.large_file {
                 write_local_zip64_extra_field(writer, file)?;
@@ -1053,13 +1055,14 @@ impl<W: Write + Seek> ZipWriter<W> {
     /// same name as a file already in the archive.
     ///
     /// The data should be written using the [`Write`] implementation on this [`ZipWriter`]
-    pub fn start_file<S, T: FileOptionExtension>(
+    pub fn start_file<S, T: FileOptionExtension, SToOwned>(
         &mut self,
         name: S,
         mut options: FileOptions<T>,
     ) -> ZipResult<()>
     where
-        S: Into<Box<str>>,
+        S: Into<Box<str>> + ToOwned<Owned = SToOwned>,
+        SToOwned: Into<Box<str>>,
     {
         Self::normalize_options(&mut options);
         let make_new_self = self.inner.prepare_next_writer(
@@ -1188,9 +1191,10 @@ impl<W: Write + Seek> ZipWriter<W> {
     ///     Ok(())
     /// }
     /// ```
-    pub fn raw_copy_file_rename<S>(&mut self, mut file: ZipFile, name: S) -> ZipResult<()>
+    pub fn raw_copy_file_rename<S, SToOwned>(&mut self, mut file: ZipFile, name: S) -> ZipResult<()>
     where
-        S: Into<Box<str>>,
+        S: Into<Box<str>> + ToOwned<Owned = SToOwned>,
+        SToOwned: Into<Box<str>>,
     {
         let mut options = SimpleFileOptions::default()
             .large_file(file.compressed_size().max(file.size()) > spec::ZIP64_BYTES_THR)
@@ -1323,14 +1327,15 @@ impl<W: Write + Seek> ZipWriter<W> {
     /// implementations may materialize a symlink as a regular file, possibly with the
     /// content incorrectly set to the symlink target. For maximum portability, consider
     /// storing a regular file instead.
-    pub fn add_symlink<N, T, E: FileOptionExtension>(
+    pub fn add_symlink<N, NToOwned, T, E: FileOptionExtension>(
         &mut self,
         name: N,
         target: T,
         mut options: FileOptions<E>,
     ) -> ZipResult<()>
     where
-        N: Into<Box<str>>,
+        N: Into<Box<str>> + ToOwned<Owned = NToOwned>,
+        NToOwned: Into<Box<str>>,
         T: Into<Box<str>>,
     {
         if options.permissions.is_none() {
@@ -1450,7 +1455,8 @@ impl<W: Write + Seek> ZipWriter<W> {
         self.finish_file()?;
         let src_index = self.index_by_name(src_name)?;
         let mut dest_data = self.files[src_index].to_owned();
-        dest_data.file_name = dest_name.into();
+        dest_data.file_name = dest_name.to_string().into();
+        dest_data.file_name_raw = dest_name.to_string().into_bytes().into();
         self.insert_file_data(dest_data)?;
         Ok(())
     }
@@ -1803,12 +1809,11 @@ fn write_central_directory_header<T: Write>(writer: &mut T, file: &ZipFileData)
     writer.write_u16_le(version_made_by)?;
     // version needed to extract
     writer.write_u16_le(file.version_needed())?;
-    // general purpose bit flag
+    // general puprose bit flag
-    let flag = if !file.file_name.is_ascii() {
+    let is_utf8 = std::str::from_utf8(&file.file_name_raw).is_ok();
-        1u16 << 11
+    let is_ascii = file.file_name_raw.is_ascii();
-    } else {
+    let flag = if is_utf8 && !is_ascii { 1u16 << 11 } else { 0 }
-        0
+        | if file.encrypted { 1u16 << 0 } else { 0 };
-    } | if file.encrypted { 1u16 << 0 } else { 0 };
     writer.write_u16_le(flag)?;
     // compression method
     #[allow(deprecated)]
@@ -1823,7 +1828,7 @@ fn write_central_directory_header<T: Write>(writer: &mut T, file: &ZipFileData)
     // uncompressed size
     writer.write_u32_le(file.uncompressed_size.min(spec::ZIP64_BYTES_THR) as u32)?;
     // file name length
-    writer.write_u16_le(file.file_name.as_bytes().len() as u16)?;
+    writer.write_u16_le(file.file_name_raw.len() as u16)?;
     // extra field length
     writer.write_u16_le(
         zip64_extra_field_length
@@ -1841,7 +1846,7 @@ fn write_central_directory_header<T: Write>(writer: &mut T, file: &ZipFileData)
     // relative offset of local header
     writer.write_u32_le(file.header_start.min(spec::ZIP64_BYTES_THR) as u32)?;
     // file name
-    writer.write_all(file.file_name.as_bytes())?;
+    writer.write_all(&file.file_name_raw)?;
     // zip64 extra field
     writer.write_all(&zip64_extra_field[..zip64_extra_field_length as usize])?;
     // extra field
@@ -1906,7 +1911,7 @@ fn update_local_zip64_extra_field<T: Write + Seek>(
     writer: &mut T,
     file: &ZipFileData,
 ) -> ZipResult<()> {
-    let zip64_extra_field = file.header_start + 30 + file.file_name.as_bytes().len() as u64;
+    let zip64_extra_field = file.header_start + 30 + file.file_name_raw.len() as u64;
     writer.seek(SeekFrom::Start(zip64_extra_field + 4))?;
     writer.write_u64_le(file.uncompressed_size)?;
     writer.write_u64_le(file.compressed_size)?;
@@ -2136,6 +2141,64 @@ mod test {
     const SECOND_FILENAME: &str = "different_name.xyz";
     const THIRD_FILENAME: &str = "third_name.xyz";
 
+    #[test]
+    fn write_non_utf8() {
+        let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()));
+        let options = FileOptions {
+            compression_method: CompressionMethod::Stored,
+            compression_level: None,
+            last_modified_time: DateTime::default(),
+            permissions: Some(33188),
+            large_file: false,
+            encrypt_with: None,
+            extended_options: (),
+            alignment: 1,
+            #[cfg(feature = "deflate-zopfli")]
+            zopfli_buffer_size: None,
+        };
+
+        // GB18030
+        // "中文" = [214, 208, 206, 196]
+        let filename = unsafe { String::from_utf8_unchecked(vec![214, 208, 206, 196]) };
+        writer.start_file(filename, options).unwrap();
+        writer.write_all(b"encoding GB18030").unwrap();
+
+        // SHIFT_JIS
+        // "日文" = [147, 250, 149, 182]
+        let filename = unsafe { String::from_utf8_unchecked(vec![147, 250, 149, 182]) };
+        writer.start_file(filename, options).unwrap();
+        writer.write_all(b"encoding SHIFT_JIS").unwrap();
+
+        let result = writer.finish().unwrap();
+        assert_eq!(result.get_ref().len(), 224);
+
+        let mut v = Vec::new();
+        v.extend_from_slice(include_bytes!("../tests/data/non_utf8.zip"));
+
+        // FIXME: Update the actual file once https://github.com/zip-rs/zip2/pull/106 is merged
+        v[4] = 10;
+        v[54] = 10;
+        v[108] = 10;
+        v[158] = 10;
+
+        assert_eq!(result.get_ref(), &v);
+    }
+
+    #[test]
+    fn path_to_string() {
+        let mut path = std::path::PathBuf::new();
+        #[cfg(windows)]
+        path.push(r"C:\");
+        #[cfg(unix)]
+        path.push("/");
+        path.push("windows");
+        path.push("..");
+        path.push(".");
+        path.push("system32");
+        let path_str = super::path_to_string(&path);
+        assert_eq!(&*path_str, "system32");
+    }
+
     #[test]
     fn test_shallow_copy() {
         let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()));