Update external spawn (#406)

* Simplify external spawn, improve arg cleaning

* Fix tests

* Fix windows test

crates/nu-command/src/system/run_external.rs

@@ -10,7 +10,10 @@ use nu_protocol::engine::{EngineState, Stack};
 use nu_protocol::{ast::Call, engine::Command, ShellError, Signature, SyntaxShape, Value};
 use nu_protocol::{Category, Config, IntoInterruptiblePipelineData, PipelineData, Span, Spanned};
 
+use itertools::Itertools;
+
 use nu_engine::CallExt;
+use regex::Regex;
 
 const OUTPUT_BUFFER_SIZE: usize = 8192;
 
@@ -206,22 +209,68 @@ impl ExternalCommand {
             //TODO. This should be modifiable from the config file.
             // We could give the option to call from powershell
             // for minimal builds cwd is unused
-            let mut process = CommandSys::new("cmd");
+            if self.name.item.ends_with(".cmd") || self.name.item.ends_with(".bat") {
-            process.arg("/c");
+                self.spawn_cmd_command()
-            process.arg(&self.name.item);
+            } else {
-            for arg in &self.args {
+                self.spawn_simple_command()
-                // Clean the args before we use them:
-                // https://stackoverflow.com/questions/1200235/how-to-pass-a-quoted-pipe-character-to-cmd-exe
-                // cmd.exe needs to have a caret to escape a pipe
-                let arg = arg.replace("|", "^|");
-                process.arg(&arg);
             }
-            process
+        } else if self.name.item.ends_with(".sh") {
+            self.spawn_sh_command()
         } else {
-            let cmd_with_args = vec![self.name.item.clone(), self.args.join(" ")].join(" ");
+            self.spawn_simple_command()
-            let mut process = CommandSys::new("sh");
+        }
-            process.arg("-c").arg(cmd_with_args);
+    }
-            process
+
+    /// Spawn a command without shelling out to an external shell
+    fn spawn_simple_command(&self) -> std::process::Command {
+        let mut process = std::process::Command::new(&self.name.item);
+
+        for arg in &self.args {
+            process.arg(&arg);
+        }
+
+        process
+    }
+
+    /// Spawn a cmd command with `cmd /c args...`
+    fn spawn_cmd_command(&self) -> std::process::Command {
+        let mut process = std::process::Command::new("cmd");
+        process.arg("/c");
+        process.arg(&self.name.item);
+        for arg in &self.args {
+            // Clean the args before we use them:
+            // https://stackoverflow.com/questions/1200235/how-to-pass-a-quoted-pipe-character-to-cmd-exe
+            // cmd.exe needs to have a caret to escape a pipe
+            let arg = arg.replace("|", "^|");
+            process.arg(&arg);
+        }
+        process
+    }
+
+    /// Spawn a sh command with `sh -c args...`
+    fn spawn_sh_command(&self) -> std::process::Command {
+        let joined_and_escaped_arguments =
+            self.args.iter().map(|arg| shell_arg_escape(arg)).join(" ");
+        let cmd_with_args = vec![self.name.item.clone(), joined_and_escaped_arguments].join(" ");
+        let mut process = std::process::Command::new("sh");
+        process.arg("-c").arg(cmd_with_args);
+        process
+    }
+}
+
+fn has_unsafe_shell_characters(arg: &str) -> bool {
+    let re: Regex = Regex::new(r"[^\w@%+=:,./-]").expect("regex to be valid");
+
+    re.is_match(arg)
+}
+
+fn shell_arg_escape(arg: &str) -> String {
+    match arg {
+        "" => String::from("''"),
+        s if !has_unsafe_shell_characters(s) => String::from(s),
+        _ => {
+            let single_quotes_escaped = arg.split('\'').join("'\"'\"'");
+            format!("'{}'", single_quotes_escaped)
         }
     }
 }

src/tests.rs

@@ -56,9 +56,9 @@ fn fail_test(input: &str, expected: &str) -> TestResult {
 
 fn not_found_msg() -> &'static str {
     if cfg!(windows) {
-        "not recognized"
+        "cannot find"
     } else {
-        "not found"
+        "No such"
     }
 }