diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 409f44a128..51dade205c 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -202,9 +202,8 @@ impl ExternalCommand { // This has the full list of cmd.exe "internal" commands: https://ss64.com/nt/syntax-internal.html // I (Reilly) went through the full list and whittled it down to ones that are potentially useful: - const CMD_INTERNAL_COMMANDS: [&str; 10] = [ - "ASSOC", "CLS", "DIR", "ECHO", "FTYPE", "MKLINK", "PAUSE", "START", "VER", - "VOL", + const CMD_INTERNAL_COMMANDS: [&str; 9] = [ + "ASSOC", "CLS", "ECHO", "FTYPE", "MKLINK", "PAUSE", "START", "VER", "VOL", ]; let command_name_upper = self.name.item.to_uppercase(); let looks_like_cmd_internal = CMD_INTERNAL_COMMANDS @@ -527,7 +526,7 @@ impl ExternalCommand { ) -> Result { let mut process = if let Some(d) = self.env_vars.get("PWD") { let mut process = if use_cmd { - self.spawn_cmd_command() + self.spawn_cmd_command(d) } else { self.create_command(d)? }; @@ -578,7 +577,7 @@ impl ExternalCommand { // We could give the option to call from powershell // for minimal builds cwd is unused if self.name.item.ends_with(".cmd") || self.name.item.ends_with(".bat") { - Ok(self.spawn_cmd_command()) + Ok(self.spawn_cmd_command(cwd)) } else { self.spawn_simple_command(cwd) } @@ -599,76 +598,14 @@ impl ExternalCommand { let mut process = std::process::Command::new(head); for (arg, arg_keep_raw) in self.args.iter().zip(self.arg_keep_raw.iter()) { - // if arg is quoted, like "aa", 'aa', `aa`, or: - // if arg is a variable or String interpolation, like: $variable_name, $"($variable_name)" - // `as_a_whole` will be true, so nu won't remove the inner quotes. - let (trimmed_args, run_glob_expansion, mut keep_raw) = trim_enclosing_quotes(&arg.item); - if *arg_keep_raw { - keep_raw = true; - } - - let mut arg = Spanned { - item: if keep_raw { - trimmed_args - } else { - remove_quotes(trimmed_args) - }, - span: arg.span, - }; - - if !keep_raw { - arg.item = nu_path::expand_tilde(arg.item) - .to_string_lossy() - .to_string(); - } - - let cwd = PathBuf::from(cwd); - - if arg.item.contains('*') && run_glob_expansion { - if let Ok((prefix, matches)) = - nu_engine::glob_from(&arg, &cwd, self.name.span, None) - { - let matches: Vec<_> = matches.collect(); - - // FIXME: do we want to special-case this further? We might accidentally expand when they don't - // intend to - if matches.is_empty() { - process.arg(&arg.item); - } - for m in matches { - if let Ok(arg) = m { - let arg = if let Some(prefix) = &prefix { - if let Ok(remainder) = arg.strip_prefix(prefix) { - let new_prefix = if let Some(pfx) = diff_paths(prefix, &cwd) { - pfx - } else { - prefix.to_path_buf() - }; - - new_prefix.join(remainder).to_string_lossy().to_string() - } else { - arg.to_string_lossy().to_string() - } - } else { - arg.to_string_lossy().to_string() - }; - - process.arg(&arg); - } else { - process.arg(&arg.item); - } - } - } - } else { - process.arg(&arg.item); - } + trim_expand_and_apply_arg(&mut process, arg, arg_keep_raw, cwd); } Ok(process) } /// Spawn a cmd command with `cmd /c args...` - pub fn spawn_cmd_command(&self) -> std::process::Command { + pub fn spawn_cmd_command(&self, cwd: &str) -> std::process::Command { let mut process = std::process::Command::new("cmd"); // Disable AutoRun @@ -678,13 +615,17 @@ impl ExternalCommand { process.arg("/c"); process.arg(&self.name.item); - for arg in &self.args { - // Clean the args before we use them: + for (arg, arg_keep_raw) in self.args.iter().zip(self.arg_keep_raw.iter()) { // 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.item.replace('|', "^|"); - process.arg(&arg); + let arg = Spanned { + item: arg.item.replace('|', "^|"), + span: arg.span, + }; + + trim_expand_and_apply_arg(&mut process, &arg, arg_keep_raw, cwd) } + process } @@ -702,6 +643,71 @@ impl ExternalCommand { } } +fn trim_expand_and_apply_arg( + process: &mut CommandSys, + arg: &Spanned, + arg_keep_raw: &bool, + cwd: &str, +) { + // if arg is quoted, like "aa", 'aa', `aa`, or: + // if arg is a variable or String interpolation, like: $variable_name, $"($variable_name)" + // `as_a_whole` will be true, so nu won't remove the inner quotes. + let (trimmed_args, run_glob_expansion, mut keep_raw) = trim_enclosing_quotes(&arg.item); + if *arg_keep_raw { + keep_raw = true; + } + let mut arg = Spanned { + item: if keep_raw { + trimmed_args + } else { + remove_quotes(trimmed_args) + }, + span: arg.span, + }; + if !keep_raw { + arg.item = nu_path::expand_tilde(arg.item) + .to_string_lossy() + .to_string(); + } + let cwd = PathBuf::from(cwd); + if arg.item.contains('*') && run_glob_expansion { + if let Ok((prefix, matches)) = nu_engine::glob_from(&arg, &cwd, arg.span, None) { + let matches: Vec<_> = matches.collect(); + + // FIXME: do we want to special-case this further? We might accidentally expand when they don't + // intend to + if matches.is_empty() { + process.arg(&arg.item); + } + for m in matches { + if let Ok(arg) = m { + let arg = if let Some(prefix) = &prefix { + if let Ok(remainder) = arg.strip_prefix(prefix) { + let new_prefix = if let Some(pfx) = diff_paths(prefix, &cwd) { + pfx + } else { + prefix.to_path_buf() + }; + + new_prefix.join(remainder).to_string_lossy().to_string() + } else { + arg.to_string_lossy().to_string() + } + } else { + arg.to_string_lossy().to_string() + }; + + process.arg(&arg); + } else { + process.arg(&arg.item); + } + } + } + } else { + process.arg(&arg.item); + } +} + /// Given an invalid command name, try to suggest an alternative fn suggest_command(attempted_command: &str, engine_state: &EngineState) -> Option { let commands = engine_state.get_signatures(false); diff --git a/crates/nu-command/tests/commands/redirection.rs b/crates/nu-command/tests/commands/redirection.rs index 08da592100..14ee683f8e 100644 --- a/crates/nu-command/tests/commands/redirection.rs +++ b/crates/nu-command/tests/commands/redirection.rs @@ -20,7 +20,7 @@ fn redirect_err() { Playground::setup("redirect_err_test", |dirs, _sandbox| { let output = nu!( cwd: dirs.test(), - "dir missingapplication err> a; (open a | size).bytes >= 16" + "vol missingdrive err> a; (open a | size).bytes >= 16" ); assert!(output.out.contains("true")); @@ -46,7 +46,7 @@ fn redirect_outerr() { Playground::setup("redirect_outerr_test", |dirs, _sandbox| { let output = nu!( cwd: dirs.test(), - "dir missingapplication out+err> a; (open a | size).bytes >= 16" + "vol missingdrive out+err> a; (open a | size).bytes >= 16" ); assert!(output.out.contains("true")); diff --git a/crates/nu-command/tests/commands/run_external.rs b/crates/nu-command/tests/commands/run_external.rs index 95a7b8e672..0898273be1 100644 --- a/crates/nu-command/tests/commands/run_external.rs +++ b/crates/nu-command/tests/commands/run_external.rs @@ -1,3 +1,4 @@ +#[cfg(not(windows))] use nu_test_support::fs::Stub::EmptyFile; use nu_test_support::playground::Playground; use nu_test_support::{nu, pipeline}; @@ -212,49 +213,6 @@ fn external_command_not_expand_tilde_with_quotes() { ) } -#[cfg(windows)] -#[test] -fn explicit_glob_windows() { - Playground::setup("external with explicit glob", |dirs, sandbox| { - sandbox.with_files(vec![ - EmptyFile("D&D_volume_1.txt"), - EmptyFile("D&D_volume_2.txt"), - EmptyFile("foo.sh"), - ]); - - let actual = nu!( - cwd: dirs.test(), pipeline( - r#" - ^dir | glob '*.txt' | length - "# - )); - - assert_eq!(actual.out, "2"); - }) -} - -#[cfg(windows)] -#[test] -fn bare_word_expand_path_glob_windows() { - Playground::setup("bare word should do the expansion", |dirs, sandbox| { - sandbox.with_files(vec![ - EmptyFile("D&D_volume_1.txt"), - EmptyFile("D&D_volume_2.txt"), - EmptyFile("foo.sh"), - ]); - - let actual = nu!( - cwd: dirs.test(), pipeline( - r#" - ^dir *.txt - "# - )); - - assert!(actual.out.contains("D&D_volume_1.txt")); - assert!(actual.out.contains("D&D_volume_2.txt")); - }) -} - #[cfg(windows)] #[test] fn failed_command_with_semicolon_will_not_execute_following_cmds_windows() { @@ -329,3 +287,16 @@ fn can_run_batch_files_without_bat_extension() { }, ); } + +#[cfg(windows)] +#[test] +fn quotes_trimmed_when_shelling_out() { + // regression test for a bug where we weren't trimming quotes around string args before shelling out to cmd.exe + let actual = nu!(cwd: ".", pipeline( + r#" + ^echo "foo" + "# + )); + + assert_eq!(actual.out, "foo"); +}