Skip to content

Commit

Permalink
respond to review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
cosmicexplorer committed May 8, 2024
1 parent 80b2361 commit 18d5255
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 19 deletions.
4 changes: 2 additions & 2 deletions src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1285,9 +1285,9 @@ impl<'a> Drop for ZipFile<'a> {
/// * `data_start`: set to 0
/// * `external_attributes`: `unix_mode()`: will return None
pub fn read_zipfile_from_stream<'a, R: Read>(reader: &'a mut R) -> ZipResult<Option<ZipFile<'_>>> {
let mut block = vec![0u8; mem::size_of::<ZipLocalEntryBlock>()];
let mut block = [0u8; mem::size_of::<ZipLocalEntryBlock>()];
reader.read_exact(&mut block)?;
let block = block.into_boxed_slice();
let block: Box<[u8]> = block.into();

let signature = spec::Magic::from_le_bytes(
block[..mem::size_of_val(&spec::CENTRAL_DIRECTORY_HEADER_SIGNATURE)]
Expand Down
51 changes: 36 additions & 15 deletions src/spec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![allow(clippy::wrong_self_convention)]
#![macro_use]

use crate::result::{ZipError, ZipResult};
Expand Down Expand Up @@ -86,7 +85,7 @@ macro_rules! to_le {
#[derive(Copy, Clone, Debug)]
#[repr(packed)]
pub struct CDEBlock {
pub magic: Magic,
magic: Magic,
pub disk_number: u16,
pub disk_with_central_directory: u16,
pub number_of_files_on_this_disk: u16,
Expand All @@ -97,6 +96,7 @@ pub struct CDEBlock {
}

impl CDEBlock {
#[allow(clippy::wrong_self_convention)]
#[inline(always)]
fn from_le(mut self) -> Self {
from_le![
Expand Down Expand Up @@ -252,17 +252,26 @@ impl CentralDirectoryEnd {
}
}

/* We always want to make sure we go allllll the way back to the start of the file if
* we can't find it elsewhere. However, our `while` condition doesn't check that. So we
* avoid infinite looping by checking at the end of the loop. */
if window_start == search_upper_bound {
break;
}
debug_assert!(END_WINDOW_SIZE > mem::size_of_val(&CENTRAL_DIRECTORY_END_SIGNATURE));
/* Shift the window by END_WINDOW_SIZE bytes, but make sure to cover matches that
* overlap our nice neat window boundaries! */
window_start = (window_start
/* NB: To catch matches across window boundaries, we need to make our blocks overlap
* by the width of the pattern to match. */
+ mem::size_of_val(&CENTRAL_DIRECTORY_END_SIGNATURE) as u64)
/* This should never happen, but make sure we don't go past the end of the file. */
.min(file_length);
window_start = window_start
.saturating_sub(
/* Shift the window upon each iteration. */
END_WINDOW_SIZE as u64
/* But don't shift all the way -- make sure to catch matches across window
* boundaries! */
- mem::size_of_val(&CENTRAL_DIRECTORY_END_SIGNATURE) as u64,
/* Shift the window upon each iteration so we search END_WINDOW_SIZE bytes at
* once (unless limited by file_length). */
END_WINDOW_SIZE as u64,
)
/* This will never go below the value of `search_upper_bound`, so we have a special
* `if window_start == search_upper_bound` check above. */
Expand All @@ -285,13 +294,14 @@ impl CentralDirectoryEnd {
#[derive(Copy, Clone)]
#[repr(packed)]
pub struct Zip64CDELocatorBlock {
pub magic: Magic,
magic: Magic,
pub disk_with_central_directory: u32,
pub end_of_central_directory_offset: u64,
pub number_of_disks: u32,
}

impl Zip64CDELocatorBlock {
#[allow(clippy::wrong_self_convention)]
#[inline(always)]
fn from_le(mut self) -> Self {
from_le![
Expand Down Expand Up @@ -384,7 +394,7 @@ impl Zip64CentralDirectoryEndLocator {
#[derive(Copy, Clone)]
#[repr(packed)]
pub struct Zip64CDEBlock {
pub magic: Magic,
magic: Magic,
pub record_size: u64,
pub version_made_by: u16,
pub version_needed_to_extract: u16,
Expand All @@ -397,6 +407,7 @@ pub struct Zip64CDEBlock {
}

impl Zip64CDEBlock {
#[allow(clippy::wrong_self_convention)]
#[inline(always)]
fn from_le(mut self) -> Self {
from_le![
Expand Down Expand Up @@ -537,19 +548,29 @@ impl Zip64CentralDirectoryEnd {
results.push((cde, archive_offset));
}

/* We always want to make sure we go allllll the way back to the start of the file if
* we can't find it elsewhere. However, our `while` condition doesn't check that. So we
* avoid infinite looping by checking at the end of the loop. */
if window_start == nominal_offset {
break;
}
debug_assert!(
END_WINDOW_SIZE > mem::size_of_val(&ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE)
);
/* Shift the window by END_WINDOW_SIZE bytes, but make sure to cover matches that
* overlap our nice neat window boundaries! */
window_start = (window_start
/* NB: To catch matches across window boundaries, we need to make our blocks overlap
* by the width of the pattern to match. */
+ mem::size_of_val(&ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE) as u64)
/* This may never happen, but make sure we don't go past the end of the specified
* range. */
.min(search_upper_bound);
window_start = window_start
.saturating_sub(
/* Shift the window upon each iteration. */
END_WINDOW_SIZE as u64
/* But don't shift all the way -- make sure to catch matches across window
* boundaries! */
- mem::size_of_val(&ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE) as u64,
/* Shift the window upon each iteration so we search END_WINDOW_SIZE bytes at
* once (unless limited by search_upper_bound). */
END_WINDOW_SIZE as u64,
)
/* This will never go below the value of `nominal_offset`, so we have a special
* `if window_start == nominal_offset` check above. */
Expand Down Expand Up @@ -651,7 +672,7 @@ mod test {
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[repr(packed)]
pub struct TestBlock {
pub magic: Magic,
magic: Magic,
pub file_name_length: u16,
}

Expand Down
4 changes: 2 additions & 2 deletions src/types.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(clippy::wrong_self_convention)]

//! Types that specify what is contained in a ZIP.
use crate::cp437::FromCp437;
use crate::write::{FileOptionExtension, FileOptions};
Expand Down Expand Up @@ -707,6 +705,7 @@ pub(crate) struct ZipEntryBlock {
}

impl ZipEntryBlock {
#[allow(clippy::wrong_self_convention)]
#[inline(always)]
fn from_le(mut self) -> Self {
from_le![
Expand Down Expand Up @@ -795,6 +794,7 @@ pub(crate) struct ZipLocalEntryBlock {
}

impl ZipLocalEntryBlock {
#[allow(clippy::wrong_self_convention)]
#[inline(always)]
fn from_le(mut self) -> Self {
from_le![
Expand Down

0 comments on commit 18d5255

Please sign in to comment.