From 8ec6f220902d2994f9916a94e8ea43450b6917ab Mon Sep 17 00:00:00 2001 From: Manos Pitsidianakis Date: Mon, 6 May 2024 13:22:28 +0300 Subject: [PATCH] Use ShellExpandTrait::expand in more user-provided paths ShellExpandTrait::expand was not used consistently, leading to only some functionalities supporting things like tilde expansion. Fixes #387 Signed-off-by: Manos Pitsidianakis --- meli/src/conf.rs | 4 ++-- meli/src/mail/listing.rs | 7 +++++-- meli/src/mail/view/utils.rs | 4 ++-- meli/src/manpages.rs | 4 +++- meli/src/subcommands.rs | 7 ++++--- meli/src/types/helpers.rs | 7 ++++--- melib/src/addressbook.rs | 16 +++++++++++++--- melib/src/conf.rs | 3 ++- melib/src/maildir/backend.rs | 8 ++++---- melib/src/maildir/mod.rs | 2 +- melib/src/mbox/mod.rs | 8 ++++---- melib/src/notmuch/mod.rs | 25 +++++++++++++++++++------ melib/src/utils/shellexpand.rs | 25 ++++++++++++++++--------- 13 files changed, 79 insertions(+), 41 deletions(-) diff --git a/meli/src/conf.rs b/meli/src/conf.rs index 0e1f5c93..df03f2a7 100644 --- a/meli/src/conf.rs +++ b/meli/src/conf.rs @@ -34,7 +34,7 @@ use std::{ use melib::{ backends::{MailboxHash, TagHash}, search::Query, - SortField, SortOrder, StderrLogger, + ShellExpandTrait, SortField, SortOrder, StderrLogger, }; use crate::{ @@ -311,7 +311,7 @@ pub fn get_config_file() -> Result { .set_source(Some(std::sync::Arc::new(Box::new(err)))) })?; match env::var("MELI_CONFIG") { - Ok(path) => Ok(PathBuf::from(path)), + Ok(path) => Ok(PathBuf::from(path).expand()), Err(_) => Ok(xdg_dirs .place_config_file("config.toml") .chain_err_summary(|| { diff --git a/meli/src/mail/listing.rs b/meli/src/mail/listing.rs index 1b3e41e1..626aaa94 100644 --- a/meli/src/mail/listing.rs +++ b/meli/src/mail/listing.rs @@ -31,7 +31,8 @@ use std::{ use futures::future::try_join_all; use melib::{ - backends::EnvelopeHashBatch, mbox::MboxMetadata, utils::datetime, Flag, FlagOp, UnixTimestamp, + backends::EnvelopeHashBatch, mbox::MboxMetadata, utils::datetime, Flag, FlagOp, + ShellExpandTrait, UnixTimestamp, }; use smallvec::SmallVec; @@ -777,6 +778,7 @@ pub trait MailListingTrait: ListingTrait { if path.is_relative() { path = context.current_dir().join(&path); } + path = path.expand(); let account = &mut context.accounts[&account_hash]; let format = (*format).unwrap_or_default(); let collection = account.collection.clone(); @@ -1742,12 +1744,13 @@ impl Component for Listing { return true; } Action::Listing(ListingAction::Import(file_path, mailbox_path)) => { + let file_path = file_path.expand(); let account = &mut context.accounts[self.cursor_pos.account]; if let Err(err) = account .mailbox_by_path(mailbox_path) .and_then(|mailbox_hash| { Ok(( - std::fs::read(file_path).chain_err_summary(|| { + std::fs::read(&file_path).chain_err_summary(|| { format!("Could not read {}", file_path.display()) })?, mailbox_hash, diff --git a/meli/src/mail/view/utils.rs b/meli/src/mail/view/utils.rs index d00ddce2..87e5858d 100644 --- a/meli/src/mail/view/utils.rs +++ b/meli/src/mail/view/utils.rs @@ -21,10 +21,10 @@ use std::{fs::File, io::Write, os::unix::fs::PermissionsExt, path::Path}; -use melib::Result; +use melib::{Result, ShellExpandTrait}; pub fn save_attachment(path: &Path, bytes: &[u8]) -> Result<()> { - let mut f = File::create(path)?; + let mut f = File::create(path.expand())?; let mut permissions = f.metadata()?.permissions(); permissions.set_mode(0o600); // Read/write for owner only. f.set_permissions(permissions)?; diff --git a/meli/src/manpages.rs b/meli/src/manpages.rs index 9ca90126..eed066a7 100644 --- a/meli/src/manpages.rs +++ b/meli/src/manpages.rs @@ -28,7 +28,7 @@ use std::{ }; use flate2::bufread::GzDecoder; -use melib::log; +use melib::{log, ShellExpandTrait}; use crate::{Error, Result}; @@ -87,6 +87,7 @@ impl ManPages { pub fn install(destination: Option) -> Result { fn path_valid(p: &Path, tries: &mut Vec) -> bool { tries.push(p.into()); + let p = p.expand(); p.exists() && p.is_dir() && fs::metadata(p) @@ -116,6 +117,7 @@ impl ManPages { else { return Err(format!("Could not write to any of these paths: {:?}", tries).into()); }; + path = path.expand(); for (p, dir) in [ (Self::Main, "man1"), diff --git a/meli/src/subcommands.rs b/meli/src/subcommands.rs index 70f39fc2..bdc2a126 100644 --- a/meli/src/subcommands.rs +++ b/meli/src/subcommands.rs @@ -27,13 +27,13 @@ use std::{ }; use crossbeam::channel::{Receiver, Sender}; -use melib::Result; +use melib::{Result, ShellExpandTrait}; use crate::*; pub fn create_config(path: Option) -> Result<()> { let config_path = if let Some(path) = path { - path + path.expand() } else { conf::get_config_file()? }; @@ -131,7 +131,7 @@ pub fn compiled_with() -> Result<()> { pub fn test_config(path: Option) -> Result<()> { let config_path = if let Some(path) = path { - path + path.expand() } else { crate::conf::get_config_file()? }; @@ -144,6 +144,7 @@ pub fn view( sender: Sender, receiver: Receiver, ) -> Result { + let path = path.expand(); if !path.exists() { return Err(Error::new(format!( "`{}` is not a valid path", diff --git a/meli/src/types/helpers.rs b/meli/src/types/helpers.rs index 531903b9..991199b8 100644 --- a/meli/src/types/helpers.rs +++ b/meli/src/types/helpers.rs @@ -30,7 +30,7 @@ use std::{ path::{Path, PathBuf}, }; -use melib::{error::*, uuid::Uuid}; +use melib::{error::*, uuid::Uuid, ShellExpandTrait}; /// Temporary file that can optionally cleaned up when it is dropped. #[derive(Debug)] @@ -105,7 +105,8 @@ impl File { path.set_extension(ext); } fn inner(path: &Path, bytes: &[u8], delete_on_drop: bool) -> Result { - let mut f = std::fs::File::create(path)?; + let path = path.expand(); + let mut f = std::fs::File::create(&path)?; let metadata = f.metadata()?; let mut permissions = metadata.permissions(); @@ -115,7 +116,7 @@ impl File { f.write_all(bytes)?; f.flush()?; Ok(File { - path: path.to_path_buf(), + path, delete_on_drop, }) } diff --git a/melib/src/addressbook.rs b/melib/src/addressbook.rs index c2bb0559..7b00f640 100644 --- a/melib/src/addressbook.rs +++ b/melib/src/addressbook.rs @@ -27,6 +27,7 @@ use std::{ collections::HashMap, hash::{Hash, Hasher}, ops::Deref, + path::Path, }; use indexmap::IndexMap; @@ -35,6 +36,7 @@ use uuid::Uuid; use crate::utils::{ datetime::{now, timestamp_to_string, UnixTimestamp}, parsec::Parser, + shellexpand::ShellExpandTrait, }; #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] @@ -139,8 +141,8 @@ impl AddressBook { pub fn with_account(s: &crate::conf::AccountSettings) -> Self { let mut ret = Self::new(s.name.clone()); - if let Some(mutt_alias_file) = s.extra.get("mutt_alias_file").map(String::as_str) { - match std::fs::read_to_string(std::path::Path::new(mutt_alias_file)) + if let Some(mutt_alias_file) = s.extra.get("mutt_alias_file") { + match std::fs::read_to_string(Path::new(mutt_alias_file).expand()) .map_err(|err| err.to_string()) .and_then(|contents| { contents @@ -165,7 +167,8 @@ impl AddressBook { } #[cfg(feature = "vcard")] if let Some(vcard_path) = s.vcard_folder() { - match vcard::load_cards(std::path::Path::new(vcard_path)) { + let expanded_path = Path::new(vcard_path).expand(); + match vcard::load_cards(&expanded_path) { Ok(cards) => { for c in cards { ret.add_card(c); @@ -173,6 +176,13 @@ impl AddressBook { } Err(err) => { log::warn!("Could not load vcards from {:?}: {}", vcard_path, err); + if expanded_path.display().to_string() != vcard_path { + log::warn!( + "Note: vcard_folder was expanded from {} to {}", + vcard_path, + expanded_path.display() + ); + } } } } diff --git a/melib/src/conf.rs b/melib/src/conf.rs index 976a2892..bc5e4d97 100644 --- a/melib/src/conf.rs +++ b/melib/src/conf.rs @@ -27,6 +27,7 @@ use crate::{ backends::SpecialUsageMailbox, email::Address, error::{Error, ErrorKind, Result}, + ShellExpandTrait, }; pub use crate::{SortField, SortOrder}; @@ -107,7 +108,7 @@ impl AccountSettings { #[cfg(feature = "vcard")] { if let Some(folder) = self.extra.remove("vcard_folder") { - let path = Path::new(&folder); + let path = Path::new(&folder).expand(); if !matches!(path.try_exists(), Ok(true)) { return Err(Error::new(format!( diff --git a/melib/src/maildir/backend.rs b/melib/src/maildir/backend.rs index 992163ec..4ab80e72 100644 --- a/melib/src/maildir/backend.rs +++ b/melib/src/maildir/backend.rs @@ -1078,11 +1078,11 @@ impl MaildirType { settings: &AccountSettings, p: P, ) -> Result> { - if !p.as_ref().exists() || !p.as_ref().is_dir() { + if !p.as_ref().try_exists().unwrap_or(false) || !p.as_ref().is_dir() { return Err(Error::new(format!( "Configuration error: Path \"{}\" {}", p.as_ref().display(), - if !p.as_ref().exists() { + if !p.as_ref().try_exists().unwrap_or(false) { "does not exist." } else { "is not a directory." @@ -1147,7 +1147,7 @@ impl MaildirType { Ok(children) } let root_mailbox = PathBuf::from(&settings.root_mailbox).expand(); - if !root_mailbox.exists() { + if !root_mailbox.try_exists().unwrap_or(false) { return Err(Error::new(format!( "Configuration error ({}): root_mailbox `{}` is not a valid directory.", settings.name, @@ -1294,7 +1294,7 @@ impl MaildirType { pub fn validate_config(s: &mut AccountSettings) -> Result<()> { let root_mailbox = PathBuf::from(&s.root_mailbox).expand(); - if !root_mailbox.exists() { + if !root_mailbox.try_exists().unwrap_or(false) { return Err(Error::new(format!( "Configuration error ({}): root_mailbox `{}` is not a valid directory.", s.name, diff --git a/melib/src/maildir/mod.rs b/melib/src/maildir/mod.rs index 5ca4a760..0219b86d 100644 --- a/melib/src/maildir/mod.rs +++ b/melib/src/maildir/mod.rs @@ -138,7 +138,7 @@ impl MaildirMailbox { accept_invalid: bool, settings: &AccountSettings, ) -> Result { - let pathbuf = PathBuf::from(&path); + let pathbuf = PathBuf::from(&path).expand(); let mut h = DefaultHasher::new(); pathbuf.hash(&mut h); diff --git a/melib/src/mbox/mod.rs b/melib/src/mbox/mod.rs index a74f5957..d2215e1d 100644 --- a/melib/src/mbox/mod.rs +++ b/melib/src/mbox/mod.rs @@ -1298,7 +1298,7 @@ impl MboxType { event_consumer: BackendEventConsumer, ) -> Result> { let path = Path::new(s.root_mailbox.as_str()).expand(); - if !path.exists() { + if !path.try_exists().unwrap_or(false) { return Err(Error::new(format!( "\"root_mailbox\" {} for account {} is not a valid path.", s.root_mailbox.as_str(), @@ -1371,8 +1371,8 @@ impl MboxType { for (k, f) in s.mailboxes.iter() { if let Some(path_str) = f.extra.get("path") { let hash = MailboxHash(get_path_hash!(path_str)); - let pathbuf: PathBuf = path_str.into(); - if !pathbuf.exists() || pathbuf.is_dir() { + let pathbuf: PathBuf = Path::new(path_str).expand(); + if !pathbuf.try_exists().unwrap_or(false) || pathbuf.is_dir() { return Err(Error::new(format!( "mbox mailbox configuration entry \"{}\" path value {} is not a file.", k, path_str @@ -1451,7 +1451,7 @@ impl MboxType { }; } let path = Path::new(s.root_mailbox.as_str()).expand(); - if !path.exists() { + if !path.try_exists().unwrap_or(false) { return Err(Error::new(format!( "\"root_mailbox\" {} for account {} is not a valid path.", s.root_mailbox.as_str(), diff --git a/melib/src/notmuch/mod.rs b/melib/src/notmuch/mod.rs index 84ab424f..1b89f293 100644 --- a/melib/src/notmuch/mod.rs +++ b/melib/src/notmuch/mod.rs @@ -284,7 +284,15 @@ impl NotmuchDb { let mut dlpath = Cow::Borrowed("libnotmuch.so"); let mut custom_dlpath = false; if let Some(lib_path) = s.extra.get("library_file_path") { - dlpath = Cow::Owned(lib_path.to_string()); + let expanded_path = Path::new(lib_path).expand(); + let expanded_path_string = expanded_path.display().to_string(); + dlpath = if &expanded_path_string != lib_path + && expanded_path.try_exists().unwrap_or(false) + { + Cow::Owned(expanded_path_string) + } else { + Cow::Owned(lib_path.to_string()) + }; custom_dlpath = true; } let lib = Arc::new(NotmuchLibrary { @@ -311,7 +319,7 @@ impl NotmuchDb { dlpath, }); let mut path = Path::new(s.root_mailbox.as_str()).expand(); - if !path.exists() { + if !path.try_exists().unwrap_or(false) { return Err(Error::new(format!( "Notmuch `root_mailbox` {} for account {} does not exist.", s.root_mailbox.as_str(), @@ -328,7 +336,7 @@ impl NotmuchDb { .set_kind(ErrorKind::Configuration)); } path.push(".notmuch"); - if !path.exists() || !path.is_dir() { + if !path.try_exists().unwrap_or(false) || !path.is_dir() { return Err(Error::new(format!( "Notmuch `root_mailbox` {} for account {} does not contain a `.notmuch` \ subdirectory.", @@ -412,7 +420,7 @@ impl NotmuchDb { pub fn validate_config(s: &mut AccountSettings) -> Result<()> { let mut path = Path::new(s.root_mailbox.as_str()).expand(); - if !path.exists() { + if !path.try_exists().unwrap_or(false) { return Err(Error::new(format!( "Notmuch `root_mailbox` {} for account {} does not exist.", s.root_mailbox.as_str(), @@ -429,7 +437,7 @@ impl NotmuchDb { .set_kind(ErrorKind::Configuration)); } path.push(".notmuch"); - if !path.exists() || !path.is_dir() { + if !path.try_exists().unwrap_or(false) || !path.is_dir() { return Err(Error::new(format!( "Notmuch `root_mailbox` {} for account {} does not contain a `.notmuch` \ subdirectory.", @@ -442,7 +450,12 @@ impl NotmuchDb { let account_name = s.name.to_string(); if let Some(lib_path) = s.extra.remove("library_file_path") { - if !Path::new(&lib_path).exists() || Path::new(&lib_path).is_dir() { + let expanded_path = Path::new(&lib_path).expand(); + if (!Path::new(&lib_path).try_exists().unwrap_or(false) + || Path::new(&lib_path).is_dir()) + && !Path::new(&expanded_path).try_exists().unwrap_or(false) + || Path::new(&expanded_path).is_dir() + { return Err(Error::new(format!( "Notmuch `library_file_path` setting value `{}` for account {} does not exist \ or is a directory.", diff --git a/melib/src/utils/shellexpand.rs b/melib/src/utils/shellexpand.rs index 623505f0..4a378f65 100644 --- a/melib/src/utils/shellexpand.rs +++ b/melib/src/utils/shellexpand.rs @@ -29,25 +29,36 @@ use std::{ use smallvec::SmallVec; +// [ref:needs_dev_doc]: ShellExpandTrait +// [ref:needs_unit_test]: ShellExpandTrait pub trait ShellExpandTrait { + // [ref:needs_dev_doc] fn expand(&self) -> PathBuf; + // [ref:needs_dev_doc] fn complete(&self, force: bool) -> SmallVec<[String; 128]>; } impl ShellExpandTrait for Path { fn expand(&self) -> PathBuf { + // [ref:TODO]: ShellExpandTrait: add support for parameters in braces ${ } + // https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02 let mut ret = PathBuf::new(); for c in self.components() { - let c_to_str = c.as_os_str().to_str(); + let c_to_str = c.as_os_str(); match c_to_str { - Some("~") => { + tilde if tilde == "~" => { if let Ok(home_dir) = std::env::var("HOME") { ret.push(home_dir) } else { - return PathBuf::new(); + // POSIX says that if HOME is unset, the results of tilde expansion is + // unspecified. + // https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_01 + // Abort expansion. + return self.to_path_buf(); } } - Some(var) if var.starts_with('$') => { + var if var.to_string_lossy().starts_with('$') => { + let var = var.to_string_lossy(); let env_name = var.split_at(1).1; if env_name.chars().all(char::is_uppercase) { ret.push(std::env::var(env_name).unwrap_or_default()); @@ -55,13 +66,9 @@ impl ShellExpandTrait for Path { ret.push(c); } } - Some(_) => { + _ => { ret.push(c); } - None => { - /* path is invalid */ - return PathBuf::new(); - } } } ret