diff --git a/pkgar-core/src/error.rs b/pkgar-core/src/error.rs index d7fd5a068fc00484562c7aebbac6e23ade28475e..c3ef77e1b5a4488e87caf2685a47c898280d4f35 100644 --- a/pkgar-core/src/error.rs +++ b/pkgar-core/src/error.rs @@ -14,7 +14,6 @@ pub enum Error { TryFromInt(core::num::TryFromIntError), } -//TODO: Improve Error messages impl Display for Error { fn fmt(&self, f: &mut Formatter) -> Result { use Error::*; diff --git a/pkgar-keys/src/error.rs b/pkgar-keys/src/error.rs index 4dff76b1f743a266a183928a3d3b0b86e6e0071a..d8679e2fb56a412d25d71f2d79ef82547ffd8ce0 100644 --- a/pkgar-keys/src/error.rs +++ b/pkgar-keys/src/error.rs @@ -1,5 +1,5 @@ use std::io; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use error_chain::error_chain; use user_error::UFE; @@ -47,44 +47,16 @@ error_chain! { impl UFE for Error {} -/* -/// An error which includes path context and implements `UFE` for easy display. -#[derive(Debug, Error)] -#[error("File: {path}")] -pub struct Error { - #[source] - pub src: ErrorKind, - pub path: PathBuf, +// Allow .chain_err(|| path ) +impl From<&Path> for ErrorKind { + fn from(path: &Path) -> ErrorKind { + ErrorKind::Path(path.to_path_buf()) + } } -/// The main error type that is used by this library internally. For additional -/// contextual information, most public routines use [`Error`](struct.Error.html). -#[non_exhaustive] -#[derive(Debug, Error)] -pub enum ErrorKind { - #[error("Io")] - Io(#[from] io::Error), - - #[error("Key length invalid")] - KeyInvalid, - - #[error("Public and secret keys do not match")] - KeyMismatch, - - #[error("Invalid nonce length")] - NonceInvalid, - - #[error("Incorrect passphrase")] - PassphraseIncorrect, - - #[error("Passphrases did not match")] - PassphraseMismatch, - - #[error("Serialization")] - Ser(#[from] toml::ser::Error), - - #[error("Deserialization")] - Deser(#[from] toml::de::Error), +impl From<&PathBuf> for ErrorKind { + fn from(path: &PathBuf) -> ErrorKind { + ErrorKind::Path(path.clone()) + } } -*/ diff --git a/pkgar-keys/src/lib.rs b/pkgar-keys/src/lib.rs index b079c83dbf87813690bb1474273bc492f5e9274a..788ad03c60eb56132d2e2bda78b7b5aa2a30ccf0 100644 --- a/pkgar-keys/src/lib.rs +++ b/pkgar-keys/src/lib.rs @@ -85,10 +85,10 @@ impl PublicKeyFile { /// Parse a `PublicKeyFile` from `file` (in toml format). pub fn open(file: impl AsRef<Path>) -> Result<PublicKeyFile, Error> { let content = fs::read_to_string(&file) - .chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )?; + .chain_err(|| file.as_ref() )?; toml::from_str(&content) - .chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) ) + .chain_err(|| file.as_ref() ) } /// Write `self` serialized as toml to `w`. @@ -101,8 +101,8 @@ impl PublicKeyFile { pub fn save(&self, file: impl AsRef<Path>) -> Result<(), Error> { self.write( File::create(&file) - .chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )? - ).chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) ) + .chain_err(|| file.as_ref() )? + ).chain_err(|| file.as_ref() ) } } @@ -206,10 +206,10 @@ impl SecretKeyFile { /// Parse a `SecretKeyFile` from `file` (in toml format). pub fn open(file: impl AsRef<Path>) -> Result<SecretKeyFile, Error> { let content = fs::read_to_string(&file) - .chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )?; + .chain_err(|| file.as_ref() )?; toml::from_str(&content) - .chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) ) + .chain_err(|| file.as_ref() ) } /// Write `self` serialized as toml to `w`. @@ -229,8 +229,8 @@ impl SecretKeyFile { .create(true) .mode(0o600) .open(&file) - .chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )? - ).chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) ) + .chain_err(|| file.as_ref() )? + ).chain_err(|| file.as_ref() ) } /// Ensure that the internal state of this struct is encrypted. @@ -358,7 +358,7 @@ impl Eq for Passwd {} /// directories will not be created. pub fn gen_keypair(pkey_path: &Path, skey_path: &Path) -> Result<(PublicKeyFile, SecretKeyFile), Error> { let passwd = Passwd::prompt_new() - .chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?; + .chain_err(|| skey_path )?; let (pkey_file, mut skey_file) = SecretKeyFile::new(); @@ -376,9 +376,9 @@ fn prompt_skey(skey_path: &Path, prompt: impl AsRef<str>) -> Result<SecretKeyFil if key_file.is_encrypted() { let passwd = Passwd::prompt(&format!("{} {}: ", prompt.as_ref(), skey_path.display())) - .chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?; + .chain_err(|| skey_path )?; key_file.decrypt(passwd) - .chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?; + .chain_err(|| skey_path )?; } Ok(key_file) } @@ -395,7 +395,7 @@ pub fn re_encrypt(skey_path: &Path) -> Result<(), Error> { let mut skey_file = prompt_skey(skey_path, "Old passphrase for")?; let passwd = Passwd::prompt_new() - .chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?; + .chain_err(|| skey_path )?; skey_file.encrypt(passwd); skey_file.save(skey_path) diff --git a/pkgar-keys/src/main.rs b/pkgar-keys/src/main.rs index ae9e64074e12394fbbf0f39ecd1b00100f5844dd..f324933234a795c7dab9bdad4a0831af6dddc250 100644 --- a/pkgar-keys/src/main.rs +++ b/pkgar-keys/src/main.rs @@ -1,6 +1,6 @@ use std::io; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process; use clap::clap_app; @@ -54,7 +54,7 @@ fn cli() -> Result<i32, Error> { "gen" => { if let Some(keydir) = skey_path.parent() { fs::create_dir_all(&keydir) - .chain_err(|| ErrorKind::Path(keydir.to_path_buf()) )?; + .chain_err(|| keydir )?; } if ! submatches.is_present("force") { @@ -62,7 +62,7 @@ fn cli() -> Result<i32, Error> { return Err(Error::from_kind(ErrorKind::Io( io::Error::from(io::ErrorKind::AlreadyExists) ))) - .chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) ); + .chain_err(|| &skey_path ); } } @@ -87,7 +87,7 @@ fn cli() -> Result<i32, Error> { pkey.save(file)?; } else { pkey.write(io::stdout().lock()) - .chain_err(|| ErrorKind::Path(PathBuf::from("stdout")) )?; + .chain_err(|| Path::new("stdout") )?; } }, "rencrypt" => { diff --git a/pkgar/src/bin.rs b/pkgar/src/bin.rs index e0173f530c69b1786423d5914867bc4012c299e7..1fcb2625163a78b459ac500d505a1f49718a3402 100644 --- a/pkgar/src/bin.rs +++ b/pkgar/src/bin.rs @@ -94,12 +94,12 @@ pub fn create( .create(true) .truncate(true) .open(&archive_path) - .chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )?; + .chain_err(|| archive_path.as_ref() )?; // Create a list of entries let mut entries = Vec::new(); folder_entries(&folder, &folder, &mut entries) - .chain_err(|| ErrorKind::Path(folder.as_ref().to_path_buf()) ) + .chain_err(|| folder.as_ref() ) .chain_err(|| "Recursing buildroot" )?; // Create initial header @@ -117,13 +117,14 @@ pub fn create( for entry in &mut entries { entry.offset = data_size; data_size = data_size.checked_add(entry.size) - .ok_or(pkgar_core::Error::Overflow)?; - //.chain_err(|| ErrorKind::Entry(*entry) )?; + .ok_or(pkgar_core::Error::Overflow) + .map_err(Error::from) + .chain_err(|| ErrorKind::Entry(*entry) )?; } let data_offset = header.total_size()?; archive_file.seek(SeekFrom::Start(data_offset as u64)) - .chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) ) + .chain_err(|| archive_path.as_ref() ) .chain_err(|| format!("Seek to {} (data offset)", data_offset) )?; //TODO: fallocate data_offset + data_size @@ -144,19 +145,19 @@ pub fn create( let mut entry_file = fs::OpenOptions::new() .read(true) .open(&path) - .chain_err(|| ErrorKind::Path(path.to_path_buf()) )?; + .chain_err(|| &path )?; copy_and_hash(&mut entry_file, &mut archive_file, &mut buf) - .chain_err(|| ErrorKind::Path(path.to_path_buf()) ) + .chain_err(|| &path ) .chain_err(|| format!("Writing entry to archive: '{}'", relative.display()) )? }, Mode::SYMLINK => { let destination = fs::read_link(&path) - .chain_err(|| ErrorKind::Path(path.to_path_buf()) )?; + .chain_err(|| &path )?; let mut data = destination.as_os_str().as_bytes(); copy_and_hash(&mut data, &mut archive_file, &mut buf) - .chain_err(|| ErrorKind::Path(path.to_path_buf()) ) + .chain_err(|| &path ) .chain_err(|| format!("Writing entry to archive: '{}'", relative.display()) )? }, _ => return Err(Error::from( @@ -182,12 +183,12 @@ pub fn create( // Write archive header archive_file.seek(SeekFrom::Start(0)) - .chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )?; + .chain_err(|| archive_path.as_ref() )?; archive_file.write_all(unsafe { plain::as_bytes(&header) }) - .chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )?; + .chain_err(|| archive_path.as_ref() )?; // Write each entry header for entry in &entries { @@ -195,7 +196,7 @@ pub fn create( archive_file.write_all(unsafe { plain::as_bytes(entry) }) - .chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) ) + .chain_err(|| archive_path.as_ref() ) .chain_err(|| format!("Write entry {}", checked_path.display()) )?; } @@ -262,7 +263,7 @@ pub fn verify( .join(entry.check_path()?); let expected = File::open(&expected_path) - .chain_err(|| ErrorKind::Path(expected_path.to_path_buf()) )?; + .chain_err(|| &expected_path )?; let (count, hash) = copy_and_hash(expected, io::sink(), &mut buf)?; diff --git a/pkgar/src/ext.rs b/pkgar/src/ext.rs index 4f8ceb986ab4c583d300c231105aafffd3b1cae3..dc2f81353f43ee6345cf1adf22bc6ec452ba22cc 100644 --- a/pkgar/src/ext.rs +++ b/pkgar/src/ext.rs @@ -67,7 +67,6 @@ pub trait PackageSrcExt /// A reader that provides acess to one entry's data within a `PackageSrc`. /// Use `PackageSrcExt::entry_reader` for construction -//TODO: Fix the types for this pub struct EntryReader<'a, Src> where Src: PackageSrc { @@ -84,6 +83,8 @@ impl<Src, E> Read for EntryReader<'_, Src> fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { let count = self.src.read_entry(self.entry, self.pos, buf) // This is a little painful, since e is pkgar::Error... + // However, this is likely to be a very rarely triggered error + // condition. .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string()) )?; diff --git a/pkgar/src/lib.rs b/pkgar/src/lib.rs index a700e86bfcc5dd702c7576ec0c5a6a75dbdb9533..b24ef367a2f740c3f9aac2fce7266c529ee64c1d 100644 --- a/pkgar/src/lib.rs +++ b/pkgar/src/lib.rs @@ -67,6 +67,19 @@ error_chain! { impl UFE for Error {} +// Allow .chain_err(|| path ) +impl From<&Path> for ErrorKind { + fn from(path: &Path) -> ErrorKind { + ErrorKind::Path(path.to_path_buf()) + } +} + +impl From<&PathBuf> for ErrorKind { + fn from(path: &PathBuf) -> ErrorKind { + ErrorKind::Path(path.clone()) + } +} + // Unfortunately error_chain does not handle types that don't implement // std::error::Error very well. impl From<pkgar_core::Error> for Error { @@ -75,11 +88,6 @@ impl From<pkgar_core::Error> for Error { } } -impl From<&Path> for ErrorKind { - fn from(path: &Path) -> ErrorKind { - ErrorKind::Path(path.to_path_buf()) - } -} /* // Apparently this conflicts with the first implementation; Just use map_err // with Error::from for chaining errors to a pkgar_core Result. @@ -94,154 +102,3 @@ impl<T> ResultExt<T> for Result<T, pkgar_core::Error> { } }*/ -/* -/// This mimics the way std::io::Error works, to manage adding context in an -/// adequate manner, without too much boilderplate. -#[derive(Debug, Error)] -enum Repr { - /// pkgar_keys::Error is very high level and already contains path context - #[error(transparent)] - Keys(#[from] pkgar_keys::Error), - - /// Ideally this should never make it all the way back to the user without - /// being converted into a Complex error. - #[error(transparent)] - Kind(ErrorKind), - - #[error("{}", match (path, reason) { - (Some(path), Some(reason)) => format!("{}: {}", reason, path.display()), - (Some(path), None) => format!("File: {}", path.display()), - (None, Some(reason)) => reason.clone(), - (None, None) => String::new(), - })] - Complex { - path: Option<PathBuf>, - reason: Option<String>, - entry: Option<Entry>, - src: ErrorKind, - } -} - -impl Repr { - fn as_complex( - self, - new_path: Option<PathBuf>, - new_reason: Option<String>, - new_entry: Option<Entry>, - ) -> Repr { - match self { - Repr::Kind(src) => Repr::Complex { - path: new_path, - reason: new_reason, - entry: new_entry, - src, - }, - Repr::Complex { path, reason, entry, src } => Repr::Complex { - path: new_path.or(path), - reason: new_reason.or(reason), - entry: new_entry.or(entry), - src, - }, - _ => self, - } - } -} - -/// Primary error type for pkgar. Provides optional path and reason context -/// for the underlying [`ErrorKind`](enum.ErrorKind.html). -#[derive(Debug, Error)] -#[error(transparent)] -pub struct Error { - #[from] - repr: Repr, -} - -impl Error { - /// Set the path associated with this error. Calling `path()` multiple - /// times will only store the most recently added path. - /// - /// Most pkgar APIs will have already called `path()`; therefore, it's - /// unlikely that consumers of the library will need to use this function. - pub fn path(mut self, path: impl AsRef<Path>) -> Error { - let path = Some(path.as_ref().to_path_buf()); - self.repr = self.repr.as_complex(path, None, None); - self - } - - /// Set the reason associated with this error. Calling `reason()` multiple - /// times will only store the most recently added reason. - /// - /// Most pkgar APIs will have already called `reason()`; therefore, it's - /// unlikely that consumers of the library will need to use this function. - pub fn reason(mut self, reason: impl ToString) -> Error { - let reason = Some(reason.to_string()); - self.repr = self.repr.as_complex(None, reason, None); - self - } - - pub fn entry(mut self, entry: Entry) -> Error { - self.repr = self.repr.as_complex(None, None, Some(entry)); - self - } -} - -impl<K> From<K> for Error - where K: Into<ErrorKind> -{ - fn from(e: K) -> Error { - Error::from(Repr::Kind(e.into())) - } -} - -impl From<pkgar_keys::Error> for Error { - fn from(e: pkgar_keys::Error) -> Error { - Error::from(Repr::Keys(e)) - } -} - - -/// Primary enumeration of error types producible by the pkgar crate. Most -/// interaction with this type will be via [`Error`](struct.Error.html). -#[derive(Debug, Error)] -#[non_exhaustive] -pub enum ErrorKind { - #[error("Io")] - Io(#[from] io::Error), - - #[error("Failed to commit transaction. {changed} files changed, {remaining} files remaining")] - FailedCommit { - changed: usize, - remaining: usize, - #[source] - source: io::Error, - }, - - #[error("Package: {0}")] - Core(pkgar_core::Error), - - #[error("Invalid path component: {0}")] - InvalidPathComponent(PathBuf), - - #[error("Entry size mismatch: expected {expected}, got {actual}")] - LengthMismatch { - actual: u64, - expected: u64, - }, - - #[error("Invalid Mode Kind: {0:#o}")] - InvalidModeKind(Mode), -} - -impl ErrorKind { - pub fn as_error(self) -> Error { - Error::from(self) - } -} - -// Core::Error doesn't implement std::Error, so thiserror won't generate this impl -impl From<pkgar_core::Error> for ErrorKind { - fn from(err: pkgar_core::Error) -> ErrorKind { - ErrorKind::Core(err) - } -} -*/ diff --git a/pkgar/src/package.rs b/pkgar/src/package.rs index 3f15ba7f0d768ea766473e6acf32f7b5f412c66f..dd831904b61547db0a487276e51ab6e941fa28d8 100644 --- a/pkgar/src/package.rs +++ b/pkgar/src/package.rs @@ -26,7 +26,7 @@ impl PackageFile { let file = OpenOptions::new() .read(true) .open(&path) - .chain_err(|| path.as_path() )?; + .chain_err(|| &path )?; let mut new = PackageFile { path, diff --git a/pkgar/src/transaction.rs b/pkgar/src/transaction.rs index 9ea949acf9b9e3067b7727e9550541bb0df01154..575544df304de009ce057ba7c75aa645e5e7b509 100644 --- a/pkgar/src/transaction.rs +++ b/pkgar/src/transaction.rs @@ -46,11 +46,11 @@ fn temp_path(target_path: impl AsRef<Path>, entry_hash: Hash) -> Result<PathBuf, hash_path }; - let parent = target_path.parent() + let parent_dir = target_path.parent() .ok_or(ErrorKind::InvalidPathComponent(PathBuf::from("/")))?; - fs::create_dir_all(parent) - .chain_err(|| parent )?; - Ok(parent.join(tmp_name)) + fs::create_dir_all(parent_dir) + .chain_err(|| parent_dir )?; + Ok(parent_dir.join(tmp_name)) } enum Action { @@ -63,16 +63,16 @@ impl Action { fn commit(&self) -> Result<(), Error> { match self { Action::Rename(tmp, target) => fs::rename(&tmp, target) - .chain_err(|| tmp.as_path() ), + .chain_err(|| tmp ), Action::Remove(target) => fs::remove_file(&target) - .chain_err(|| target.as_path() ), + .chain_err(|| target ), } } fn abort(&self) -> Result<(), Error> { match self { Action::Rename(tmp, _) => fs::remove_file(&tmp) - .chain_err(|| tmp.as_path() ), + .chain_err(|| tmp ), Action::Remove(_) => Ok(()), } } @@ -122,13 +122,13 @@ impl Transaction { .chain_err(|| tmp_path.as_path() )?; copy_and_hash(src.entry_reader(entry), &mut tmp_file, &mut buf) - .chain_err(|| tmp_path.as_path() ) + .chain_err(|| &tmp_path ) .chain_err(|| format!("Copying entry to tempfile: '{}'", relative_path.display()) )? }, Mode::SYMLINK => { let mut data = Vec::new(); let (size, hash) = copy_and_hash(src.entry_reader(entry), &mut data, &mut buf) - .chain_err(|| tmp_path.as_path() ) + .chain_err(|| &tmp_path ) .chain_err(|| format!("Copying entry to tempfile: '{}'", relative_path.display()) )?; let sym_target = Path::new(OsStr::from_bytes(&data)); @@ -205,11 +205,11 @@ impl Transaction { "target path was not in the base path"); let candidate = File::open(&target_path) - .chain_err(|| target_path.as_path() )?; + .chain_err(|| &target_path )?; // Ensure that the deletion candidate on disk has not been modified copy_and_hash(candidate, io::sink(), &mut buf) - .chain_err(|| target_path.as_path() ) + .chain_err(|| &target_path ) .chain_err(|| format!("Hashing file for entry: '{}'", relative_path.display()) )?; actions.push(Action::Remove(target_path)); @@ -225,7 +225,9 @@ impl Transaction { if let Err(err) = action.commit() { // Should be possible to restart a failed transaction self.actions.push(action); - return Err(err.chain_err(|| ErrorKind::FailedCommit(count, self.actions.len()) )); + return Err(err + .chain_err(|| ErrorKind::FailedCommit(count, self.actions.len()) ) + ); } count += 1; } @@ -244,8 +246,10 @@ impl Transaction { // This is inherently inefficent, no biggie self.actions.insert(0, action); if last_failed { - //TODO: Somehow indicate that this is a failed abort instead of a commit - return Err(err.chain_err(|| ErrorKind::FailedCommit(count, self.actions.len()) )); + return Err(err + .chain_err(|| ErrorKind::FailedCommit(count, self.actions.len()) ) + .chain_err(|| "Abort triggered" ) + ); } else { last_failed = true; }