fix: first pass on maintainer comments

Author: RisaI

src/read.rs

@@ -8,7 +8,7 @@ use crate::crc32::Crc32Reader;
 use crate::extra_fields::{ExtendedTimestamp, ExtraField};
 use crate::read::zip_archive::{Shared, SharedBuilder};
 use crate::result::{ZipError, ZipResult};
-use crate::spec::{self, CentralDirectoryEndInfo, FixedSizeBlock, Pod};
+use crate::spec::{self, CentralDirectoryEndInfo, DataAndPosition, FixedSizeBlock, Pod};
 use crate::types::{
     AesMode, AesVendorVersion, DateTime, System, ZipCentralEntryBlock, ZipFileData,
     ZipLocalEntryBlock,
@@ -450,7 +450,7 @@ impl<'a> TryFrom<&'a CentralDirectoryEndInfo> for CentralDirectoryInfo {
 
         let (relative_cd_offset, number_of_files, disk_number, disk_with_central_directory) =
             match &value.eocd64 {
-                Some((eocd64, _)) => {
+                Some(DataAndPosition { data: eocd64, .. }) => {
                     if eocd64.number_of_files_on_this_disk > eocd64.number_of_files {
                         return Err(InvalidArchive(
                         "ZIP64 footer indicates more files on this disk than in the whole archive",
@@ -469,10 +469,10 @@ impl<'a> TryFrom<&'a CentralDirectoryEndInfo> for CentralDirectoryInfo {
                     )
                 }
                 _ => (
-                    value.eocd.0.central_directory_offset as u64,
-                    value.eocd.0.number_of_files_on_this_disk as usize,
-                    value.eocd.0.disk_number as u32,
-                    value.eocd.0.disk_with_central_directory as u32,
+                    value.eocd.data.central_directory_offset as u64,
+                    value.eocd.data.number_of_files_on_this_disk as usize,
+                    value.eocd.data.disk_number as u32,
+                    value.eocd.data.disk_with_central_directory as u32,
                 ),
             };
 
@@ -608,8 +608,8 @@ impl<R: Read + Seek> ZipArchive<R> {
         let shared = Self::read_central_header(info, config, reader)?;
 
         Ok(shared.build(
-            cde.eocd.0.zip_file_comment,
-            cde.eocd64.map(|(v, _)| v.extensible_data_sector),
+            cde.eocd.data.zip_file_comment,
+            cde.eocd64.map(|v| v.data.extensible_data_sector),
         ))
     }
 
@@ -1551,7 +1551,7 @@ pub fn read_zipfile_from_stream<'a, R: Read>(reader: &'a mut R) -> ZipResult<Opt
     let mut block = ZipLocalEntryBlock::zeroed();
     reader.read_exact(block.as_bytes_mut())?;
 
-    match block.magic().from_le() {
+    match block.magic() {
         spec::Magic::LOCAL_FILE_HEADER_SIGNATURE => (),
         spec::Magic::CENTRAL_DIRECTORY_HEADER_SIGNATURE => return Ok(None),
         _ => return Err(ZipLocalEntryBlock::WRONG_MAGIC_ERROR),

src/read/magic_finder.rs

@@ -17,22 +17,21 @@ pub struct MagicFinder<'a> {
 
 impl<'a> MagicFinder<'a> {
     /// Create a new magic bytes finder to look within specific bounds.
-    pub fn new(magic_bytes: &'a [u8], bounds: (u64, u64)) -> Self {
+    pub fn new(magic_bytes: &'a [u8], start_inclusive: u64, end_exclusive: u64) -> Self {
         const BUFFER_SIZE: usize = 2048;
 
         // Smaller buffer size would be unable to locate bytes.
         // Equal buffer size would stall (the window could not be moved).
-        debug_assert!(BUFFER_SIZE > magic_bytes.len());
+        debug_assert!(BUFFER_SIZE >= magic_bytes.len());
 
         Self {
             buffer: vec![0; BUFFER_SIZE].into_boxed_slice(),
             finder: FinderRev::new(magic_bytes),
-            cursor: bounds
-                .1
+            cursor: end_exclusive
                 .saturating_sub(BUFFER_SIZE as u64)
-                .clamp(bounds.0, bounds.1),
+                .clamp(start_inclusive, end_exclusive),
             mid_buffer_offset: None,
-            bounds,
+            bounds: (start_inclusive, end_exclusive),
         }
     }
 
@@ -109,7 +108,7 @@ impl<'a> MagicFinder<'a> {
             /* Move cursor to the next chunk, cover magic at boundary by shifting by needle length. */
             self.cursor = self
                 .cursor
-                .saturating_add(self.finder.needle().len() as u64)
+                .saturating_add(self.finder.needle().len() as u64 - 1)
                 .saturating_sub(self.buffer.len() as u64)
                 .clamp(self.bounds.0, self.bounds.1);
         }
@@ -139,7 +138,7 @@ impl<'a> OptimisticMagicFinder<'a> {
     /// Create a new empty optimistic magic bytes finder.
     pub fn new_empty() -> Self {
         Self {
-            inner: MagicFinder::new(&[], (0, 0)),
+            inner: MagicFinder::new(&[], 0, 0),
             initial_guess: None,
         }
     }
@@ -183,6 +182,9 @@ impl<'a> OptimisticMagicFinder<'a> {
             // If a match is not found, but the initial guess was mandatory, return an error.
             if mandatory {
                 return Ok(None);
+            } else {
+                // If the initial guess was not mandatory, remove it, as it was not found.
+                self.initial_guess.take();
             }
         }
 

src/spec.rs

@@ -290,7 +290,7 @@ pub(crate) struct Zip32CentralDirectoryEnd {
 }
 
 impl Zip32CentralDirectoryEnd {
-    fn block_and_comment(self) -> ZipResult<(Zip32CDEBlock, Box<[u8]>)> {
+    fn try_into_block_and_comment(self) -> ZipResult<(Zip32CDEBlock, Box<[u8]>)> {
         let Self {
             disk_number,
             disk_with_central_directory,
@@ -344,7 +344,7 @@ impl Zip32CentralDirectoryEnd {
     }
 
     pub fn write<T: Write>(self, writer: &mut T) -> ZipResult<()> {
-        let (block, comment) = self.block_and_comment()?;
+        let (block, comment) = self.try_into_block_and_comment()?;
         block.write(writer)?;
         writer.write_all(&comment)?;
         Ok(())
@@ -513,7 +513,7 @@ impl Zip64CentralDirectoryEnd {
         })
     }
 
-    pub fn block_and_comment(self) -> (Zip64CDEBlock, Box<[u8]>) {
+    pub fn into_block_and_comment(self) -> (Zip64CDEBlock, Box<[u8]>) {
         let Self {
             record_size,
             version_made_by,
@@ -545,16 +545,31 @@ impl Zip64CentralDirectoryEnd {
     }
 
     pub fn write<T: Write>(self, writer: &mut T) -> ZipResult<()> {
-        let (block, comment) = self.block_and_comment();
+        let (block, comment) = self.into_block_and_comment();
         block.write(writer)?;
         writer.write_all(&comment)?;
         Ok(())
     }
 }
 
+pub(crate) struct DataAndPosition<T> {
+    pub data: T,
+    #[allow(dead_code)]
+    pub position: u64,
+}
+
+impl<T> From<(T, u64)> for DataAndPosition<T> {
+    fn from(value: (T, u64)) -> Self {
+        Self {
+            data: value.0,
+            position: value.1,
+        }
+    }
+}
+
 pub(crate) struct CentralDirectoryEndInfo {
-    pub eocd: (Zip32CentralDirectoryEnd, u64),
-    pub eocd64: Option<(Zip64CentralDirectoryEnd, u64)>,
+    pub eocd: DataAndPosition<Zip32CentralDirectoryEnd>,
+    pub eocd64: Option<DataAndPosition<Zip64CentralDirectoryEnd>>,
 
     pub archive_offset: u64,
 }
@@ -580,7 +595,7 @@ pub fn find_central_directory<R: Read + Seek>(
     let file_length = reader.seek(io::SeekFrom::End(0))?;
 
     // Instantiate the mandatory finder
-    let mut eocd_finder = MagicFinder::new(&EOCD_SIG_BYTES, (0, file_length));
+    let mut eocd_finder = MagicFinder::new(&EOCD_SIG_BYTES, 0, file_length);
     let mut subfinder: Option<OptimisticMagicFinder<'static>> = None;
 
     // Keep the last errors for cases of improper EOCD instances.
@@ -612,7 +627,7 @@ pub fn find_central_directory<R: Read + Seek>(
             // If the archive is empty, there is nothing more to be checked, the archive is correct.
             if eocd.number_of_files == 0 {
                 return Ok(CentralDirectoryEndInfo {
-                    eocd: (eocd, eocd_offset),
+                    eocd: (eocd, eocd_offset).into(),
                     eocd64: None,
                     archive_offset: eocd_offset - relative_cd_offset,
                 });
@@ -646,7 +661,7 @@ pub fn find_central_directory<R: Read + Seek>(
                 let archive_offset = cd_offset - relative_cd_offset;
 
                 return Ok(CentralDirectoryEndInfo {
-                    eocd: (eocd, eocd_offset),
+                    eocd: (eocd, eocd_offset).into(),
                     eocd64: None,
                     archive_offset,
                 });
@@ -737,8 +752,8 @@ pub fn find_central_directory<R: Read + Seek>(
             ) {
                 Ok(eocd64) => {
                     return Ok(CentralDirectoryEndInfo {
-                        eocd: (eocd, eocd_offset),
-                        eocd64: Some((eocd64, eocd64_offset)),
+                        eocd: (eocd, eocd_offset).into(),
+                        eocd64: Some((eocd64, eocd64_offset).into()),
                         archive_offset,
                     })
                 }