Merge pull request #95 from watchexec/wrap-whitespace-in-args

Wrap whitespace and quotes in command arguments

src/cli.rs

@@ -5,7 +5,7 @@ use clap::{App, Arg, Error};
 
 #[derive(Debug)]
 pub struct Args {
-    pub cmd: String,
+    pub cmd: Vec<String>,
     pub paths: Vec<String>,
     pub filters: Vec<String>,
     pub ignores: Vec<String>,
@@ -120,9 +120,7 @@ pub fn get_args() -> Args {
         .arg(Arg::with_name("once").short("1").hidden(true))
         .get_matches();
 
-    let cmd = values_t!(args.values_of("command"), String)
-        .unwrap()
-        .join(" ");
+    let cmd: Vec<String> = values_t!(args.values_of("command"), String).unwrap();
     let paths = values_t!(args.values_of("path"), String).unwrap_or(vec![String::from(".")]);
 
     // Treat --kill as --signal SIGKILL (for compatibility with older syntax)

src/process.rs

@@ -2,12 +2,47 @@ use std::path::PathBuf;
 use std::collections::{HashMap, HashSet};
 use pathop::PathOp;
 
-pub fn spawn(cmd: &str, updated_paths: Vec<PathOp>, no_shell: bool) -> Process {
+pub fn spawn(cmd: &Vec<String>, updated_paths: Vec<PathOp>, no_shell: bool) -> Process {
     self::imp::Process::new(cmd, updated_paths, no_shell).expect("unable to spawn process")
 }
 
 pub use self::imp::Process;
 
+fn needs_wrapping(s: &String) -> bool {
+    s.contains(|ch| match ch {
+        ' ' | '\t' | '\'' | '"' => true,
+        _ => false
+    })
+}
+
+#[cfg(target_family = "unix")]
+fn wrap_in_quotes(s: &String) -> String {
+    format!("'{}'", if s.contains('\'') {
+        s.replace('\'', "'\"'\"'")
+    } else {
+        s.clone()
+    })
+}
+
+#[cfg(target_family = "windows")]
+fn wrap_in_quotes(s: &String) -> String {
+    format!("\"{}\"", if s.contains('"') {
+        s.replace('"', "\"\"")
+    } else {
+        s.clone()
+    })
+}
+
+fn wrap_commands(cmd: &Vec<String>) -> Vec<String> {
+    cmd.iter().map(|fragment| {
+        if needs_wrapping(fragment) {
+            wrap_in_quotes(fragment)
+        } else {
+            fragment.clone()
+        }
+    }).collect()
+}
+
 #[cfg(target_family = "unix")]
 mod imp {
     use nix::{self, Error};
@@ -15,6 +50,7 @@ mod imp {
     use std::io::{self, Result};
     use std::process::Command;
     use std::sync::*;
+    use super::wrap_commands;
     use signal::Signal;
     use pathop::PathOp;
 
@@ -35,7 +71,7 @@ mod imp {
     #[allow(unknown_lints)]
     #[allow(mutex_atomic)]
     impl Process {
-        pub fn new(cmd: &str, updated_paths: Vec<PathOp>, no_shell: bool) -> Result<Process> {
+        pub fn new(cmd: &Vec<String>, updated_paths: Vec<PathOp>, no_shell: bool) -> Result<Process> {
             use nix::unistd::*;
             use std::os::unix::process::CommandExt;
 
@@ -45,13 +81,13 @@ mod imp {
             // but is a little less performant and can cause trouble when using custom signals
             // (e.g. --signal SIGHUP)
             let mut command = if no_shell {
-                let mut split = cmd.split_whitespace();
-                let mut command = Command::new(split.next().unwrap());
-                command.args(split);
+                let (head, tail) = cmd.split_first().unwrap();
+                let mut command = Command::new(head);
+                command.args(tail);
                 command
             } else {
                 let mut command = Command::new("sh");
-                command.arg("-c").arg(cmd);
+                command.arg("-c").arg(wrap_commands(cmd).join(" "));
                 command
             };
 
@@ -134,6 +170,7 @@ mod imp {
     use std::mem;
     use std::process::Command;
     use std::ptr;
+    use super::wrap_commands;
     use kernel32::*;
     use winapi::*;
     use signal::Signal;
@@ -151,7 +188,7 @@ mod imp {
     }
 
     impl Process {
-        pub fn new(cmd: &str, updated_paths: Vec<PathOp>, no_shell: bool) -> Result<Process> {
+        pub fn new(cmd: &Vec<String>, updated_paths: Vec<PathOp>, no_shell: bool) -> Result<Process> {
             use std::os::windows::io::IntoRawHandle;
             use std::os::windows::process::CommandExt;
 
@@ -194,23 +231,17 @@ mod imp {
                 panic!("failed to set job info: {}", last_err());
             }
 
+            let mut command;
+            if no_shell {
+                let (arg0, args) = cmd.split_first().unwrap();
+                command = Command::new(arg0);
+                command.args(args);
+            } else {
+                command = Command::new("cmd.exe");
+                command.arg("/C");
+                command.arg(wrap_commands(cmd).join(" "));
+            }
 
-            let mut iter_args = cmd.split_whitespace();
-            let arg0 = match no_shell {
-                true => iter_args.next().unwrap(),
-                false => "cmd.exe",
-            };
-
-            // TODO: There might be a better way of doing this with &str.
-            //       I've had to fall back to String, as I wasn't able to join(" ") a Vec<&str>
-            //       into a &str
-            let args: Vec<String> = match no_shell {
-                true => iter_args.map(str::to_string).collect(),
-                false => vec!["/C".to_string(), iter_args.collect::<Vec<&str>>().join(" ")],
-            };
-
-            let mut command = Command::new(arg0);
-            command.args(args);
             command.creation_flags(CREATE_SUSPENDED);
             debug!("Assembled command {:?}", command);
 
@@ -394,10 +425,51 @@ mod tests {
     use super::spawn;
     use super::get_longest_common_path;
     use super::collect_path_env_vars;
+    use super::wrap_commands;
 
     #[test]
     fn test_start() {
-        let _ = spawn("echo hi", vec![], true);
+        let _ = spawn(&vec!["echo".into(), "hi".into()], vec![], true);
+    }
+
+    #[test]
+    fn wrap_commands_that_have_whitespace() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello world".into()]),
+            vec!["echo".into(), "'hello world'".into()] as Vec<String>
+        );
+    }
+
+    #[test]
+    fn wrap_commands_that_have_long_whitespace() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello    world".into()]),
+            vec!["echo".into(), "'hello    world'".into()] as Vec<String>
+        );
+    }
+
+    #[test]
+    fn wrap_commands_that_have_single_quotes() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello ' world".into()]),
+            vec!["echo".into(), "'hello '\"'\"' world'".into()] as Vec<String>
+        );
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello'world".into()]),
+            vec!["echo".into(), "'hello'\"'\"'world'".into()] as Vec<String>
+        );
+    }
+
+    #[test]
+    fn wrap_commands_that_have_double_quotes() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello \" world".into()]),
+            vec!["echo".into(), "'hello \" world'".into()] as Vec<String>
+        );
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello\"world".into()]),
+            vec!["echo".into(), "'hello\"world'".into()] as Vec<String>
+        );
     }
 
     #[test]
@@ -445,3 +517,55 @@ mod tests {
     }
 }
 
+#[cfg(test)]
+#[cfg(target_family = "windows")]
+mod tests {
+    use super::spawn;
+    use super::wrap_commands;
+
+    #[test]
+    fn test_start() {
+        let _ = spawn(&vec!["echo".into(), "hi".into()], vec![], true);
+    }
+
+    #[test]
+    fn wrap_commands_that_have_whitespace() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello world".into()]),
+            vec!["echo".into(), "\"hello world\"".into()] as Vec<String>
+        );
+    }
+
+    #[test]
+    fn wrap_commands_that_have_long_whitespace() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello    world".into()]),
+            vec!["echo".into(), "\"hello    world\"".into()] as Vec<String>
+        );
+    }
+
+    #[test]
+    fn wrap_commands_that_have_single_quotes() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello ' world".into()]),
+            vec!["echo".into(), "\"hello ' world\"".into()] as Vec<String>
+        );
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello'world".into()]),
+            vec!["echo".into(), "\"hello'world\"".into()] as Vec<String>
+        );
+    }
+
+    #[test]
+    fn wrap_commands_that_have_double_quotes() {
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello \" world".into()]),
+            vec!["echo".into(), "\"hello \"\" world\"".into()] as Vec<String>
+        );
+        assert_eq!(
+            wrap_commands(&vec!["echo".into(), "hello\"world".into()]),
+            vec!["echo".into(), "\"hello\"\"world\"".into()] as Vec<String>
+        );
+    }
+}
+

src/run.rs

@@ -32,7 +32,7 @@ fn init_logger(debug: bool) {
         .init();
 }
 
-#[cfg(target_os="linux")]
+#[cfg(target_os = "linux")]
 fn should_switch_to_poll(e: &Error) -> bool {
     use nix::libc;
 
@@ -42,9 +42,8 @@ fn should_switch_to_poll(e: &Error) -> bool {
     }
 }
 
-#[cfg(not(target_os="linux"))]
+#[cfg(not(target_os = "linux"))]
 fn should_switch_to_poll(_: &Error) -> bool {
-    // not known conditions to switch
     false
 }