From 55ba2d30c1c18d2a496a61037da3980a2f4314c7 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Thu, 3 Nov 2022 12:46:43 -0400 Subject: [PATCH] Quote arguments when executing in a shell (#118) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Quote arguments when executing in a shell Fixes #107 * Parse quotes in `tmux_arguments` This makes it possible to encode spaces in arguments. Maybe the config value should be an array instead? * Print error causes Co-authored-by: Thomas Schönauer <37108907+DottoDev@users.noreply.github.com> --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/config.rs | 13 ++++++++++--- src/executor.rs | 11 ++++++----- src/main.rs | 7 +++++-- src/steps/powershell.rs | 1 + src/steps/remote/ssh.rs | 2 +- src/steps/tmux.rs | 14 ++++++-------- 8 files changed, 37 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f2f6a3a..8c7754bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1606,6 +1606,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae1a47186c03a32177042e55dbc5fd5aee900b8e0069a8d70fba96a9375cd012" +[[package]] +name = "shell-words" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde" + [[package]] name = "shellexpand" version = "2.1.2" @@ -1943,6 +1949,7 @@ dependencies = [ "self_update", "semver", "serde", + "shell-words", "shellexpand", "strum 0.24.1", "sys-info", diff --git a/Cargo.toml b/Cargo.toml index 83869b73..c609ba6b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,7 @@ futures = "0.3" regex = "1.5" sys-info = "0.9" semver = "1.0" +shell-words = "1.1.0" [target.'cfg(target_os = "macos")'.dependencies] notify-rust = "4.5" diff --git a/src/config.rs b/src/config.rs index d050532c..f9fa06d0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,7 +4,6 @@ use std::fs::write; use std::path::PathBuf; use std::process::Command; use std::{env, fs}; - use anyhow::Result; use clap::{ArgEnum, Parser}; use directories::BaseDirs; @@ -626,8 +625,16 @@ impl Config { } /// Extra Tmux arguments - pub fn tmux_arguments(&self) -> &Option { - &self.config_file.tmux_arguments + pub fn tmux_arguments(&self) -> anyhow::Result> { + let args = &self.config_file.tmux_arguments.as_deref().unwrap_or_default(); + shell_words::split(args) + // The only time the parse failed is in case of a missing close quote. + // The error message looks like this: + // Error: Failed to parse `tmux_arguments`: `'foo` + // + // Caused by: + // missing closing quote + .with_context(|| format!("Failed to parse `tmux_arguments`: `{args}`")) } /// Prompt for a key before exiting diff --git a/src/executor.rs b/src/executor.rs index b568225e..b68b30a4 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -194,11 +194,12 @@ impl DryCommand { print!( "Dry running: {} {}", self.program.to_string_lossy(), - self.args - .iter() - .map(|a| String::from(a.to_string_lossy())) - .collect::>() - .join(" ") + shell_words::join( + self.args + .iter() + .map(|a| String::from(a.to_string_lossy())) + .collect::>() + ) ); match &self.directory { Some(dir) => println!(" in {}", dir.to_string_lossy()), diff --git a/src/main.rs b/src/main.rs index 02e5cc5a..d08d207e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -79,7 +79,7 @@ fn run() -> Result<()> { if config.run_in_tmux() && env::var("TOPGRADE_INSIDE_TMUX").is_err() { #[cfg(unix)] { - tmux::run_in_tmux(config.tmux_arguments()); + tmux::run_in_tmux(config.tmux_arguments()?); } } @@ -524,7 +524,10 @@ fn main() { .is_some()); if !skip_print { - println!("Error: {}", error); + // The `Debug` implementation of `anyhow::Result` prints a multi-line + // error message that includes all the 'causes' added with + // `.with_context(...)` calls. + println!("Error: {:?}", error); } exit(1); } diff --git a/src/steps/powershell.rs b/src/steps/powershell.rs index 78145ed1..bd1c00b4 100644 --- a/src/steps/powershell.rs +++ b/src/steps/powershell.rs @@ -79,6 +79,7 @@ impl Powershell { println!("Updating modules..."); ctx.run_type() .execute(powershell) + // This probably doesn't need `shell_words::join`. .args(["-NoProfile", "-Command", &cmd.join(" ")]) .check_run() } diff --git a/src/steps/remote/ssh.rs b/src/steps/remote/ssh.rs index 8636b895..c94552fb 100644 --- a/src/steps/remote/ssh.rs +++ b/src/steps/remote/ssh.rs @@ -24,7 +24,7 @@ pub fn ssh_step(ctx: &ExecutionContext, hostname: &str) -> Result<()> { #[cfg(unix)] { prepare_async_ssh_command(&mut args); - crate::tmux::run_command(ctx, &args.join(" "))?; + crate::tmux::run_command(ctx, &shell_words::join(args))?; Err(SkipStep(String::from("Remote Topgrade launched in Tmux")).into()) } diff --git a/src/steps/tmux.rs b/src/steps/tmux.rs index 85082b98..436865f9 100644 --- a/src/steps/tmux.rs +++ b/src/steps/tmux.rs @@ -29,12 +29,10 @@ struct Tmux { } impl Tmux { - fn new(args: &Option) -> Self { + fn new(args: Vec) -> Self { Self { tmux: which("tmux").expect("Could not find tmux"), - args: args - .as_ref() - .map(|args| args.split_whitespace().map(String::from).collect()), + args: if args.is_empty() { None } else { Some(args) }, } } @@ -75,7 +73,7 @@ impl Tmux { } } -pub fn run_in_tmux(args: &Option) -> ! { +pub fn run_in_tmux(args: Vec) -> ! { let command = { let mut command = vec![ String::from("env"), @@ -83,10 +81,10 @@ pub fn run_in_tmux(args: &Option) -> ! { String::from("TOPGRADE_INSIDE_TMUX=1"), ]; command.extend(env::args()); - command.join(" ") + shell_words::join(command) }; - let tmux = Tmux::new(args); + let tmux = Tmux::new(args.clone()); if !tmux.has_session("topgrade").expect("Error detecting a tmux session") { tmux.new_session("topgrade").expect("Error creating a tmux session"); @@ -108,7 +106,7 @@ pub fn run_in_tmux(args: &Option) -> ! { } pub fn run_command(ctx: &ExecutionContext, command: &str) -> Result<()> { - Tmux::new(ctx.config().tmux_arguments()) + Tmux::new(ctx.config().tmux_arguments()?) .build() .args(["new-window", "-a", "-t", "topgrade:1", command]) .env_remove("TMUX")