diff --git a/src/read.rs b/src/read.rs index 830a5851..963b5265 100644 --- a/src/read.rs +++ b/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, FixedSizeBlock, Zip32CentralDirectoryEnd, ZIP64_ENTRY_THR}; +use crate::spec::{self, FixedSizeBlock, Pod, Zip32CentralDirectoryEnd, ZIP64_ENTRY_THR}; use crate::types::{ AesMode, AesVendorVersion, DateTime, System, ZipCentralEntryBlock, ZipFileData, ZipLocalEntryBlock, @@ -1760,19 +1760,17 @@ pub fn read_zipfile_from_stream<'a, R: Read>(reader: &'a mut R) -> ZipResult()]; - reader.read_exact(&mut block)?; - let block: Box<[u8]> = block.into(); - let signature = spec::Magic::from_first_le_bytes(&block); + let mut block = ZipLocalEntryBlock::zeroed(); + reader.read_exact(block.as_bytes_mut())?; - match signature { + match block.magic().from_le() { spec::Magic::LOCAL_FILE_HEADER_SIGNATURE => (), spec::Magic::CENTRAL_DIRECTORY_HEADER_SIGNATURE => return Ok(None), _ => return Err(ZipLocalEntryBlock::WRONG_MAGIC_ERROR), } - let block = ZipLocalEntryBlock::interpret(&block)?; + let block = block.from_le(); let mut result = ZipFileData::from_local_block(block, reader)?; diff --git a/src/spec.rs b/src/spec.rs index 8a3bdf90..80cbdee0 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -2,11 +2,11 @@ use crate::result::{ZipError, ZipResult}; use core::mem; -use core::mem::align_of; use memchr::memmem::FinderRev; use std::io; use std::io::prelude::*; use std::rc::Rc; +use std::slice; /// "Magic" header values used in the zip spec to locate metadata records. /// @@ -26,12 +26,6 @@ impl Magic { Self(u32::from_le_bytes(bytes)) } - #[inline(always)] - pub fn from_first_le_bytes(data: &[u8]) -> Self { - let first_bytes: [u8; 4] = data[..mem::size_of::()].try_into().unwrap(); - Self::from_le_bytes(first_bytes) - } - #[inline(always)] pub const fn to_le_bytes(self) -> [u8; 4] { self.0.to_le_bytes() @@ -97,64 +91,56 @@ impl ExtraFieldMagic { pub const ZIP64_BYTES_THR: u64 = u32::MAX as u64; pub const ZIP64_ENTRY_THR: usize = u16::MAX as usize; -pub(crate) trait FixedSizeBlock: Sized + Copy { +/// # Safety +/// +/// - No padding/uninit bytes +/// - All bytes patterns must be valid +/// - No cell, pointers +/// +/// See `bytemuck::Pod` for more details. +pub(crate) unsafe trait Pod: Copy + 'static { + #[inline] + fn zeroed() -> Self { + unsafe { mem::zeroed() } + } + + #[inline] + fn as_bytes(&self) -> &[u8] { + unsafe { slice::from_raw_parts(self as *const Self as *const u8, mem::size_of::()) } + } + + #[inline] + fn as_bytes_mut(&mut self) -> &mut [u8] { + unsafe { slice::from_raw_parts_mut(self as *mut Self as *mut u8, mem::size_of::()) } + } +} + +pub(crate) trait FixedSizeBlock: Pod { const MAGIC: Magic; fn magic(self) -> Magic; const WRONG_MAGIC_ERROR: ZipError; - /* TODO: use smallvec? */ - fn interpret(bytes: &[u8]) -> ZipResult { - if bytes.len() != mem::size_of::() { - return Err(ZipError::InvalidArchive("Block is wrong size")); - } - let block_ptr: *const Self = bytes.as_ptr().cast(); + #[allow(clippy::wrong_self_convention)] + fn from_le(self) -> Self; - // If alignment could be more than 1, we'd have to use read_unaligned() below - debug_assert_eq!(align_of::(), 1); + fn parse(reader: &mut R) -> ZipResult { + let mut block = Self::zeroed(); + reader.read_exact(block.as_bytes_mut())?; + let block = Self::from_le(block); - let block = unsafe { block_ptr.read() }.from_le(); if block.magic() != Self::MAGIC { return Err(Self::WRONG_MAGIC_ERROR); } Ok(block) } - #[allow(clippy::wrong_self_convention)] - fn from_le(self) -> Self; - - fn parse(reader: &mut T) -> ZipResult { - let mut block = vec![0u8; mem::size_of::()].into_boxed_slice(); - reader.read_exact(&mut block)?; - Self::interpret(&block) - } - - fn encode(self) -> Box<[u8]> { - self.to_le().serialize() - } - fn to_le(self) -> Self; - /* TODO: use Box<[u8; mem::size_of::()]> when generic_const_exprs are stabilized! */ - fn serialize(self) -> Box<[u8]> { - /* TODO: use Box::new_zeroed() when stabilized! */ - /* TODO: also consider using smallvec! */ - - // If alignment could be more than 1, we'd have to use write_unaligned() below - debug_assert_eq!(align_of::(), 1); - - let mut out_block = vec![0u8; mem::size_of::()].into_boxed_slice(); - let out_ptr: *mut Self = out_block.as_mut_ptr().cast(); - unsafe { - out_ptr.write(self); - } - out_block - } - fn write(self, writer: &mut T) -> ZipResult<()> { - let block = self.encode(); - writer.write_all(&block)?; + let block = self.to_le(); + writer.write_all(block.as_bytes())?; Ok(()) } } @@ -206,7 +192,7 @@ macro_rules! to_and_from_le { } #[derive(Copy, Clone, Debug)] -#[repr(packed)] +#[repr(packed, C)] pub(crate) struct Zip32CDEBlock { magic: Magic, pub disk_number: u16, @@ -218,6 +204,8 @@ pub(crate) struct Zip32CDEBlock { pub zip_file_comment_length: u16, } +unsafe impl Pod for Zip32CDEBlock {} + impl FixedSizeBlock for Zip32CDEBlock { const MAGIC: Magic = Magic::CENTRAL_DIRECTORY_END_SIGNATURE; @@ -395,7 +383,7 @@ impl Zip32CentralDirectoryEnd { } #[derive(Copy, Clone)] -#[repr(packed)] +#[repr(packed, C)] pub(crate) struct Zip64CDELocatorBlock { magic: Magic, pub disk_with_central_directory: u32, @@ -403,6 +391,8 @@ pub(crate) struct Zip64CDELocatorBlock { pub number_of_disks: u32, } +unsafe impl Pod for Zip64CDELocatorBlock {} + impl FixedSizeBlock for Zip64CDELocatorBlock { const MAGIC: Magic = Magic::ZIP64_CENTRAL_DIRECTORY_END_LOCATOR_SIGNATURE; @@ -465,7 +455,7 @@ impl Zip64CentralDirectoryEndLocator { } #[derive(Copy, Clone)] -#[repr(packed)] +#[repr(packed, C)] pub(crate) struct Zip64CDEBlock { magic: Magic, pub record_size: u64, @@ -479,6 +469,8 @@ pub(crate) struct Zip64CDEBlock { pub central_directory_offset: u64, } +unsafe impl Pod for Zip64CDEBlock {} + impl FixedSizeBlock for Zip64CDEBlock { const MAGIC: Magic = Magic::ZIP64_CENTRAL_DIRECTORY_END_SIGNATURE; @@ -668,12 +660,14 @@ mod test { use std::io::Cursor; #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] - #[repr(packed)] + #[repr(packed, C)] pub struct TestBlock { magic: Magic, pub file_name_length: u16, } + unsafe impl Pod for TestBlock {} + impl FixedSizeBlock for TestBlock { const MAGIC: Magic = Magic::literal(0x01111); diff --git a/src/types.rs b/src/types.rs index 91031a08..c1b7adce 100644 --- a/src/types.rs +++ b/src/types.rs @@ -12,7 +12,7 @@ use std::sync::{Arc, OnceLock}; use chrono::{Datelike, NaiveDate, NaiveDateTime, NaiveTime, Timelike}; use crate::result::{ZipError, ZipResult}; -use crate::spec::{self, FixedSizeBlock}; +use crate::spec::{self, FixedSizeBlock, Pod}; pub(crate) mod ffi { pub const S_IFDIR: u32 = 0o0040000; @@ -893,7 +893,7 @@ impl ZipFileData { } #[derive(Copy, Clone, Debug)] -#[repr(packed)] +#[repr(packed, C)] pub(crate) struct ZipCentralEntryBlock { magic: spec::Magic, pub version_made_by: u16, @@ -914,6 +914,8 @@ pub(crate) struct ZipCentralEntryBlock { pub offset: u32, } +unsafe impl Pod for ZipCentralEntryBlock {} + impl FixedSizeBlock for ZipCentralEntryBlock { const MAGIC: spec::Magic = spec::Magic::CENTRAL_DIRECTORY_HEADER_SIGNATURE; @@ -947,7 +949,7 @@ impl FixedSizeBlock for ZipCentralEntryBlock { } #[derive(Copy, Clone, Debug)] -#[repr(packed)] +#[repr(packed, C)] pub(crate) struct ZipLocalEntryBlock { magic: spec::Magic, pub version_made_by: u16, @@ -962,6 +964,8 @@ pub(crate) struct ZipLocalEntryBlock { pub extra_field_length: u16, } +unsafe impl Pod for ZipLocalEntryBlock {} + impl FixedSizeBlock for ZipLocalEntryBlock { const MAGIC: spec::Magic = spec::Magic::LOCAL_FILE_HEADER_SIGNATURE;