From 219b7e64cdaabad9e95067b0e5abcbd4e396b9c1 Mon Sep 17 00:00:00 2001
From: Matteo Bertini <matteo@naufraghi.net>
Date: Fri, 6 Dec 2019 19:07:53 +0100
Subject: [PATCH] Use shellexpand to expand ~ in external commands

Add tests for ~tilde expansion:

- test that "~" is expanded (no more "~" in output)
- ensure that "1~1" is not expanded to "1/home/user1" as it was
  before

Fixes #972

Note: the first test does not check the literal expansion because
the path on Windows is expanded as a Linux path, but the correct
expansion may come for free once `shellexpand` will use the `dirs`
crate too (https://github.com/netvl/shellexpand/issues/3).
---
 src/commands/classified/external.rs | 15 ++++----------
 tests/external_tests.rs             | 31 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/src/commands/classified/external.rs b/src/commands/classified/external.rs
index 9211b20dda..40ad55b0be 100644
--- a/src/commands/classified/external.rs
+++ b/src/commands/classified/external.rs
@@ -7,6 +7,7 @@ use nu_errors::ShellError;
 use nu_parser::ExternalCommand;
 use nu_protocol::{UntaggedValue, Value};
 use std::io::{Error, ErrorKind};
+use std::ops::Deref;
 use subprocess::Exec;
 
 use super::ClassifiedInputStream;
@@ -107,11 +108,7 @@ pub(crate) async fn run_external_command(
                     None
                 } else {
                     // Let's also replace ~ as we shell out
-                    let arg = if let Some(ref home_dir) = home_dir {
-                        arg.replace("~", home_dir.to_str().unwrap())
-                    } else {
-                        arg.replace("~", "~")
-                    };
+                    let arg = shellexpand::tilde_with_context(arg.deref(), || home_dir.as_ref());
 
                     Some(arg.replace("$it", &i))
                 }
@@ -125,11 +122,7 @@ pub(crate) async fn run_external_command(
         process = Exec::cmd(&command.name);
         for arg in command.args.iter() {
             // Let's also replace ~ as we shell out
-            let arg = if let Some(ref home_dir) = home_dir {
-                arg.replace("~", home_dir.to_str().unwrap())
-            } else {
-                arg.replace("~", "~")
-            };
+            let arg = shellexpand::tilde_with_context(arg.deref(), || home_dir.as_ref());
 
             let arg_chars: Vec<_> = arg.chars().collect();
             if arg_chars.len() > 1 && arg_chars[0] == '"' && arg_chars[arg_chars.len() - 1] == '"' {
@@ -137,7 +130,7 @@ pub(crate) async fn run_external_command(
                 let new_arg: String = arg_chars[1..arg_chars.len() - 1].iter().collect();
                 process = process.arg(new_arg);
             } else {
-                process = process.arg(arg.clone());
+                process = process.arg(arg.as_ref());
             }
         }
     }
diff --git a/tests/external_tests.rs b/tests/external_tests.rs
index 0d810acac1..6b3c3fac56 100644
--- a/tests/external_tests.rs
+++ b/tests/external_tests.rs
@@ -1,5 +1,7 @@
 mod helpers;
 
+use helpers::Playground;
+
 #[test]
 fn external_command() {
     let actual = nu!(
@@ -9,3 +11,32 @@ fn external_command() {
 
     assert!(actual.contains("1"));
 }
+
+#[test]
+fn spawn_external_process_with_home_in_arguments() {
+    Playground::setup("echo_tilde", |dirs, _| {
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"
+                sh -c "echo ~"
+            "#
+        );
+
+        assert!(
+            !actual.contains("~"),
+            format!("'{}' should not contain ~", actual)
+        );
+    })
+}
+
+#[test]
+fn spawn_external_process_with_tilde_in_arguments() {
+    let actual = nu!(
+        cwd: "tests/fixtures",
+        r#"
+            sh -c "echo 1~1"
+        "#
+    );
+
+    assert_eq!(actual, "1~1");
+}