Skip to content
This repository has been archived by the owner on Jun 2, 2024. It is now read-only.

Improve several Read methods on ZipFile #441

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions src/crc32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
check: u32,
/// Signals if `inner` stores aes encrypted data.
/// AE-2 encrypted data doesn't use crc and sets the value to 0.
ae2_encrypted: bool,
enabled: bool,
}

impl<R> Crc32Reader<R> {
Expand All @@ -23,7 +23,7 @@
inner,
hasher: Hasher::new(),
check: checksum,
ae2_encrypted,
enabled: !ae2_encrypted,
}
}

Expand All @@ -36,26 +36,57 @@
}
}

#[cold]
fn invalid_checksum() -> io::Error {
io::Error::new(io::ErrorKind::InvalidData, "Invalid checksum")
}

impl<R: Read> Read for Crc32Reader<R> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
let invalid_check = !buf.is_empty() && !self.check_matches() && !self.ae2_encrypted;
let count = self.inner.read(buf)?;

let count = match self.inner.read(buf) {
Ok(0) if invalid_check => {
return Err(io::Error::new(io::ErrorKind::Other, "Invalid checksum"))
if self.enabled {
if count == 0 && !buf.is_empty() && !self.check_matches() {
return Err(invalid_checksum());
}
Ok(n) => n,
Err(e) => return Err(e),
};
self.hasher.update(&buf[0..count]);
self.hasher.update(&buf[..count]);
}
Ok(count)
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
let start = buf.len();
let n = self.inner.read_to_end(buf)?;

if self.enabled {
self.hasher.update(&buf[start..]);
if !self.check_matches() {
return Err(invalid_checksum());
}
}

Ok(n)
}

fn read_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
let start = buf.len();
let n = self.inner.read_to_string(buf)?;

if self.enabled {
self.hasher.update(&buf.as_bytes()[start..]);
if !self.check_matches() {
return Err(invalid_checksum());
}
}

Ok(n)
}
}

#[cfg(test)]
mod test {
use super::*;
use std::io::Read;

Check failure on line 89 in src/crc32.rs

View workflow job for this annotation

GitHub Actions / clippy

the item `Read` is imported redundantly

#[test]
fn test_empty_reader() {
Expand Down
84 changes: 84 additions & 0 deletions src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,24 @@
CryptoReader::Aes { reader: r, .. } => r.read(buf),
}
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
match self {
CryptoReader::Plaintext(r) => r.read_to_end(buf),
CryptoReader::ZipCrypto(r) => r.read_to_end(buf),
#[cfg(feature = "aes-crypto")]
CryptoReader::Aes { reader: r, .. } => r.read_to_end(buf),
}
}

fn read_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
match self {
CryptoReader::Plaintext(r) => r.read_to_string(buf),
CryptoReader::ZipCrypto(r) => r.read_to_string(buf),
#[cfg(feature = "aes-crypto")]
CryptoReader::Aes { reader: r, .. } => r.read_to_string(buf),
}
}
}

impl<'a> CryptoReader<'a> {
Expand Down Expand Up @@ -153,6 +171,60 @@
ZipFileReader::Zstd(r) => r.read(buf),
}
}

fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
match self {
ZipFileReader::NoReader => panic!("ZipFileReader was in an invalid state"),
ZipFileReader::Raw(r) => r.read_exact(buf),
ZipFileReader::Stored(r) => r.read_exact(buf),
#[cfg(any(
feature = "deflate",
feature = "deflate-miniz",
feature = "deflate-zlib"
))]
ZipFileReader::Deflated(r) => r.read_exact(buf),
#[cfg(feature = "bzip2")]
ZipFileReader::Bzip2(r) => r.read_exact(buf),
#[cfg(feature = "zstd")]
ZipFileReader::Zstd(r) => r.read_exact(buf),
}
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
match self {
ZipFileReader::NoReader => panic!("ZipFileReader was in an invalid state"),
ZipFileReader::Raw(r) => r.read_to_end(buf),
ZipFileReader::Stored(r) => r.read_to_end(buf),
#[cfg(any(
feature = "deflate",
feature = "deflate-miniz",
feature = "deflate-zlib"
))]
ZipFileReader::Deflated(r) => r.read_to_end(buf),
#[cfg(feature = "bzip2")]
ZipFileReader::Bzip2(r) => r.read_to_end(buf),
#[cfg(feature = "zstd")]
ZipFileReader::Zstd(r) => r.read_to_end(buf),
}
}

fn read_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
match self {
ZipFileReader::NoReader => panic!("ZipFileReader was in an invalid state"),
ZipFileReader::Raw(r) => r.read_to_string(buf),
ZipFileReader::Stored(r) => r.read_to_string(buf),
#[cfg(any(
feature = "deflate",
feature = "deflate-miniz",
feature = "deflate-zlib"
))]
ZipFileReader::Deflated(r) => r.read_to_string(buf),
#[cfg(feature = "bzip2")]
ZipFileReader::Bzip2(r) => r.read_to_string(buf),
#[cfg(feature = "zstd")]
ZipFileReader::Zstd(r) => r.read_to_string(buf),
}
}
}

impl<'a> ZipFileReader<'a> {
Expand Down Expand Up @@ -934,7 +1006,7 @@
/// Returns whether the file is actually a directory
pub fn is_dir(&self) -> bool {
self.name()
.chars()

Check failure on line 1009 in src/read.rs

View workflow job for this annotation

GitHub Actions / clippy

manual backwards iteration
.rev()
.next()
.map_or(false, |c| c == '/' || c == '\\')
Expand Down Expand Up @@ -979,6 +1051,18 @@
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
self.get_reader().read(buf)
}

fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
self.get_reader().read_exact(buf)
}

fn read_to_end(&mut self, buf: &mut Vec<u8>) -> io::Result<usize> {
self.get_reader().read_to_end(buf)
}

fn read_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
self.get_reader().read_to_string(buf)
}
}

impl<'a> Drop for ZipFile<'a> {
Expand All @@ -991,7 +1075,7 @@
// Get the inner `Take` reader so all decryption, decompression and CRC calculation is skipped.
let mut reader: std::io::Take<&mut dyn std::io::Read> = match &mut self.reader {
ZipFileReader::NoReader => {
let innerreader = ::std::mem::replace(&mut self.crypto_reader, None);

Check failure on line 1078 in src/read.rs

View workflow job for this annotation

GitHub Actions / clippy

replacing an `Option` with `None`
innerreader.expect("Invalid reader state").into_inner()
}
reader => {
Expand Down
Loading