From f01846bd443aaf92fdd5ac20f461beac3f6ee3fd Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sat, 10 Jul 2021 15:52:06 +0100 Subject: Do not resolve executables as relative path from current directory (#658) * Refactor: child pager process creation * Protect calls to Command::new behind grep_cli::resolve_binary * Move less env-var setting into less-specific branch --- Cargo.lock | 57 ++++++++++++++++++ Cargo.toml | 1 + src/bat_utils/less.rs | 8 ++- src/bat_utils/output.rs | 155 ++++++++++++++++++++++++++++-------------------- src/main.rs | 6 +- 5 files changed, 161 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d29765c7..001e8588 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -95,6 +95,17 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea27d8d5fd867b17523bf6788b1175fa9867f34669d057e9adaf76e27bcea44b" +[[package]] +name = "bstr" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a40b47ad93e1a5404e6c18dec46b628214fee441c70f4ab5d6942142cc268a3d" +dependencies = [ + "lazy_static", + "memchr", + "regex-automata", +] + [[package]] name = "bytelines" version = "2.2.2" @@ -272,6 +283,7 @@ dependencies = [ "dirs-next", "error-chain", "git2", + "grep-cli", "itertools", "lazy_static", "pathdiff", @@ -298,6 +310,36 @@ dependencies = [ "url", ] +[[package]] +name = "globset" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10463d9ff00a2a068db14231982f5132edebad0d7660cd956a1c30292dbcbfbd" +dependencies = [ + "aho-corasick", + "bstr", + "fnv", + "log", + "regex", +] + +[[package]] +name = "grep-cli" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dd110c34bb4460d0de5062413b773e385cbf8a85a63fc535590110a09e79e8a" +dependencies = [ + "atty", + "bstr", + "globset", + "lazy_static", + "log", + "regex", + "same-file", + "termcolor", + "winapi-util", +] + [[package]] name = "hashbrown" version = "0.8.2" @@ -600,6 +642,12 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "regex-automata" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" + [[package]] name = "regex-syntax" version = "0.6.22" @@ -724,6 +772,15 @@ dependencies = [ "yaml-rust", ] +[[package]] +name = "termcolor" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" +dependencies = [ + "winapi-util", +] + [[package]] name = "terminal_size" version = "0.1.15" diff --git a/Cargo.toml b/Cargo.toml index 99a5d6e1..e6fd0c95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ box_drawing = "0.1.2" bytelines = "2.2.2" console = "0.14.1" dirs-next = "2.0.0" +grep-cli = "0.1.6" itertools = "0.10.1" lazy_static = "1.4" pathdiff = "0.2.0" diff --git a/src/bat_utils/less.rs b/src/bat_utils/less.rs index 79ecd43b..1ca9f76f 100644 --- a/src/bat_utils/less.rs +++ b/src/bat_utils/less.rs @@ -1,8 +1,12 @@ use std::process::Command; pub fn retrieve_less_version() -> Option { - let cmd = Command::new("less").arg("--version").output().ok()?; - parse_less_version(&cmd.stdout) + if let Ok(less_path) = grep_cli::resolve_binary("less") { + let cmd = Command::new(less_path).arg("--version").output().ok()?; + parse_less_version(&cmd.stdout) + } else { + None + } } fn parse_less_version(output: &[u8]) -> Option { diff --git a/src/bat_utils/output.rs b/src/bat_utils/output.rs index 1166b4e2..51819787 100644 --- a/src/bat_utils/output.rs +++ b/src/bat_utils/output.rs @@ -77,78 +77,35 @@ impl OutputType { let pagerflags = shell_words::split(&pager).chain_err(|| "Could not parse pager command.")?; - match pagerflags.split_first() { + Ok(match pagerflags.split_first() { Some((pager_name, args)) => { let pager_path = PathBuf::from(pager_name); let is_less = pager_path.file_stem() == Some(&OsString::from("less")); - let mut process = if is_less { - let mut p = Command::new(&pager_path); - if args.is_empty() || replace_arguments_to_less { - p.args(vec!["--RAW-CONTROL-CHARS"]); - - // Passing '--no-init' fixes a bug with '--quit-if-one-screen' in older - // versions of 'less'. Unfortunately, it also breaks mouse-wheel support. - // - // See: http://www.greenwoodsoftware.com/less/news.530.html - // - // For newer versions (530 or 558 on Windows), we omit '--no-init' as it - // is not needed anymore. - match retrieve_less_version() { - None => { - p.arg("--no-init"); - } - Some(version) - if (version < 530 || (cfg!(windows) && version < 558)) => - { - p.arg("--no-init"); - } - _ => {} - } - - if quit_if_one_screen { - p.arg("--quit-if-one-screen"); - } - } else { - p.args(args); - } - p.env("LESSCHARSET", "UTF-8"); - p + let process = if is_less { + _make_process_from_less_path( + pager_path, + args, + replace_arguments_to_less, + quit_if_one_screen, + config, + ) } else { - if pager_path.file_stem() == Some(&OsString::from("delta")) { - eprintln!( - "\ -It looks like you have set delta as the value of $PAGER. \ -This would result in a non-terminating recursion. \ -delta is not an appropriate value for $PAGER \ -(but it is an appropriate value for $GIT_PAGER)." - ); - std::process::exit(1); - } - let mut p = Command::new(&pager_path); - p.args(args); - p + _make_process_from_pager_path(pager_path, args) }; - if is_less && config.navigate { - if let Ok(hist_file) = - navigate::copy_less_hist_file_and_append_navigate_regexp(config) - { - process.env("LESSHISTFILE", hist_file); - if config.show_themes { - process.arg("+n"); - } - } + if let Some(mut process) = process { + process + .stdin(Stdio::piped()) + .spawn() + .map(OutputType::Pager) + .unwrap_or_else(|_| OutputType::stdout()) + } else { + OutputType::stdout() } - Ok(process - .env("LESSANSIENDCHARS", "mK") - .stdin(Stdio::piped()) - .spawn() - .map(OutputType::Pager) - .unwrap_or_else(|_| OutputType::stdout())) } - None => Ok(OutputType::stdout()), - } + None => OutputType::stdout(), + }) } fn stdout() -> Self { @@ -166,6 +123,78 @@ delta is not an appropriate value for $PAGER \ } } +fn _make_process_from_less_path( + less_path: PathBuf, + args: &[String], + replace_arguments_to_less: bool, + quit_if_one_screen: bool, + config: &config::Config, +) -> Option { + if let Ok(less_path) = grep_cli::resolve_binary(less_path) { + let mut p = Command::new(&less_path); + if args.is_empty() || replace_arguments_to_less { + p.args(vec!["--RAW-CONTROL-CHARS"]); + + // Passing '--no-init' fixes a bug with '--quit-if-one-screen' in older + // versions of 'less'. Unfortunately, it also breaks mouse-wheel support. + // + // See: http://www.greenwoodsoftware.com/less/news.530.html + // + // For newer versions (530 or 558 on Windows), we omit '--no-init' as it + // is not needed anymore. + match retrieve_less_version() { + None => { + p.arg("--no-init"); + } + Some(version) if (version < 530 || (cfg!(windows) && version < 558)) => { + p.arg("--no-init"); + } + _ => {} + } + + if quit_if_one_screen { + p.arg("--quit-if-one-screen"); + } + } else { + p.args(args); + } + p.env("LESSCHARSET", "UTF-8"); + p.env("LESSANSIENDCHARS", "mK"); + if config.navigate { + if let Ok(hist_file) = navigate::copy_less_hist_file_and_append_navigate_regexp(config) + { + p.env("LESSHISTFILE", hist_file); + if config.show_themes { + p.arg("+n"); + } + } + } + Some(p) + } else { + None + } +} + +fn _make_process_from_pager_path(pager_path: PathBuf, args: &[String]) -> Option { + if pager_path.file_stem() == Some(&OsString::from("delta")) { + eprintln!( + "\ +It looks like you have set delta as the value of $PAGER. \ +This would result in a non-terminating recursion. \ +delta is not an appropriate value for $PAGER \ +(but it is an appropriate value for $GIT_PAGER)." + ); + std::process::exit(1); + } + if let Ok(pager_path) = grep_cli::resolve_binary(pager_path) { + let mut p = Command::new(&pager_path); + p.args(args); + Some(p) + } else { + None + } +} + impl Drop for OutputType { fn drop(&mut self) { if let OutputType::Pager(ref mut command) = *self { diff --git a/src/main.rs b/src/main.rs index 94b4c745..c388f492 100644 --- a/src/main.rs +++ b/src/main.rs @@ -132,7 +132,11 @@ You can also use delta to diff two files: `delta file_A file_B`." let diff_command = "git"; let minus_file = minus_file.unwrap_or_else(die); let plus_file = plus_file.unwrap_or_else(die); - let mut diff_process = process::Command::new(PathBuf::from(diff_command)) + let diff_command_path = match grep_cli::resolve_binary(PathBuf::from(diff_command)) { + Ok(path) => path, + Err(_) => return config.error_exit_code, + }; + let mut diff_process = process::Command::new(diff_command_path) .args(&["diff", "--no-index"]) .args(&[minus_file, plus_file]) .stdout(process::Stdio::piped()) -- cgit v1.2.3