diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ab09462..4302f5f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Parse error on Cygwin/MSYS due to CRLF line endings. - Fzf: handle spaces correctly in preview window. - Bash: avoid initializing completions on older versions. +- Fzf: avoid launching binary from current directory on Windows. ## [0.7.9] - 2021-11-02 diff --git a/Cargo.lock b/Cargo.lock index 937b7ba7..3cf63829 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -679,6 +679,26 @@ version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0066c8d12af8b5acd21e00547c3797fde4e8677254a7ee429176ccebbe93dd80" +[[package]] +name = "thiserror" +version = "1.0.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "854babe52e4df1653706b98fcfc05843010039b406875930a70e4d9644e5c417" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa32fd3f627f367fe16f893e2597ae3c05020f8bba2666a4e6ea73d377e5714b" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "thread_local" version = "1.1.3" @@ -792,4 +812,5 @@ dependencies = [ "rstest", "serde", "tempfile", + "thiserror", ] diff --git a/Cargo.toml b/Cargo.toml index 2c5d5791..82f6a54d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ glob = "0.3.0" ordered-float = "2.0.0" serde = { version = "1.0.116", features = ["derive"] } tempfile = "3.1.0" +thiserror = "1.0.30" [target.'cfg(windows)'.dependencies] rand = { version = "0.8.4", features = [ diff --git a/src/db/mod.rs b/src/db/mod.rs index 7ae5fcb1..08062f8c 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -120,6 +120,12 @@ impl<'file> Database<'file> { } } +#[cfg(unix)] +fn persist>(file: NamedTempFile, path: P) -> Result<(), PersistError> { + file.persist(path)?; + Ok(()) +} + #[cfg(windows)] fn persist>(mut file: NamedTempFile, path: P) -> Result<(), PersistError> { use std::thread; @@ -152,12 +158,6 @@ fn persist>(mut file: NamedTempFile, path: P) -> Result<(), Persi Ok(()) } -#[cfg(unix)] -fn persist>(file: NamedTempFile, path: P) -> Result<(), PersistError> { - file.persist(path)?; - Ok(()) -} - pub struct DatabaseFile { buffer: Vec, data_dir: PathBuf, diff --git a/src/error.rs b/src/error.rs index b4fd35a8..782e59f9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,20 +1,19 @@ -use std::fmt::{self, Display, Formatter}; use std::io; use anyhow::{bail, Context, Result}; +use thiserror::Error; + +#[derive(Debug, Error)] +#[error("could not find fzf, is it installed?")] +pub struct FzfNotFound; /// Custom error type for early exit. -#[derive(Debug)] +#[derive(Debug, Error)] +#[error("")] pub struct SilentExit { pub code: i32, } -impl Display for SilentExit { - fn fmt(&self, _: &mut Formatter) -> fmt::Result { - Ok(()) - } -} - pub trait BrokenPipeHandler { fn pipe_exit(self, device: &str) -> Result<()>; } diff --git a/src/fzf.rs b/src/fzf.rs index aacdb730..033cba54 100644 --- a/src/fzf.rs +++ b/src/fzf.rs @@ -1,11 +1,11 @@ use std::io::{self, Read}; use std::mem; -use std::process::{Child, ChildStdin, Command, Stdio}; +use std::process::{Child, ChildStdin, Stdio}; use anyhow::{bail, Context, Result}; -use crate::config; -use crate::error::SilentExit; +use crate::error::{FzfNotFound, SilentExit}; +use crate::{config, util}; pub struct Fzf { child: Child, @@ -13,7 +13,8 @@ pub struct Fzf { impl Fzf { pub fn new(multiple: bool) -> Result { - let mut command = Command::new("fzf"); + let bin = if cfg!(windows) { "fzf.exe" } else { "fzf" }; + let mut command = util::get_command(bin).map_err(|_| FzfNotFound)?; if multiple { command.arg("-m"); } @@ -37,9 +38,7 @@ impl Fzf { let child = match command.spawn() { Ok(child) => child, - Err(e) if e.kind() == io::ErrorKind::NotFound => { - bail!("could not find fzf, is it installed?") - } + Err(e) if e.kind() == io::ErrorKind::NotFound => bail!(FzfNotFound), Err(e) => Err(e).context("could not launch fzf")?, }; diff --git a/src/util.rs b/src/util.rs index 8f1b7e76..16025c78 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,5 +1,6 @@ use std::env; use std::path::{Component, Path, PathBuf}; +use std::process::Command; use std::time::SystemTime; use anyhow::{bail, Context, Result}; @@ -21,6 +22,35 @@ pub fn current_time() -> Result { Ok(current_time) } +/// Constructs a new [`Command`] for launching the program with the name +/// `program`. +/// +/// On Windows, CreateProcess implicitly searches the current working directory +/// for the executable, which is a potential security issue. Instead, we resolve +/// the path to the executable and then pass it to CreateProcess. +/// +/// On other platforms, this is a no-op. +/// +pub fn get_command>(program: P) -> Result { + let program = program.as_ref(); + if !cfg!(windows) { + return Ok(Command::new(program)); + } + + let paths = env::var_os("PATH").context("PATH environment variable not set")?; + for path in env::split_paths(&paths) { + if path.as_os_str().is_empty() { + continue; + } + let path = path.join(program); + if path.metadata().map_or(false, |m| !m.is_dir()) { + return Ok(Command::new(path)); + } + } + + bail!("executable not found in PATH: {}", program.display()); +} + pub fn path_to_str>(path: &P) -> Result<&str> { let path = path.as_ref(); path.to_str().with_context(|| format!("invalid unicode in path: {}", path.display()))