Make InvalidPassword a kind of ZipError

CHANGELOG.md

@@ -229,7 +229,7 @@
 
  - Updated dependencies.
 
-## [0.10.4]
+## [0.11.0]
 
 ### Added
 
@@ -238,4 +238,9 @@
 
 ### Changed
 
-- Updated dependencies.
+ - `InvalidPassword` is now a kind of `ZipError` to eliminate the need for nested `Result` structs.
+ - Updated dependencies.
+
+### Fixed
+
+ - Fixed some rare bugs that could cause panics when trying to read an invalid ZIP file or using an incorrect password.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "zip_next"
-version = "0.10.4"
+version = "0.11.0"
 authors = ["Mathijs van de Nes <git@mathijs.vd-nes.nl>", "Marli Frost <marli@frost.red>", "Ryan Levick <ryan.levick@gmail.com>",
 "Chris Hennick <hennickc@amazon.com>"]
 license = "MIT"

src/read.rs

@@ -5,7 +5,7 @@ use crate::aes::{AesReader, AesReaderValid};
 use crate::compression::CompressionMethod;
 use crate::cp437::FromCp437;
 use crate::crc32::Crc32Reader;
-use crate::result::{InvalidPassword, ZipError, ZipResult};
+use crate::result::{ZipError, ZipResult};
 use crate::spec;
 use crate::types::{AesMode, AesVendorVersion, AtomicU64, DateTime, System, ZipFileData};
 use crate::zipcrypto::{ZipCryptoReader, ZipCryptoReaderValid, ZipCryptoValidator};
@@ -74,6 +74,7 @@ pub(crate) mod zip_archive {
     }
 }
 
+use crate::result::ZipError::InvalidPassword;
 pub use zip_archive::ZipArchive;
 
 #[allow(clippy::large_enum_variant)]
@@ -229,7 +230,7 @@ pub(crate) fn make_crypto_reader<'a>(
     password: Option<&[u8]>,
     aes_info: Option<(AesMode, AesVendorVersion)>,
     #[cfg(feature = "aes-crypto")] compressed_size: u64,
-) -> ZipResult<Result<CryptoReader<'a>, InvalidPassword>> {
+) -> ZipResult<CryptoReader<'a>> {
     #[allow(deprecated)]
     {
         if let CompressionMethod::Unsupported(_) = compression_method {
@@ -247,7 +248,7 @@ pub(crate) fn make_crypto_reader<'a>(
         #[cfg(feature = "aes-crypto")]
         (Some(password), Some((aes_mode, vendor_version))) => {
             match AesReader::new(reader, aes_mode, compressed_size).validate(password)? {
-                None => return Ok(Err(InvalidPassword)),
+                None => return Err(InvalidPassword),
                 Some(r) => CryptoReader::Aes {
                     reader: r,
                     vendor_version,
@@ -261,14 +262,14 @@ pub(crate) fn make_crypto_reader<'a>(
                 ZipCryptoValidator::PkzipCrc32(crc32)
             };
             match ZipCryptoReader::new(reader, password).validate(validator)? {
-                None => return Ok(Err(InvalidPassword)),
+                None => return Err(InvalidPassword),
                 Some(r) => CryptoReader::ZipCrypto(r),
             }
         }
-        (None, Some(_)) => return Ok(Err(InvalidPassword)),
+        (None, Some(_)) => return Err(InvalidPassword),
         (None, None) => CryptoReader::Plaintext(reader),
     };
-    Ok(Ok(reader))
+    Ok(reader)
 }
 
 pub(crate) fn make_reader(
@@ -588,24 +589,20 @@ impl<R: Read + Seek> ZipArchive<R> {
     /// There are many passwords out there that will also pass the validity checks
     /// we are able to perform. This is a weakness of the ZipCrypto algorithm,
     /// due to its fairly primitive approach to cryptography.
-    pub fn by_name_decrypt<'a>(
+    pub fn by_name_decrypt(&mut self, name: &str, password: &[u8]) -> ZipResult<ZipFile> {
-        &'a mut self,
-        name: &str,
-        password: &[u8],
-    ) -> ZipResult<Result<ZipFile<'a>, InvalidPassword>> {
         self.by_name_with_optional_password(name, Some(password))
     }
 
     /// Search for a file entry by name
     pub fn by_name(&mut self, name: &str) -> ZipResult<ZipFile> {
-        Ok(self.by_name_with_optional_password(name, None)?.unwrap())
+        self.by_name_with_optional_password(name, None)
     }
 
     fn by_name_with_optional_password<'a>(
         &'a mut self,
         name: &str,
         password: Option<&[u8]>,
-    ) -> ZipResult<Result<ZipFile<'a>, InvalidPassword>> {
+    ) -> ZipResult<ZipFile<'a>> {
         let index = match self.shared.names_map.get(name) {
             Some(index) => *index,
             None => {
@@ -632,15 +629,13 @@ impl<R: Read + Seek> ZipArchive<R> {
         &mut self,
         file_number: usize,
         password: &[u8],
-    ) -> ZipResult<Result<ZipFile, InvalidPassword>> {
+    ) -> ZipResult<ZipFile<'_>> {
         self.by_index_with_optional_password(file_number, Some(password))
     }
 
     /// Get a contained file by index
     pub fn by_index(&mut self, file_number: usize) -> ZipResult<ZipFile<'_>> {
-        Ok(self
+        self.by_index_with_optional_password(file_number, None)
-            .by_index_with_optional_password(file_number, None)?
-            .unwrap())
     }
 
     /// Get a contained file by index without decompressing it
@@ -663,7 +658,7 @@ impl<R: Read + Seek> ZipArchive<R> {
         &mut self,
         file_number: usize,
         mut password: Option<&[u8]>,
-    ) -> ZipResult<Result<ZipFile, InvalidPassword>> {
+    ) -> ZipResult<ZipFile<'_>> {
         let data = self
             .shared
             .files
@@ -677,7 +672,7 @@ impl<R: Read + Seek> ZipArchive<R> {
         }
         let limit_reader = find_content(data, &mut self.reader)?;
 
-        match make_crypto_reader(
+        let crypto_reader = make_crypto_reader(
             data.compression_method,
             data.crc32,
             data.last_modified_time,
@@ -687,15 +682,12 @@ impl<R: Read + Seek> ZipArchive<R> {
             data.aes_mode,
             #[cfg(feature = "aes-crypto")]
             data.compressed_size,
-        ) {
+        )?;
-            Ok(Ok(crypto_reader)) => Ok(Ok(ZipFile {
+        Ok(ZipFile {
-                crypto_reader: Some(crypto_reader),
+            crypto_reader: Some(crypto_reader),
-                reader: ZipFileReader::NoReader,
+            reader: ZipFileReader::NoReader,
-                data: Cow::Borrowed(data),
+            data: Cow::Borrowed(data),
-            })),
+        })
-            Err(e) => Err(e),
-            Ok(Err(e)) => Ok(Err(e)),
-        }
     }
 
     /// Unwrap and return the inner reader object
@@ -1186,8 +1178,7 @@ pub fn read_zipfile_from_stream<'a, R: Read>(reader: &'a mut R) -> ZipResult<Opt
         None,
         #[cfg(feature = "aes-crypto")]
         result.compressed_size,
-    )?
+    )?;
-    .unwrap();
 
     Ok(Some(ZipFile {
         data: Cow::Owned(result),

src/result.rs

@@ -9,20 +9,9 @@ use std::num::TryFromIntError;
 /// Generic result type with ZipError as its error variant
 pub type ZipResult<T> = Result<T, ZipError>;
 
-/// The given password is wrong
-#[derive(Debug)]
-pub struct InvalidPassword;
-
-impl fmt::Display for InvalidPassword {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "invalid password for file in archive")
-    }
-}
-
-impl Error for InvalidPassword {}
-
 /// Error type for Zip
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum ZipError {
     /// An Error caused by I/O
     Io(io::Error),
@@ -35,6 +24,9 @@ pub enum ZipError {
 
     /// The requested file could not be found in the archive
     FileNotFound,
+
+    /// The password provided is incorrect
+    InvalidPassword,
 }
 
 impl From<io::Error> for ZipError {
@@ -56,6 +48,7 @@ impl fmt::Display for ZipError {
             ZipError::InvalidArchive(err) => write!(fmt, "invalid Zip archive: {err}"),
             ZipError::UnsupportedArchive(err) => write!(fmt, "unsupported Zip archive: {err}"),
             ZipError::FileNotFound => write!(fmt, "specified file not found in archive"),
+            ZipError::InvalidPassword => write!(fmt, "incorrect password for encrypted file"),
         }
     }
 }

tests/zip_crypto.rs

@@ -18,7 +18,7 @@
 // 000000c5
 
 use std::io::Cursor;
-use std::io::Read;
+use zip_next::result::ZipError;
 
 #[test]
 fn encrypting_file() {
@@ -36,13 +36,14 @@ fn encrypting_file() {
     archive.finish().unwrap();
     drop(archive);
     let mut archive = zip_next::ZipArchive::new(Cursor::new(&mut buf)).unwrap();
-    let mut file = archive.by_index_decrypt(0, b"password").unwrap().unwrap();
+    let mut file = archive.by_index_decrypt(0, b"password").unwrap();
     let mut buf = Vec::new();
     file.read_to_end(&mut buf).unwrap();
     assert_eq!(buf, b"test");
 }
 #[test]
 fn encrypted_file() {
+    use std::io::Read;
     let zip_file_bytes = &mut Cursor::new(vec![
         0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0x54, 0xbd, 0xb5, 0x50, 0x2f,
         0x20, 0x79, 0x55, 0x2f, 0x00, 0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
@@ -82,20 +83,17 @@ fn encrypted_file() {
         // Wrong password
         let file = archive.by_index_decrypt(0, b"wrong password");
         match file {
-            Ok(Err(zip_next::result::InvalidPassword)) => (),
+            Err(ZipError::InvalidPassword) => (),
             Err(_) => panic!(
                 "Expected InvalidPassword error when opening encrypted file with wrong password"
             ),
-            Ok(Ok(_)) => panic!("Error: Successfully opened encrypted file with wrong password?!"),
+            Ok(_) => panic!("Error: Successfully opened encrypted file with wrong password?!"),
         }
     }
 
     {
         // Correct password, read contents
-        let mut file = archive
+        let mut file = archive.by_index_decrypt(0, "test".as_bytes()).unwrap();
-            .by_index_decrypt(0, "test".as_bytes())
-            .unwrap()
-            .unwrap();
         let file_name = file.enclosed_name().unwrap();
         assert_eq!(file_name, std::path::PathBuf::from("test.txt"));