From 36fedb5ea004c47d5c6088c2592b7431f8f213e8 Mon Sep 17 00:00:00 2001 From: Danyel Date: Thu, 5 Sep 2024 22:03:26 +0200 Subject: [PATCH 1/2] refactor(modes/list): DRY iterator impl with macro --- src/error/nul.rs | 5 +++ src/error/rar.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++ src/open_archive.rs | 65 +++++++++++-------------------- 3 files changed, 123 insertions(+), 42 deletions(-) create mode 100644 src/error/nul.rs create mode 100644 src/error/rar.rs diff --git a/src/error/nul.rs b/src/error/nul.rs new file mode 100644 index 0000000..ea4e487 --- /dev/null +++ b/src/error/nul.rs @@ -0,0 +1,5 @@ +use thiserror::Error; + +#[derive(PartialEq, Eq, Error, Debug, Clone, Copy)] +#[error("unexpected NUL at {0}")] +pub struct Error(pub usize); diff --git a/src/error/rar.rs b/src/error/rar.rs new file mode 100644 index 0000000..fc0782b --- /dev/null +++ b/src/error/rar.rs @@ -0,0 +1,95 @@ +use thiserror::Error; +use unrar_sys as native; + +pub type Result = std::result::Result; + +#[derive(PartialEq, Eq, Error, Debug, Clone, Copy)] +pub enum Error { + #[error("Archive header damaged")] + ArchiveHeaderDamaged, + #[error("File header damaged")] + FileHeaderDamaged, + #[error("File CRC error")] + FileCRCError, + #[error("Unknown encryption")] + UnkownEncryption, + #[error("Could not open next volume")] + NextVolumeNotFound, + #[error("Unknown archive format")] + UnknownFormat, + #[error("Could not open archive")] + EOpen, + #[error("Not enough memory")] + NoMemory, + #[error("Not a RAR archive")] + BadArchive, + #[error("Could not create file")] + ECreate, + #[error("Could not close file")] + EClose, + #[error("Read error")] + ERead, + #[error("Write error")] + EWrite, + #[error("Archive comment was truncated to fit to buffer")] + SmallBuf, + #[error("Password for encrypted archive not specified")] + MissingPassword, + #[error("Wrong password was specified")] + BadPassword, + #[error("Unknown error")] + Unknown, + // From the UnRARDLL docs: + // When attempting to unpack a reference record (see RAR -oi switch), + // source file for this reference was not found. + // Entire archive needs to be unpacked to properly create file references. + // This error is returned when attempting to unpack the reference + // record without its source file. + #[error("Cannot open file source for reference record")] + EReference, +} + +impl Error { + pub(crate) fn at_open(code: u32) -> Option { + Self::from(code, When::Open) + } + + pub(crate) fn at_header(code: u32) -> Option { + Self::from(code, When::Header) + } + + pub(crate) fn at_process(code: u32) -> Option { + Self::from(code, When::Process) + } + + fn from(code: u32, when: When) -> Option { + use Error::*; + match (code, when) { + (native::ERAR_BAD_DATA, When::Open) => Some(ArchiveHeaderDamaged), + (native::ERAR_BAD_DATA, When::Header) => Some(FileHeaderDamaged), + (native::ERAR_BAD_DATA, When::Process) => Some(FileCRCError), + (native::ERAR_UNKNOWN_FORMAT, When::Open) => Some(UnkownEncryption), + (native::ERAR_EOPEN, When::Process) => Some(NextVolumeNotFound), + (native::ERAR_NO_MEMORY, _) => Some(NoMemory), + (native::ERAR_BAD_ARCHIVE, _) => Some(BadArchive), + (native::ERAR_UNKNOWN_FORMAT, _) => Some(UnknownFormat), + (native::ERAR_EOPEN, _) => Some(EOpen), + (native::ERAR_ECREATE, _) => Some(ECreate), + (native::ERAR_ECLOSE, _) => Some(EClose), + (native::ERAR_EREAD, _) => Some(ERead), + (native::ERAR_EWRITE, _) => Some(EWrite), + (native::ERAR_SMALL_BUF, _) => Some(SmallBuf), + (native::ERAR_UNKNOWN, _) => Some(Unknown), + (native::ERAR_MISSING_PASSWORD, _) => Some(MissingPassword), + (native::ERAR_EREFERENCE, _) => Some(EReference), + (native::ERAR_BAD_PASSWORD, _) => Some(BadPassword), + _ => None, + } + } +} + +enum When { + Open, + Header, + Process, +} diff --git a/src/open_archive.rs b/src/open_archive.rs index dbabaf8..25cc166 100644 --- a/src/open_archive.rs +++ b/src/open_archive.rs @@ -288,57 +288,38 @@ impl OpenArchive { } } -impl Iterator for OpenArchive { - type Item = Result; - - fn next(&mut self) -> Option { - if self.damaged { - return None; - } - match read_header(&self.handle) { - Ok(Some(header)) => { - match Internal::::process_file_raw(&self.handle, None, None) { - Ok(_) => Some(Ok(header)), - Err(s) => { - self.damaged = true; - Some(Err(s)) - } +macro_rules! iterator_impl { + ($x: ident) => { + impl Iterator for OpenArchive<$x, CursorBeforeHeader> { + type Item = Result; + + fn next(&mut self) -> Option { + if self.damaged { + return None; } - } - Ok(None) => None, - Err(s) => { - self.damaged = true; - Some(Err(s)) - } - } - } -} - -impl Iterator for OpenArchive { - type Item = Result; - - fn next(&mut self) -> Option { - if self.damaged { - return None; - } - match read_header(&self.handle) { - Ok(Some(header)) => { - match Internal::::process_file_raw(&self.handle, None, None) { - Ok(_) => Some(Ok(header)), + match read_header(&self.handle) { + Ok(Some(header)) => { + match Internal::::process_file_raw(&self.handle, None, None) + { + Ok(_) => Some(Ok(header)), + Err(s) => { + self.damaged = true; + Some(Err(s)) + } + } + } + Ok(None) => None, Err(s) => { self.damaged = true; Some(Err(s)) } } } - Ok(None) => None, - Err(s) => { - self.damaged = true; - Some(Err(s)) - } } - } + }; } +iterator_impl!(List); +iterator_impl!(ListSplit); impl OpenArchive { /// returns the file header for the file that follows which is to be processed next. From fa23d888555a7c587579936cf27ecce058074cef Mon Sep 17 00:00:00 2001 From: Danyel Date: Thu, 5 Sep 2024 22:33:00 +0200 Subject: [PATCH 2/2] feat!: improve errors drastically --- Cargo.toml | 1 + README.md | 5 +- examples/basic_extract.rs | 7 +- examples/read_named.rs | 4 +- src/archive.rs | 66 ++++++----- src/error.rs | 240 +++++++++++++++++--------------------- src/error/nul.rs | 5 - src/error/rar.rs | 95 --------------- src/lib.rs | 10 +- src/open_archive.rs | 111 +++++++++--------- src/pathed/all.rs | 13 ++- src/pathed/linux.rs | 13 ++- src/pathed/mod.rs | 43 ++++++- tests/crypted.rs | 7 +- tests/simple.rs | 2 +- unrar_sys/src/lib.rs | 44 +++---- 16 files changed, 295 insertions(+), 371 deletions(-) delete mode 100644 src/error/nul.rs delete mode 100644 src/error/rar.rs diff --git a/Cargo.toml b/Cargo.toml index 6a7c2d2..c145909 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ repository = "https://github.com/muja/unrar.rs" regex = "1" bitflags = "1" widestring = "1" +thiserror = "1.0.50" [dependencies.unrar_sys] path = "unrar_sys" diff --git a/README.md b/README.md index 0c5a8fe..f68338a 100644 --- a/README.md +++ b/README.md @@ -100,15 +100,14 @@ For more sophisticated examples, please look inside the `examples/` folder. Here's what a function that returns the first content of a file could look like: ```rust -fn first_file_content>(path: P) -> UnrarResult> { - let archive = Archive::new(&path).open_for_processing()?; // cursor: before header +fn first_file_content>(path: P) -> Result, Box> { + let archive = unrar::Archive::new(&path).open_for_processing()?; // cursor: before header let archive = archive.read_header()?.expect("empty archive"); // cursor: before file dbg!(&archive.entry().filename); let (data, _rest) = archive.read()?; // cursor: before header Ok(data) } # use std::path::Path; -# use unrar::{Archive, UnrarResult}; # # let data = first_file_content("data/version.rar").unwrap(); # assert_eq!(std::str::from_utf8(&data), Ok("unrar-0.4.0")); diff --git a/examples/basic_extract.rs b/examples/basic_extract.rs index 9c26c77..dc0b42d 100644 --- a/examples/basic_extract.rs +++ b/examples/basic_extract.rs @@ -1,10 +1,9 @@ use unrar::Archive; fn main() -> Result<(), Box> { - let mut archive = - Archive::new("../archive.rar") - .open_for_processing() - .unwrap(); + let args = std::env::args(); + let file = args.skip(1).next().unwrap_or("archive.rar".to_owned()); + let mut archive = Archive::new(&file).open_for_processing().unwrap(); while let Some(header) = archive.read_header()? { println!( "{} bytes: {}", diff --git a/examples/read_named.rs b/examples/read_named.rs index ee55ca7..19e195a 100644 --- a/examples/read_named.rs +++ b/examples/read_named.rs @@ -1,6 +1,6 @@ -use unrar::{Archive, UnrarResult}; +use unrar::Archive; -fn main() -> UnrarResult<()> { +fn main() -> Result<(), Box> { // Basic args parsing // Usage: cargo run --example extract_named let mut args = std::env::args_os().skip(1); diff --git a/src/archive.rs b/src/archive.rs index 89c4ecd..ebb3c3b 100644 --- a/src/archive.rs +++ b/src/archive.rs @@ -1,4 +1,5 @@ -use crate::error::*; +use crate::Nulable; +use crate::OpenError; use crate::open_archive::{CursorBeforeHeader, List, ListSplit, OpenArchive, OpenMode, Process}; use regex::Regex; use std::borrow::Cow; @@ -323,10 +324,14 @@ impl<'a> Archive<'a> { /// /// See also: [`Process`] /// - /// # Panics + /// # Errors /// - /// Panics if `self.filename` contains nul values. - pub fn open_for_processing(self) -> UnrarResult> { + /// - `NulError` if `self.filename` or `self.password` contains nul values. + /// - `RarError` if there was an error opening/reading/decoding the archive. + /// + pub fn open_for_processing( + self, + ) -> Result, Nulable> { self.open(None) } @@ -334,10 +339,14 @@ impl<'a> Archive<'a> { /// /// See also: [`List`] /// - /// # Panics + /// # Errors + /// + /// - `NulError` if `self.filename` or `self.password` contains nul values. + /// - `RarError` if there was an error opening/reading/decoding the archive. /// - /// Panics if `self.filename` contains nul values. - pub fn open_for_listing(self) -> UnrarResult> { + pub fn open_for_listing( + self, + ) -> Result, Nulable> { self.open(None) } @@ -348,23 +357,22 @@ impl<'a> Archive<'a> { /// /// See also: [`ListSplit`] /// - /// # Panics + /// # Errors /// - /// Panics if `self.filename` contains nul values. - - pub fn open_for_listing_split(self) -> UnrarResult> { + /// - `NulError` if `self.filename` or `self.password` contains nul values. + /// - `RarError` if there was an error opening/reading/decoding the archive. + /// + pub fn open_for_listing_split( + self, + ) -> Result, Nulable> { self.open(None) } /// Opens the underlying archive with the provided parameters. - /// - /// # Panics - /// - /// Panics if `path` contains nul values. fn open( self, recover: Option<&mut Option>>, - ) -> UnrarResult> { + ) -> Result, Nulable> { OpenArchive::new(&self.filename, self.password, recover) } @@ -378,8 +386,8 @@ impl<'a> Archive<'a> { /// # Example: I don't care if there was a recoverable error /// /// ```no_run - /// # use unrar::{Archive, List, UnrarResult}; - /// # fn x() -> UnrarResult<()> { + /// # use unrar::{Archive, List}; + /// # fn x() -> Result<(), unrar::Nulable> { /// let mut open_archive = Archive::new("file").break_open::(None)?; /// // use open_archive /// # Ok(()) @@ -389,8 +397,8 @@ impl<'a> Archive<'a> { /// # Example: I want to know if there was a recoverable error /// /// ```no_run - /// # use unrar::{Archive, List, UnrarResult}; - /// # fn x() -> UnrarResult<()> { + /// # use unrar::{Archive, List}; + /// # fn x() -> Result<(), unrar::Nulable> { /// let mut possible_error = None; /// let mut open_archive = Archive::new("file").break_open::(Some(&mut possible_error))?; /// // check the error, e.g.: @@ -400,21 +408,23 @@ impl<'a> Archive<'a> { /// # } /// ``` /// - /// # Panics + /// # Errors + /// + /// - `NulError` if `self.filename` or `self.password` contains nul values. + /// - `RarError` if there was an error opening/reading/decoding the archive. /// - /// Panics if `path` contains nul values. pub fn break_open( self, - error: Option<&mut Option>, - ) -> UnrarResult> { + error: Option<&mut Option>, + ) -> Result, Nulable> { let mut recovered = None; self.open(Some(&mut recovered)) - .or_else(|x| match recovered { - Some(archive) => { - error.map(|error| *error = Some(x)); + .or_else(|e| match (recovered, e) { + (Some(archive), Nulable::Rar(e)) => { + error.map(|error| *error = Some(e)); Ok(archive) } - None => Err(x), + (_, _) => Err(e), }) } } diff --git a/src/error.rs b/src/error.rs index 55daf03..5e2373f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,146 +1,116 @@ #![allow(missing_docs)] -use super::*; -use std::error; -use std::ffi; -use std::fmt; -use std::result::Result; - - -#[derive(PartialEq, Eq, Debug, Clone, Copy)] -#[repr(i32)] -pub enum Code { - Success = native::ERAR_SUCCESS, - EndArchive = native::ERAR_END_ARCHIVE, - NoMemory = native::ERAR_NO_MEMORY, - BadData = native::ERAR_BAD_DATA, - BadArchive = native::ERAR_BAD_ARCHIVE, - UnknownFormat = native::ERAR_UNKNOWN_FORMAT, - EOpen = native::ERAR_EOPEN, - ECreate = native::ERAR_ECREATE, - EClose = native::ERAR_ECLOSE, - ERead = native::ERAR_EREAD, - EWrite = native::ERAR_EWRITE, - SmallBuf = native::ERAR_SMALL_BUF, - Unknown = native::ERAR_UNKNOWN, - MissingPassword = native::ERAR_MISSING_PASSWORD, - // From the UnRARDLL docs: - // When attempting to unpack a reference record (see RAR -oi switch), - // source file for this reference was not found. - // Entire archive needs to be unpacked to properly create file references. - // This error is returned when attempting to unpack the reference - // record without its source file. - EReference = native::ERAR_EREFERENCE, - BadPassword = native::ERAR_BAD_PASSWORD, -} - -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum When { - Open, - Read, - Process, -} - -impl Code { - pub fn from(code: i32) -> Option { - use Code::*; - match code { - native::ERAR_SUCCESS => Some(Success), - native::ERAR_END_ARCHIVE => Some(EndArchive), - native::ERAR_NO_MEMORY => Some(NoMemory), - native::ERAR_BAD_DATA => Some(BadData), - native::ERAR_BAD_ARCHIVE => Some(BadArchive), - native::ERAR_UNKNOWN_FORMAT => Some(UnknownFormat), - native::ERAR_EOPEN => Some(EOpen), - native::ERAR_ECREATE => Some(ECreate), - native::ERAR_ECLOSE => Some(EClose), - native::ERAR_EREAD => Some(ERead), - native::ERAR_EWRITE => Some(EWrite), - native::ERAR_SMALL_BUF => Some(SmallBuf), - native::ERAR_UNKNOWN => Some(Unknown), - native::ERAR_MISSING_PASSWORD => Some(MissingPassword), - native::ERAR_EREFERENCE => Some(EReference), - native::ERAR_BAD_PASSWORD => Some(BadPassword), - _ => None, +use thiserror::Error; +use unrar_sys as native; + +macro_rules! rarerror { + ($name:ident$(#[$($tags:tt)*]$variant_name:ident=$variant_code:path),+$(,)?) => { + #[repr(u32)] + #[derive(PartialEq, Eq, Error, Debug, Clone, Copy)] + pub enum $name { + #[error("Unknown error")] + Unknown = native::ERAR_UNKNOWN, + $( + #[$($tags)*] + $variant_name = $variant_code + ),+ } - } -} - -#[derive(PartialEq)] -pub struct UnrarError { - pub code: Code, - pub when: When, -} - -impl std::error::Error for UnrarError {} - -impl fmt::Debug for UnrarError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{:?}@{:?}", self.code, self.when)?; - write!(f, " ({})", self) - } -} - -impl fmt::Display for UnrarError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use self::Code::*; - use self::When::*; - match (self.code, self.when) { - (BadData, Open) => write!(f, "Archive header damaged"), - (BadData, Read) => write!(f, "File header damaged"), - (BadData, Process) => write!(f, "File CRC error"), - (UnknownFormat, Open) => write!(f, "Unknown encryption"), - (EOpen, Process) => write!(f, "Could not open next volume"), - (UnknownFormat, _) => write!(f, "Unknown archive format"), - (EOpen, _) => write!(f, "Could not open archive"), - (NoMemory, _) => write!(f, "Not enough memory"), - (BadArchive, _) => write!(f, "Not a RAR archive"), - (ECreate, _) => write!(f, "Could not create file"), - (EClose, _) => write!(f, "Could not close file"), - (ERead, _) => write!(f, "Read error"), - (EWrite, _) => write!(f, "Write error"), - (SmallBuf, _) => write!(f, "Archive comment was truncated to fit to buffer"), - (MissingPassword, _) => write!(f, "Password for encrypted archive not specified"), - (EReference, _) => write!(f, "Cannot open file source for reference record"), - (BadPassword, _) => write!(f, "Wrong password was specified"), - (Unknown, _) => write!(f, "Unknown error"), - (EndArchive, _) => write!(f, "Archive end"), - (Success, _) => write!(f, "Success"), + impl From for $name { + fn from(code: u32) -> Self { + use $name::*; + match code { + $( + $variant_code => $variant_name, + )+ + _ => Unknown, + } + } } } } -impl UnrarError { - pub fn from(code: Code, when: When) -> Self { - UnrarError { code, when } - } -} - -pub type UnrarResult = Result; - -#[derive(Debug)] -pub struct NulError(usize); - -impl fmt::Display for NulError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "nul value found at position: {}", self.0) - } -} +rarerror!(OpenError + #[error("Archive header damaged")] + ArchiveHeaderDamaged = native::ERAR_BAD_DATA, + #[error("Unknown encryption")] + UnkownEncryption = native::ERAR_UNKNOWN_FORMAT, + #[error("Wrong password was specified")] + BadPassword = native::ERAR_BAD_PASSWORD, + #[error("Not enough memory")] + NoMemory = native::ERAR_NO_MEMORY, + #[error("Could not open archive")] + Open = native::ERAR_EOPEN, +); -impl error::Error for NulError { - fn description(&self) -> &str { - "nul value found" - } -} +rarerror!(HeaderError + #[error("End of archive")] + EndArchive = native::ERAR_END_ARCHIVE, + #[error("File header damaged")] + BrokenHeader = native::ERAR_BAD_DATA, + #[error("Password was not provided for encrypted file header")] + MissingPassword = native::ERAR_MISSING_PASSWORD, + #[error("Could not open next volume")] + VolumeOpen = native::ERAR_EOPEN, +); + +rarerror!(ProcessError + #[error("File CRC error")] + FileCRC = native::ERAR_BAD_DATA, + #[error("Unknown archive format")] + UnknownFormat = native::ERAR_UNKNOWN_FORMAT, + #[error("Could not open next volume")] + VolumeOpen = native::ERAR_EOPEN, + #[error("Read error")] + Read = native::ERAR_EREAD, + #[error("Write error")] + Write = native::ERAR_EWRITE, + #[error("Not enough memory")] + NoMemory = native::ERAR_NO_MEMORY, + #[error("Cannot open file source for reference record")] + EReference = native::ERAR_EREFERENCE, + #[error("Entered password is invalid")] + BadPassword = native::ERAR_BAD_PASSWORD, + #[error("Password was not provided for encrypted file header")] + MissingPassword = native::ERAR_MISSING_PASSWORD, +); -impl From> for NulError { - fn from(e: widestring::error::ContainsNul) -> NulError { - NulError(e.nul_position()) - } +rarerror!(ExtractError + #[error("File CRC error")] + FileCRC = native::ERAR_BAD_DATA, + #[error("Unknown archive format")] + UnknownFormat = native::ERAR_UNKNOWN_FORMAT, + #[error("Could not open next volume")] + VolumeOpen = native::ERAR_EOPEN, + #[error("Read error")] + Read = native::ERAR_EREAD, + #[error("Write error")] + Write = native::ERAR_EWRITE, + #[error("Not enough memory")] + NoMemory = native::ERAR_NO_MEMORY, + #[error("Cannot open file source for reference record")] + EReference = native::ERAR_EREFERENCE, + #[error("Entered password is invalid")] + BadPassword = native::ERAR_BAD_PASSWORD, + #[error("Password was not provided for encrypted file header")] + MissingPassword = native::ERAR_MISSING_PASSWORD, + #[error("File create error")] + FileCreate = native::ERAR_ECREATE, + #[error("File close error")] + FileClose = native::ERAR_ECLOSE, +); + +#[derive(PartialEq, Eq, Error, Debug, Clone, Copy)] +pub enum IterateError { + #[error(transparent)] + Header(#[from] HeaderError), + #[error(transparent)] + Skip(#[from] ProcessError), + } -impl From for NulError { - fn from(e: ffi::NulError) -> NulError { - NulError(e.nul_position()) - } -} +pub trait RarError: std::fmt::Display {} +impl RarError for OpenError {} +impl RarError for HeaderError {} +impl RarError for ProcessError {} +impl RarError for ExtractError {} +impl RarError for IterateError {} diff --git a/src/error/nul.rs b/src/error/nul.rs deleted file mode 100644 index ea4e487..0000000 --- a/src/error/nul.rs +++ /dev/null @@ -1,5 +0,0 @@ -use thiserror::Error; - -#[derive(PartialEq, Eq, Error, Debug, Clone, Copy)] -#[error("unexpected NUL at {0}")] -pub struct Error(pub usize); diff --git a/src/error/rar.rs b/src/error/rar.rs deleted file mode 100644 index fc0782b..0000000 --- a/src/error/rar.rs +++ /dev/null @@ -1,95 +0,0 @@ -use thiserror::Error; -use unrar_sys as native; - -pub type Result = std::result::Result; - -#[derive(PartialEq, Eq, Error, Debug, Clone, Copy)] -pub enum Error { - #[error("Archive header damaged")] - ArchiveHeaderDamaged, - #[error("File header damaged")] - FileHeaderDamaged, - #[error("File CRC error")] - FileCRCError, - #[error("Unknown encryption")] - UnkownEncryption, - #[error("Could not open next volume")] - NextVolumeNotFound, - #[error("Unknown archive format")] - UnknownFormat, - #[error("Could not open archive")] - EOpen, - #[error("Not enough memory")] - NoMemory, - #[error("Not a RAR archive")] - BadArchive, - #[error("Could not create file")] - ECreate, - #[error("Could not close file")] - EClose, - #[error("Read error")] - ERead, - #[error("Write error")] - EWrite, - #[error("Archive comment was truncated to fit to buffer")] - SmallBuf, - #[error("Password for encrypted archive not specified")] - MissingPassword, - #[error("Wrong password was specified")] - BadPassword, - #[error("Unknown error")] - Unknown, - // From the UnRARDLL docs: - // When attempting to unpack a reference record (see RAR -oi switch), - // source file for this reference was not found. - // Entire archive needs to be unpacked to properly create file references. - // This error is returned when attempting to unpack the reference - // record without its source file. - #[error("Cannot open file source for reference record")] - EReference, -} - -impl Error { - pub(crate) fn at_open(code: u32) -> Option { - Self::from(code, When::Open) - } - - pub(crate) fn at_header(code: u32) -> Option { - Self::from(code, When::Header) - } - - pub(crate) fn at_process(code: u32) -> Option { - Self::from(code, When::Process) - } - - fn from(code: u32, when: When) -> Option { - use Error::*; - match (code, when) { - (native::ERAR_BAD_DATA, When::Open) => Some(ArchiveHeaderDamaged), - (native::ERAR_BAD_DATA, When::Header) => Some(FileHeaderDamaged), - (native::ERAR_BAD_DATA, When::Process) => Some(FileCRCError), - (native::ERAR_UNKNOWN_FORMAT, When::Open) => Some(UnkownEncryption), - (native::ERAR_EOPEN, When::Process) => Some(NextVolumeNotFound), - (native::ERAR_NO_MEMORY, _) => Some(NoMemory), - (native::ERAR_BAD_ARCHIVE, _) => Some(BadArchive), - (native::ERAR_UNKNOWN_FORMAT, _) => Some(UnknownFormat), - (native::ERAR_EOPEN, _) => Some(EOpen), - (native::ERAR_ECREATE, _) => Some(ECreate), - (native::ERAR_ECLOSE, _) => Some(EClose), - (native::ERAR_EREAD, _) => Some(ERead), - (native::ERAR_EWRITE, _) => Some(EWrite), - (native::ERAR_SMALL_BUF, _) => Some(SmallBuf), - (native::ERAR_UNKNOWN, _) => Some(Unknown), - (native::ERAR_MISSING_PASSWORD, _) => Some(MissingPassword), - (native::ERAR_EREFERENCE, _) => Some(EReference), - (native::ERAR_BAD_PASSWORD, _) => Some(BadPassword), - _ => None, - } - } -} - -enum When { - Open, - Header, - Process, -} diff --git a/src/lib.rs b/src/lib.rs index ef542af..21a2c0f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,13 +1,15 @@ #![doc = include_str!("../README.md")] #![warn(missing_docs)] -pub use archive::Archive; -use unrar_sys as native; mod archive; -pub mod error; +mod error; mod pathed; mod open_archive; -pub use error::UnrarResult; + +pub use error::*; +pub use pathed::Nulable; + +pub use archive::Archive; pub use open_archive::{ CursorBeforeFile, CursorBeforeHeader, FileHeader, List, ListSplit, OpenArchive, Process, VolumeInfo, diff --git a/src/open_archive.rs b/src/open_archive.rs index 25cc166..803c568 100644 --- a/src/open_archive.rs +++ b/src/open_archive.rs @@ -1,9 +1,11 @@ use super::error::*; -use super::*; +use super::pathed; +use super::pathed::Nulable; use std::fmt; use std::os::raw::{c_int, c_uint}; use std::path::{Path, PathBuf}; use std::ptr::NonNull; +use unrar_sys as native; bitflags::bitflags! { #[derive(Default)] @@ -201,7 +203,7 @@ impl OpenArchive { /// # Example how you *might* use this /// /// ```no_run - /// use unrar::{Archive, error::{When, Code}}; + /// use unrar::{Archive, IterateError}; /// /// let mut archive = Archive::new("corrupt.rar").open_for_listing().expect("archive error"); /// loop { @@ -213,8 +215,8 @@ impl OpenArchive { /// } /// } /// match error { - /// // your special recoverable error, please submit a PR with reproducible archive - /// Some(e) if (e.when, e.code) == (When::Process, Code::BadData) => archive.force_heal(), + /// // your special recoverable error, please open an issue with reproducible archive + /// Some(IterateError::Skip(unrar::ProcessError::Unknown)) => archive.force_heal(), /// Some(e) => panic!("irrecoverable error: {e}"), /// None => break, /// } @@ -230,19 +232,22 @@ impl OpenArchive { filename: &Path, password: Option<&[u8]>, recover: Option<&mut Option>, - ) -> UnrarResult { - let filename = pathed::construct(filename); + ) -> Result> { + let filename = pathed::construct(filename)?; let mut data = native::OpenArchiveDataEx::new(filename.as_ptr() as *const _, Mode::VALUE as u32); let handle = NonNull::new(unsafe { native::RAROpenArchiveEx(&mut data as *mut _) } as *mut _); - let arc = handle.and_then(|handle| { + if let Some(handle) = handle { if let Some(pw) = password { - let cpw = std::ffi::CString::new(pw).unwrap(); + let cpw = std::ffi::CString::new(pw)?; unsafe { native::RARSetPassword(handle.as_ptr(), cpw.as_ptr() as *const _) } } + } + + let arc = handle.and_then(|handle| { Some(OpenArchive { handle: Handle(handle), damaged: false, @@ -251,13 +256,12 @@ impl OpenArchive { marker: std::marker::PhantomData, }) }); - let result = Code::from(data.open_result as i32).unwrap(); - match (arc, result) { - (Some(arc), Code::Success) => Ok(arc), - (arc, _) => { + match (arc, data.open_result) { + (Some(arc), native::ERAR_SUCCESS) => Ok(arc), + (arc, code) => { recover.and_then(|recover| arc.and_then(|arc| recover.replace(arc))); - Err(UnrarError::from(result, When::Open)) + Err(Nulable::Rar(code.try_into().unwrap())) } } } @@ -277,7 +281,7 @@ impl OpenArchive { /// let archive = archive.unwrap().unwrap(); /// assert_eq!(archive.entry().filename.as_os_str(), "VERSION"); /// ``` - pub fn read_header(self) -> UnrarResult>> { + pub fn read_header(self) -> Result>, HeaderError> { Ok(read_header(&self.handle)?.map(|entry| OpenArchive { extra: CursorBeforeFile { header: entry }, damaged: self.damaged, @@ -291,7 +295,7 @@ impl OpenArchive { macro_rules! iterator_impl { ($x: ident) => { impl Iterator for OpenArchive<$x, CursorBeforeHeader> { - type Item = Result; + type Item = Result; fn next(&mut self) -> Option { if self.damaged { @@ -299,19 +303,18 @@ macro_rules! iterator_impl { } match read_header(&self.handle) { Ok(Some(header)) => { - match Internal::::process_file_raw(&self.handle, None, None) - { + match Internal::::process_file_raw(&self.handle, None, None) { Ok(_) => Some(Ok(header)), Err(s) => { self.damaged = true; - Some(Err(s)) + Some(Err(s.into())) } } } Ok(None) => None, Err(s) => { self.damaged = true; - Some(Err(s)) + Some(Err(s.into())) } } } @@ -328,7 +331,7 @@ impl OpenArchive { } /// skips over the next file, not doing anything with it. - pub fn skip(self) -> UnrarResult> { + pub fn skip(self) -> Result, ProcessError> { self.process_file::(None, None) } @@ -336,7 +339,7 @@ impl OpenArchive { self, path: Option<&pathed::RarStr>, file: Option<&pathed::RarStr>, - ) -> UnrarResult> { + ) -> Result, ProcessError> { Ok(self.process_file_x::(path, file)?.1) } @@ -344,7 +347,7 @@ impl OpenArchive { self, path: Option<&pathed::RarStr>, file: Option<&pathed::RarStr>, - ) -> UnrarResult<(PM::Output, OpenArchive)> { + ) -> Result<(PM::Output, OpenArchive), ProcessError> { let result = Ok(( Internal::::process_file_raw(&self.handle, path, file)?, OpenArchive { @@ -362,46 +365,49 @@ impl OpenArchive { impl OpenArchive { /// Reads the underlying file into a `Vec` /// Returns the data as well as the owned Archive that can be processed further. - pub fn read(self) -> UnrarResult<(Vec, OpenArchive)> { + pub fn read(self) -> Result<(Vec, OpenArchive), ProcessError> { Ok(self.process_file_x::(None, None)?) } /// Test the file without extracting it - pub fn test(self) -> UnrarResult> { + pub fn test(self) -> Result, ProcessError> { Ok(self.process_file::(None, None)?) } /// Extracts the file into the current working directory + /// /// Returns the OpenArchive for further processing - pub fn extract(self) -> UnrarResult> { - self.dir_extract(None) + pub fn extract(self) -> Result, ProcessError> { + self.dir_extract(None).map_err(Nulable::unwrap) } /// Extracts the file into the specified directory. /// Returns the OpenArchive for further processing /// - /// # Panics + /// # Errors /// - /// This function will panic if `base` contains nul characters. + /// - `NulError` if `base` contains nul characters. + /// - `RarError` if there was an error during extraction. pub fn extract_with_base>( self, base: P, - ) -> UnrarResult> { + ) -> Result, Nulable> { self.dir_extract(Some(base.as_ref())) } /// Extracts the file into the specified file. /// Returns the OpenArchive for further processing /// - /// # Panics + /// # Errors /// - /// This function will panic if `dest` contains nul characters. + /// - `NulError` if `base` contains nul characters. + /// - `RarError` if there was an error during extraction. pub fn extract_to>( self, file: P, - ) -> UnrarResult> { - let dest = pathed::construct(file.as_ref()); - self.process_file::(None, Some(&dest)) + ) -> Result, Nulable> { + let dest = pathed::construct(file.as_ref())?; + self.process_file::(None, Some(&dest)).map_err(Nulable::Rar) } /// extracting into a directory if the filename has unicode characters @@ -409,13 +415,13 @@ impl OpenArchive { fn dir_extract( self, base: Option<&Path>, - ) -> UnrarResult> { - let (path, file) = pathed::preprocess_extract(base, &self.entry().filename); - self.process_file::(path.as_deref(), file.as_deref()) + ) -> Result, Nulable> { + let (path, file) = pathed::preprocess_extract(base, &self.entry().filename)?; + self.process_file::(path.as_deref(), file.as_deref()).map_err(Nulable::Rar) } } -fn read_header(handle: &Handle) -> UnrarResult> { +fn read_header(handle: &Handle) -> Result, HeaderError> { let mut userdata: Userdata<::Output> = Default::default(); unsafe { native::RARSetCallback( @@ -424,14 +430,13 @@ fn read_header(handle: &Handle) -> UnrarResult> { &mut userdata as *mut _ as native::LPARAM, ); } + let mut header = native::HeaderDataEx::default(); - let read_result = - Code::from(unsafe { native::RARReadHeaderEx(handle.0.as_ptr(), &mut header as *mut _) }) - .unwrap(); + let read_result = unsafe { native::RARReadHeaderEx(handle.0.as_ptr(), &mut header as *mut _) }; match read_result { - Code::Success => Ok(Some(header.into())), - Code::EndArchive => Ok(None), - _ => Err(UnrarError::from(read_result, When::Read)), + native::ERAR_SUCCESS => Ok(Some(header.into())), + native::ERAR_END_ARCHIVE => Ok(None), + code => Err(HeaderError::try_from(code).unwrap()), } } @@ -477,9 +482,7 @@ impl ProcessMode for Test { fn process_data(_: &mut Self::Output, _: &[u8]) {} } -struct Internal { - marker: std::marker::PhantomData, -} +struct Internal(std::marker::PhantomData); impl Internal { extern "C" fn callback( @@ -519,7 +522,7 @@ impl Internal { handle: &Handle, path: Option<&pathed::RarStr>, file: Option<&pathed::RarStr>, - ) -> UnrarResult { + ) -> Result { let mut user_data: Userdata = Default::default(); unsafe { native::RARSetCallback( @@ -528,16 +531,16 @@ impl Internal { &mut user_data as *mut _ as native::LPARAM, ); } - let process_result = Code::from(pathed::process_file( + let process_result = pathed::process_file( handle.0.as_ptr(), M::OPERATION as i32, path, file, - )) - .unwrap(); + ); match process_result { - Code::Success => Ok(user_data.0), - _ => Err(UnrarError::from(process_result, When::Process)), + native::ERAR_SUCCESS => Ok(user_data.0), + code => Err(ProcessError::try_from(code) + .expect("report this issue at: https://github.com/muja/unrar.rs/issues/new")), } } } @@ -558,7 +561,7 @@ bitflags::bitflags! { /// Created using the read_header methods in an OpenArchive, contains /// information for the file that follows which is to be processed next. #[allow(missing_docs)] -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct FileHeader { pub filename: PathBuf, flags: EntryFlags, diff --git a/src/pathed/all.rs b/src/pathed/all.rs index 3c228b6..ef94240 100644 --- a/src/pathed/all.rs +++ b/src/pathed/all.rs @@ -1,11 +1,12 @@ +use super::Nulable; use std::path::{Path, PathBuf}; use widestring::{WideCString, WideCStr}; pub(crate) type RarString = WideCString; pub(crate) type RarStr = WideCStr; -pub(crate) fn construct(path: &Path) -> RarString { - WideCString::from_os_str(path).expect("Unexpected nul in path") +pub(crate) fn construct(path: &Path) -> Result> { + Ok(WideCString::from_os_str(path)?) } pub(crate) fn process_file( @@ -13,7 +14,7 @@ pub(crate) fn process_file( operation: i32, dest_path: Option<&RarStr>, dest_name: Option<&RarStr>, -) -> i32 { +) -> u32 { unsafe { unrar_sys::RARProcessFileW( handle, @@ -24,9 +25,9 @@ pub(crate) fn process_file( } } -pub(crate) fn preprocess_extract( +pub(crate) fn preprocess_extract( base: Option<&Path>, _filename: &PathBuf, -) -> (Option, Option) { - (base.map(construct), None) +) -> Result<(Option, Option), Nulable> { + base.map(construct).transpose().map(|base| (base, None)) } diff --git a/src/pathed/linux.rs b/src/pathed/linux.rs index d0a9da1..6e628e9 100644 --- a/src/pathed/linux.rs +++ b/src/pathed/linux.rs @@ -1,11 +1,12 @@ +use super::Nulable; use std::ffi::{CString, CStr}; use std::path::{Path, PathBuf}; pub(crate) type RarString = CString; pub(crate) type RarStr = CStr; -pub(crate) fn construct>(path: P) -> RarString { - CString::new(path.as_ref().as_os_str().as_encoded_bytes()).unwrap() +pub(crate) fn construct(path: &Path) -> Result> { + CString::new(path.as_os_str().as_encoded_bytes()).map_err(Nulable::from) } pub(crate) fn process_file( @@ -13,7 +14,7 @@ pub(crate) fn process_file( operation: i32, dest_path: Option<&RarStr>, dest_name: Option<&RarStr>, -) -> i32 { +) -> u32 { unsafe { unrar_sys::RARProcessFile( handle, @@ -24,9 +25,9 @@ pub(crate) fn process_file( } } -pub(crate) fn preprocess_extract( +pub(crate) fn preprocess_extract( base: Option<&Path>, filename: &PathBuf, -) -> (Option, Option) { - (None, Some(construct(base.unwrap_or(".".as_ref()).join(filename)))) +) -> Result<(Option, Option), Nulable> { + construct(&base.unwrap_or(".".as_ref()).join(filename)).map(|e| (None, Some(e))) } diff --git a/src/pathed/mod.rs b/src/pathed/mod.rs index f8da752..e74baed 100644 --- a/src/pathed/mod.rs +++ b/src/pathed/mod.rs @@ -2,4 +2,45 @@ #[cfg_attr(not(target_os = "linux"), path = "all.rs")] mod os; -pub(crate) use os::*; \ No newline at end of file +pub(crate) use os::*; +use thiserror::Error; + +/// Error for functions that convert to an ffi String representation +/// before calling into the ffi. +#[derive(Error, Debug, PartialEq, Eq, Clone, Copy)] +pub enum Nulable { + /// Underlying ffi error, meaning that conversion was successful + /// but the ffi call failed + #[error(transparent)] + Rar(#[from] E), + /// Conversion error due to a NUL character in the String + #[error("unexpected NUL at {0}")] + Nul(usize), +} + +impl Nulable { + /// returns the contained ffi error, consuming the `self` value + /// + /// # Panics + /// + /// Panics if the `self` value is `Nul` + /// + pub fn unwrap(self) -> E { + match self { + Nulable::Rar(e) => e, + nul => panic!("{nul}"), + } + } +} + +impl From> for Nulable { + fn from(e: widestring::error::ContainsNul) -> Self { + Nulable::Nul(e.nul_position()) + } +} + +impl From for Nulable { + fn from(e: std::ffi::NulError) -> Self { + Nulable::Nul(e.nul_position()) + } +} diff --git a/tests/crypted.rs b/tests/crypted.rs index 57ac4b5..ee3449c 100644 --- a/tests/crypted.rs +++ b/tests/crypted.rs @@ -1,5 +1,4 @@ use std::path::PathBuf; -use unrar::error::{Code, When}; use unrar::Archive; #[test] @@ -22,8 +21,7 @@ fn no_password() { let read_result = header.unwrap().unwrap().read(); assert!(matches!(read_result, Err(_))); let err = read_result.unwrap_err(); - assert_eq!(err.code, Code::MissingPassword); - assert_eq!(err.when, When::Process); + assert_eq!(err, unrar::ProcessError::MissingPassword); } #[test] @@ -59,8 +57,7 @@ fn no_password_list_encrypted_headers() { .open_for_listing() .unwrap(); let err = entries.next().unwrap().unwrap_err(); - assert_eq!(err.code, Code::MissingPassword); - assert_eq!(err.when, When::Read); + assert_eq!(err, unrar::IterateError::Header(unrar::HeaderError::MissingPassword)); } #[test] diff --git a/tests/simple.rs b/tests/simple.rs index 26034c1..41cc658 100644 --- a/tests/simple.rs +++ b/tests/simple.rs @@ -40,4 +40,4 @@ fn extract_to_tempdir() { let entries = std::fs::read_dir(&temp_path).expect("read tempdir").collect::, _>>().unwrap(); assert!(entries.len() == 1); assert!(entries[0].file_name() == "VERSION"); -} +} \ No newline at end of file diff --git a/unrar_sys/src/lib.rs b/unrar_sys/src/lib.rs index 86b0128..649f242 100644 --- a/unrar_sys/src/lib.rs +++ b/unrar_sys/src/lib.rs @@ -49,22 +49,22 @@ pub type WCHAR = wchar_t; // ----------------- CONSTANTS ----------------- // -pub const ERAR_SUCCESS: c_int = 0; -pub const ERAR_END_ARCHIVE: c_int = 10; -pub const ERAR_NO_MEMORY: c_int = 11; -pub const ERAR_BAD_DATA: c_int = 12; -pub const ERAR_BAD_ARCHIVE: c_int = 13; -pub const ERAR_UNKNOWN_FORMAT: c_int = 14; -pub const ERAR_EOPEN: c_int = 15; -pub const ERAR_ECREATE: c_int = 16; -pub const ERAR_ECLOSE: c_int = 17; -pub const ERAR_EREAD: c_int = 18; -pub const ERAR_EWRITE: c_int = 19; -pub const ERAR_SMALL_BUF: c_int = 20; -pub const ERAR_UNKNOWN: c_int = 21; -pub const ERAR_MISSING_PASSWORD: c_int = 22; -pub const ERAR_EREFERENCE: c_int = 23; -pub const ERAR_BAD_PASSWORD: c_int = 24; +pub const ERAR_SUCCESS: c_uint = 0; +pub const ERAR_END_ARCHIVE: c_uint = 10; +pub const ERAR_NO_MEMORY: c_uint = 11; +pub const ERAR_BAD_DATA: c_uint = 12; +pub const ERAR_BAD_ARCHIVE: c_uint = 13; +pub const ERAR_UNKNOWN_FORMAT: c_uint = 14; +pub const ERAR_EOPEN: c_uint = 15; +pub const ERAR_ECREATE: c_uint = 16; +pub const ERAR_ECLOSE: c_uint = 17; +pub const ERAR_EREAD: c_uint = 18; +pub const ERAR_EWRITE: c_uint = 19; +pub const ERAR_SMALL_BUF: c_uint = 20; +pub const ERAR_UNKNOWN: c_uint = 21; +pub const ERAR_MISSING_PASSWORD: c_uint = 22; +pub const ERAR_EREFERENCE: c_uint = 23; +pub const ERAR_BAD_PASSWORD: c_uint = 24; pub const RAR_OM_LIST: c_uint = 0; pub const RAR_OM_EXTRACT: c_uint = 1; @@ -214,25 +214,25 @@ extern "C" { pub fn RAROpenArchiveEx(data: *const OpenArchiveDataEx) -> *const Handle; - pub fn RARCloseArchive(handle: *const Handle) -> c_int; + pub fn RARCloseArchive(handle: *const Handle) -> c_uint; - pub fn RARReadHeader(handle: *const Handle, header_data: *const HeaderData) -> c_int; + pub fn RARReadHeader(handle: *const Handle, header_data: *const HeaderData) -> c_uint; - pub fn RARReadHeaderEx(handle: *const Handle, header_data: *const HeaderDataEx) -> c_int; + pub fn RARReadHeaderEx(handle: *const Handle, header_data: *const HeaderDataEx) -> c_uint; pub fn RARProcessFile( handle: *const Handle, operation: c_int, dest_path: *const c_char, dest_name: *const c_char, - ) -> c_int; + ) -> c_uint; pub fn RARProcessFileW( handle: *const Handle, operation: c_int, dest_path: *const wchar_t, dest_name: *const wchar_t, - ) -> c_int; + ) -> c_uint; pub fn RARSetCallback(handle: *const Handle, callback: Option, user_data: LPARAM); @@ -242,7 +242,7 @@ extern "C" { pub fn RARSetPassword(handle: *const Handle, password: *const c_char); - pub fn RARGetDllVersion() -> c_int; + pub fn RARGetDllVersion() -> c_uint; } // ----------------- MINIMAL ABSTRACTIONS ----------------- //