From 40ed5728b6b9f2158b3dfc0535825a0c08b0a099 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Mon, 24 Oct 2016 18:02:13 -0400 Subject: [PATCH 01/10] Find the program using PATHEXT Windows relies on path extensions to resolve commands, extensions are found in the `PATHEXT` environment variable. We are adding a new function called `Command:find_program()` that will return the `Path` of the program found using the above env variable. Closes https://github.com/rust-lang/rust/issues/37380 Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index d371714ff0e69..f9a76b328c648 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -125,12 +125,8 @@ impl Command { self.stderr = Some(stderr); } - pub fn spawn(&mut self, default: Stdio, needs_stdin: bool) - -> io::Result<(Process, StdioPipes)> { - // To have the spawning semantics of unix/windows stay the same, we need - // to read the *child's* PATH if one is provided. See #15149 for more - // details. - let program = self.env.as_ref().and_then(|env| { + pub fn find_program(&mut self) -> Option { + self.env.as_ref().and_then(|env| { for (key, v) in env { if OsStr::new("PATH") != &**key { continue } @@ -141,12 +137,33 @@ impl Command { .with_extension(env::consts::EXE_EXTENSION); if fs::metadata(&path).is_ok() { return Some(path.into_os_string()) + } else { + // Windows relies on path extensions to resolve commands. + // Path extensions are found in the PATHEXT environment variable. + if let Some(exts) = self.env.get("PATHEXT") { + for ext in split_paths(&exts) { + let ext_str = pathext.to_str().unwrap().trim_matches('.'); + let path = path.join(self.program.to_str().unwrap()) + .with_extension(ext_str); + if fs::metadata(&path).is_ok() { + return Some(path.into_os_string()) + } + } + } } } break } None }); + } + + pub fn spawn(&mut self, default: Stdio, needs_stdin: bool) + -> io::Result<(Process, StdioPipes)> { + // To have the spawning semantics of unix/windows stay the same, we need + // to read the *child's* PATH if one is provided. See #15149 for more + // details. + let program = self.find_program(); let mut si = zeroed_startupinfo(); si.cb = mem::size_of::() as c::DWORD; From cc9d06416e1f9ed875cc69c78093a85750977b88 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Tue, 25 Oct 2016 08:25:50 -0400 Subject: [PATCH 02/10] Added env_lookup fn This new simple fun will lookup for a specific variable within `self.env` Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index f9a76b328c648..76244ba5fed25 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -140,11 +140,10 @@ impl Command { } else { // Windows relies on path extensions to resolve commands. // Path extensions are found in the PATHEXT environment variable. - if let Some(exts) = self.env.get("PATHEXT") { + if let Some(exts) = env_lookup(self.env.as_ref(), "PATHEXT") { for ext in split_paths(&exts) { let ext_str = pathext.to_str().unwrap().trim_matches('.'); - let path = path.join(self.program.to_str().unwrap()) - .with_extension(ext_str); + let path = path.with_extension(ext_str); if fs::metadata(&path).is_ok() { return Some(path.into_os_string()) } @@ -504,6 +503,16 @@ fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { } } +fn env_lookup(env: Option<&collections::HashMap>, + var: &OsStr) -> Option { + if let Some(env) = env { + if let Some(value) = env.get(var) { + return Some(value.to_os_string()) + } + } + None +} + #[cfg(test)] mod tests { use ffi::{OsStr, OsString}; From a3c72fd0798334ae5205702d07b0df5dee6afa7d Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Tue, 25 Oct 2016 17:42:38 -0700 Subject: [PATCH 03/10] Added tests for Command::env_lookup() Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 40 +++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 76244ba5fed25..0c0a058780c5f 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -504,7 +504,7 @@ fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { } fn env_lookup(env: Option<&collections::HashMap>, - var: &OsStr) -> Option { + var: &str) -> Option { if let Some(env) = env { if let Some(value) = env.get(var) { return Some(value.to_os_string()) @@ -516,7 +516,7 @@ fn env_lookup(env: Option<&collections::HashMap>, #[cfg(test)] mod tests { use ffi::{OsStr, OsString}; - use super::make_command_line; + use super::{make_command_line, env_lookup}; #[test] fn test_make_command_line() { @@ -555,4 +555,40 @@ mod tests { "\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}" ); } + + fn gen_env() -> Option> { + let env: HashMap = HashMap::new(); + env.insert(OsString::new("HOMEDRIVE"), OsString::new("C:")); + env.insert(OsString::new("PATH"), + OsString::new("C:\\awesome\\rust\\blah;C:\\binaries")); + env.insert(OsString::new("USERNAME"), OsString::new("rust")); + Some(env) + } + + #[test] + fn test_env_lookup_when_variable_exists() { + let env = gen_env(); + assert_eq!( + Some(OsString::new("C:")), + env_lookup(env.as_ref(), "HOMEDRIVE") + ); + assert_eq!( + Some(OsString::new("rust")), + env_lookup(env.as_ref(), "USERNAME") + ); + } + + #[test] + fn test_env_lookup_when_variable_does_not_exists() { + let env = gen_env(); + assert_eq!(None, env_lookup(env.as_ref(), "CRAZY_VAR")); + assert_eq!(None, env_lookup(env.as_ref(), "THIS_DOESNT_EXIST")); + } + + #[test] + fn test_env_lookup_when_none_env() { + let env = None; + assert_eq!(None, env_lookup(env.as_ref(), "HOMEDRIVE")); + assert_eq!(None, env_lookup(env.as_ref(), "THIS_DOESNT_EXIST")); + } } From 63b4c790045d1497f4bd7dc3f233cfb3831f4fa1 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Wed, 26 Oct 2016 11:50:09 -0700 Subject: [PATCH 04/10] Hoist the PATHEXT env lookup Instead of the `env_lookup()` fn we should just use the `HashMap::get()` Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 57 ++++--------------------------- 1 file changed, 7 insertions(+), 50 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 0c0a058780c5f..134e74872a769 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -127,6 +127,7 @@ impl Command { pub fn find_program(&mut self) -> Option { self.env.as_ref().and_then(|env| { + let env_pathext = env.get("PATHEXT"); for (key, v) in env { if OsStr::new("PATH") != &**key { continue } @@ -140,10 +141,12 @@ impl Command { } else { // Windows relies on path extensions to resolve commands. // Path extensions are found in the PATHEXT environment variable. - if let Some(exts) = env_lookup(self.env.as_ref(), "PATHEXT") { + if let Some(exts) = env_pathext { for ext in split_paths(&exts) { - let ext_str = pathext.to_str().unwrap().trim_matches('.'); - let path = path.with_extension(ext_str); + let ext_str = ext.to_string_lossy(); + let path = path.with_extension( + ext_str.trim_matches('.') + ); if fs::metadata(&path).is_ok() { return Some(path.into_os_string()) } @@ -503,20 +506,10 @@ fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { } } -fn env_lookup(env: Option<&collections::HashMap>, - var: &str) -> Option { - if let Some(env) = env { - if let Some(value) = env.get(var) { - return Some(value.to_os_string()) - } - } - None -} - #[cfg(test)] mod tests { use ffi::{OsStr, OsString}; - use super::{make_command_line, env_lookup}; + use super::make_command_line; #[test] fn test_make_command_line() { @@ -555,40 +548,4 @@ mod tests { "\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}" ); } - - fn gen_env() -> Option> { - let env: HashMap = HashMap::new(); - env.insert(OsString::new("HOMEDRIVE"), OsString::new("C:")); - env.insert(OsString::new("PATH"), - OsString::new("C:\\awesome\\rust\\blah;C:\\binaries")); - env.insert(OsString::new("USERNAME"), OsString::new("rust")); - Some(env) - } - - #[test] - fn test_env_lookup_when_variable_exists() { - let env = gen_env(); - assert_eq!( - Some(OsString::new("C:")), - env_lookup(env.as_ref(), "HOMEDRIVE") - ); - assert_eq!( - Some(OsString::new("rust")), - env_lookup(env.as_ref(), "USERNAME") - ); - } - - #[test] - fn test_env_lookup_when_variable_does_not_exists() { - let env = gen_env(); - assert_eq!(None, env_lookup(env.as_ref(), "CRAZY_VAR")); - assert_eq!(None, env_lookup(env.as_ref(), "THIS_DOESNT_EXIST")); - } - - #[test] - fn test_env_lookup_when_none_env() { - let env = None; - assert_eq!(None, env_lookup(env.as_ref(), "HOMEDRIVE")); - assert_eq!(None, env_lookup(env.as_ref(), "THIS_DOESNT_EXIST")); - } } From f3641903d8a0674a9b5b66d7af22156701f92353 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Wed, 26 Oct 2016 14:38:39 -0700 Subject: [PATCH 05/10] Fixed some nitpicks for better code quality Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 134e74872a769..dce9217696e5d 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -138,19 +138,19 @@ impl Command { .with_extension(env::consts::EXE_EXTENSION); if fs::metadata(&path).is_ok() { return Some(path.into_os_string()) - } else { - // Windows relies on path extensions to resolve commands. - // Path extensions are found in the PATHEXT environment variable. - if let Some(exts) = env_pathext { - for ext in split_paths(&exts) { - let ext_str = ext.to_string_lossy(); - let path = path.with_extension( - ext_str.trim_matches('.') - ); - if fs::metadata(&path).is_ok() { - return Some(path.into_os_string()) - } - } + } + + // Windows relies on path extensions to resolve commands. + // Path extensions are found in the PATHEXT environment variable. + let exts = match env_pathext { + Some(e) => e, + None => continue, + }; + + for ext in split_paths(&exts).filter_map(|e| e.to_str()) { + let path = path.with_extension(ext.trim_matches('.')); + if fs::metadata(&path).is_ok() { + return Some(path.into_os_string()) } } } From 41a0df0c28af129413534ce3bbd6595105c21bf5 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Wed, 26 Oct 2016 16:03:11 -0700 Subject: [PATCH 06/10] Added process_command tests for find_program Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 3 + .../run-pass/process_command/find_program.rs | 80 +++++++++++++++++++ .../process_command/fixtures/bin/bin.bat | 0 .../process_command/fixtures/bin/exec.exe | 0 4 files changed, 83 insertions(+) create mode 100644 src/test/run-pass/process_command/find_program.rs create mode 100644 src/test/run-pass/process_command/fixtures/bin/bin.bat create mode 100644 src/test/run-pass/process_command/fixtures/bin/exec.exe diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index dce9217696e5d..212bb7a076a75 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -112,6 +112,9 @@ impl Command { pub fn env_clear(&mut self) { self.env = Some(HashMap::new()) } + pub fn with_env(&mut self, env: &HashMap) { + self.env = Some(env) + } pub fn cwd(&mut self, dir: &OsStr) { self.cwd = Some(dir.to_os_string()) } diff --git a/src/test/run-pass/process_command/find_program.rs b/src/test/run-pass/process_command/find_program.rs new file mode 100644 index 0000000000000..b7a63c9815079 --- /dev/null +++ b/src/test/run-pass/process_command/find_program.rs @@ -0,0 +1,80 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[cfg(test)] +mod test_find_program { + use std::ffi::{OsStr, OsString}; + use std::process::Command; + use std::collections::HashMap; + use std::path::Path; + use std::fs::canonicalize; + use std::env::join_paths; + + fn gen_env() -> HashMap { + let env: HashMap = HashMap::new(); + env.insert(OsString::from("HOMEDRIVE"), OsString::from("C:")); + let p1 = canonicalize("./src/test/run-pass/process_command/fixtures/bin").unwrap(); + let p2 = canonicalize("./src/test/run-pass/process_command/fixtures").unwrap(); + let p3 = canonicalize("./src/test/run-pass/process_command").unwrap(); + let paths = vec![p1, p2, p3]; + let path = join_paths(paths).unwrap(); + env.insert(OsString::from("PATH"), OsString::from(&path)); + env.insert(OsString::from("USERNAME"), OsString::from("rust")); + env + } + + fn command_with_pathext(cmd: &str) -> Command { + let mut env = gen_env(); + env.insert( + OsString::from("PATHEXT"), + OsString::from(".COM;.EXE;.BAT;.CMD;.VBS;.VBE;.MSC") + ); + let mut cmd = Command::new(cmd); + cmd.with_env(&env); + cmd + } + + fn command_without_pathext(cmd: &str) -> Command { + let env = gen_env(); + let mut cmd = Command::new(cmd); + cmd.with_env(&env); + cmd + } + + mod with_pathext_set { + use super::command_with_pathext; + + fn command_on_path_found() { + let c = command_with_pathext("bin"); + let bat = canonicalize("./src/test/run-pass/process_command/fixtures/bin/bin.bat"); + assert_eq!(bat.ok(), c.find_program()); + } + + fn command_not_found() { + let c = command_with_pathext("missing"); + assert_eq!(None, c.find_program()); + } + } + + mod without_pathext_set { + use super::command_without_pathext; + + fn bat_command_not_found() { + let c = command_without_pathext("bin"); + assert_eq!(None, c.find_program()); + } + + fn exe_command_found() { + let c = command_without_pathext("exec"); + let exe = canonicalize("./src/test/run-pass/process_command/fixtures/bin/exec.exe"); + assert_eq!(exe.ok(), c.find_program()); + } + } +} diff --git a/src/test/run-pass/process_command/fixtures/bin/bin.bat b/src/test/run-pass/process_command/fixtures/bin/bin.bat new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/src/test/run-pass/process_command/fixtures/bin/exec.exe b/src/test/run-pass/process_command/fixtures/bin/exec.exe new file mode 100644 index 0000000000000..e69de29bb2d1d From 6b9b6f1ce3af3d4d633868be9081f4a69ffd5a65 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Fri, 28 Oct 2016 10:10:10 -0400 Subject: [PATCH 07/10] On batch files set ApplicationName to `cmd.exe` If the program is a BATCH file CreateProcess requires to start the command interpreter by setting ApplicationName to `cmd.exe` and CommandLine to the following arguments: `/c` plus the name of the batch file. Signed-off-by: Salim Afiune --- src/libstd/sys/windows/env.rs | 1 + src/libstd/sys/windows/process.rs | 41 +++++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/libstd/sys/windows/env.rs b/src/libstd/sys/windows/env.rs index e6d74895774ce..778c58dabdc09 100644 --- a/src/libstd/sys/windows/env.rs +++ b/src/libstd/sys/windows/env.rs @@ -16,4 +16,5 @@ pub mod os { pub const DLL_EXTENSION: &'static str = "dll"; pub const EXE_SUFFIX: &'static str = ".exe"; pub const EXE_EXTENSION: &'static str = "exe"; + pub const BATCH_EXTENSIONS: &'static str = vec!["bat", "cmd"]; } diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 212bb7a076a75..dd2d85f19a2d2 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -175,8 +175,7 @@ impl Command { si.dwFlags = c::STARTF_USESTDHANDLES; let program = program.as_ref().unwrap_or(&self.program); - let mut cmd_str = make_command_line(program, &self.args)?; - cmd_str.push(0); // add null terminator + let (app_name, cmd_str) = make_command_line(program, &self.args, &self.env)?; // stolen from the libuv code. let mut flags = c::CREATE_UNICODE_ENVIRONMENT; @@ -220,7 +219,7 @@ impl Command { si.hStdError = stderr.raw(); unsafe { - cvt(c::CreateProcessW(ptr::null(), + cvt(c::CreateProcessW(app_name.unwrap_or(ptr::null()), cmd_str.as_mut_ptr(), ptr::null_mut(), ptr::null_mut(), @@ -424,7 +423,31 @@ fn zeroed_process_information() -> c::PROCESS_INFORMATION { // Produces a wide string *without terminating null*; returns an error if // `prog` or any of the `args` contain a nul. -fn make_command_line(prog: &OsStr, args: &[OsString]) -> io::Result> { +fn make_command_line(prog: &OsStr, args: &[OsString], + env: Option<&collections::HashMap>) + -> io::Result<(Option>, Vec)> { + // If the program is a BATCH file we must start the command interpreter; set + // ApplicationName to cmd.exe and set CommandLine to the following arguments: + // `/c` plus the name of the batch file. + let mut app: Option> = None; + let ext = Path::new(prog).extension(); + let batch = env::consts::BATCH_EXTENSIONS.iter().find(|&&x| x == ext); + if batch.is_some() { + if let Some(e) = env { + if let Some(cmd_exe) = e.get("COMSPEC") { + app = Some(ensure_no_nuls(cmd_exe)?.encode_wide().collect()); + } + } + // If we were unable to find COMSPEC, default to `cmd.exe` + if !app.is_some() { + app = Some(OsStr::new("cmd.exe").encode_wide().collect()); + } + // Prepend the argument to the program + let mut cmd_prog = OsString::from("cmd /c "); + cmd_prog.push(prog); + let prog = cmd_prog.as_os_str(); + } + // Encode the command and arguments in a command line string such // that the spawned process may recover them using CommandLineToArgvW. let mut cmd: Vec = Vec::new(); @@ -433,7 +456,9 @@ fn make_command_line(prog: &OsStr, args: &[OsString]) -> io::Result> { cmd.push(' ' as u16); append_arg(&mut cmd, arg)?; } - return Ok(cmd); + + cmd.push(0); // add null terminator + return Ok((app, cmd)); fn append_arg(cmd: &mut Vec, arg: &OsStr) -> io::Result<()> { // If an argument has 0 characters then we need to quote it to ensure @@ -498,7 +523,6 @@ fn make_envp(env: Option<&collections::HashMap>) } fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { - match d { Some(dir) => { let mut dir_str: Vec = ensure_no_nuls(dir)?.encode_wide().collect(); @@ -515,12 +539,13 @@ mod tests { use super::make_command_line; #[test] - fn test_make_command_line() { + fn test_make_command_line_with_out_env() { fn test_wrapper(prog: &str, args: &[&str]) -> String { let command_line = &make_command_line(OsStr::new(prog), &args.iter() .map(|a| OsString::from(a)) - .collect::>()) + .collect::>(), + None) .unwrap(); String::from_utf16(command_line).unwrap() } From abe5e1a6b508405b200957e9bd278899ad8b6f50 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Mon, 31 Oct 2016 12:12:59 -0400 Subject: [PATCH 08/10] Remove find_program tests We need to move this tests to live inside the module since the `find_program` fn is a private implementation. Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 3 - .../run-pass/process_command/find_program.rs | 80 ------------------- .../process_command/fixtures/bin/bin.bat | 0 .../process_command/fixtures/bin/blah.bat | 3 + 4 files changed, 3 insertions(+), 83 deletions(-) delete mode 100644 src/test/run-pass/process_command/find_program.rs delete mode 100644 src/test/run-pass/process_command/fixtures/bin/bin.bat create mode 100644 src/test/run-pass/process_command/fixtures/bin/blah.bat diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index dd2d85f19a2d2..48af431715a21 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -112,9 +112,6 @@ impl Command { pub fn env_clear(&mut self) { self.env = Some(HashMap::new()) } - pub fn with_env(&mut self, env: &HashMap) { - self.env = Some(env) - } pub fn cwd(&mut self, dir: &OsStr) { self.cwd = Some(dir.to_os_string()) } diff --git a/src/test/run-pass/process_command/find_program.rs b/src/test/run-pass/process_command/find_program.rs deleted file mode 100644 index b7a63c9815079..0000000000000 --- a/src/test/run-pass/process_command/find_program.rs +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright 2016 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -#[cfg(test)] -mod test_find_program { - use std::ffi::{OsStr, OsString}; - use std::process::Command; - use std::collections::HashMap; - use std::path::Path; - use std::fs::canonicalize; - use std::env::join_paths; - - fn gen_env() -> HashMap { - let env: HashMap = HashMap::new(); - env.insert(OsString::from("HOMEDRIVE"), OsString::from("C:")); - let p1 = canonicalize("./src/test/run-pass/process_command/fixtures/bin").unwrap(); - let p2 = canonicalize("./src/test/run-pass/process_command/fixtures").unwrap(); - let p3 = canonicalize("./src/test/run-pass/process_command").unwrap(); - let paths = vec![p1, p2, p3]; - let path = join_paths(paths).unwrap(); - env.insert(OsString::from("PATH"), OsString::from(&path)); - env.insert(OsString::from("USERNAME"), OsString::from("rust")); - env - } - - fn command_with_pathext(cmd: &str) -> Command { - let mut env = gen_env(); - env.insert( - OsString::from("PATHEXT"), - OsString::from(".COM;.EXE;.BAT;.CMD;.VBS;.VBE;.MSC") - ); - let mut cmd = Command::new(cmd); - cmd.with_env(&env); - cmd - } - - fn command_without_pathext(cmd: &str) -> Command { - let env = gen_env(); - let mut cmd = Command::new(cmd); - cmd.with_env(&env); - cmd - } - - mod with_pathext_set { - use super::command_with_pathext; - - fn command_on_path_found() { - let c = command_with_pathext("bin"); - let bat = canonicalize("./src/test/run-pass/process_command/fixtures/bin/bin.bat"); - assert_eq!(bat.ok(), c.find_program()); - } - - fn command_not_found() { - let c = command_with_pathext("missing"); - assert_eq!(None, c.find_program()); - } - } - - mod without_pathext_set { - use super::command_without_pathext; - - fn bat_command_not_found() { - let c = command_without_pathext("bin"); - assert_eq!(None, c.find_program()); - } - - fn exe_command_found() { - let c = command_without_pathext("exec"); - let exe = canonicalize("./src/test/run-pass/process_command/fixtures/bin/exec.exe"); - assert_eq!(exe.ok(), c.find_program()); - } - } -} diff --git a/src/test/run-pass/process_command/fixtures/bin/bin.bat b/src/test/run-pass/process_command/fixtures/bin/bin.bat deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/src/test/run-pass/process_command/fixtures/bin/blah.bat b/src/test/run-pass/process_command/fixtures/bin/blah.bat new file mode 100644 index 0000000000000..2031ec8df60ef --- /dev/null +++ b/src/test/run-pass/process_command/fixtures/bin/blah.bat @@ -0,0 +1,3 @@ +@ECHO OFF +SET foo=bar +ECHO testing %foo% From 0444a6d4427c108d441b1cb98239a75afe08f0e0 Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Mon, 31 Oct 2016 17:30:11 -0400 Subject: [PATCH 09/10] Add as_vec_u16 fn Simple fn that will convert a `OsString` into a `Vec`. Very useful when you need to interact with WindowsAPI. Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 36 +++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 48af431715a21..eb35e4de93dcb 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -181,7 +181,7 @@ impl Command { } let (envp, _data) = make_envp(self.env.as_ref())?; - let (dirp, _data) = make_dirp(self.cwd.as_ref())?; + let (dirp, _data) = as_vec_u16(self.cwd.as_ref())?; let mut pi = zeroed_process_information(); // Prepare all stdio handles to be inherited by the child. This @@ -216,7 +216,7 @@ impl Command { si.hStdError = stderr.raw(); unsafe { - cvt(c::CreateProcessW(app_name.unwrap_or(ptr::null()), + cvt(c::CreateProcessW(app_name.unwrap_or(ptr::null_mut()), cmd_str.as_mut_ptr(), ptr::null_mut(), ptr::null_mut(), @@ -423,6 +423,9 @@ fn zeroed_process_information() -> c::PROCESS_INFORMATION { fn make_command_line(prog: &OsStr, args: &[OsString], env: Option<&collections::HashMap>) -> io::Result<(Option>, Vec)> { + // Encode the command and arguments in a command line string such + // that the spawned process may recover them using CommandLineToArgvW. + let mut cmd: Vec = Vec::new(); // If the program is a BATCH file we must start the command interpreter; set // ApplicationName to cmd.exe and set CommandLine to the following arguments: // `/c` plus the name of the batch file. @@ -432,22 +435,18 @@ fn make_command_line(prog: &OsStr, args: &[OsString], if batch.is_some() { if let Some(e) = env { if let Some(cmd_exe) = e.get("COMSPEC") { - app = Some(ensure_no_nuls(cmd_exe)?.encode_wide().collect()); + app = as_vec_u16(cmd_exe).ok().map(|(vec, _d)| vec); } } // If we were unable to find COMSPEC, default to `cmd.exe` - if !app.is_some() { - app = Some(OsStr::new("cmd.exe").encode_wide().collect()); + if app.is_none() { + app = as_vec_u16("cmd.exe").ok().map(|(vec, _d)| vec); } - // Prepend the argument to the program - let mut cmd_prog = OsString::from("cmd /c "); - cmd_prog.push(prog); - let prog = cmd_prog.as_os_str(); + // Prepend the argument to the CommandLine + append_arg(&mut cmd, OsString::from("cmd"))?; + append_arg(&mut cmd, OsString::from("/c"))?; } - // Encode the command and arguments in a command line string such - // that the spawned process may recover them using CommandLineToArgvW. - let mut cmd: Vec = Vec::new(); append_arg(&mut cmd, prog)?; for arg in args { cmd.push(' ' as u16); @@ -519,12 +518,13 @@ fn make_envp(env: Option<&collections::HashMap>) } } -fn make_dirp(d: Option<&OsString>) -> io::Result<(*const u16, Vec)> { - match d { - Some(dir) => { - let mut dir_str: Vec = ensure_no_nuls(dir)?.encode_wide().collect(); - dir_str.push(0); - Ok((dir_str.as_ptr(), dir_str)) +// Convert a OsString to a Vec +fn as_vec_u16(str: Option<&OsString>) -> io::Result<(*const u16, Vec)> { + match str { + Some(s) => { + let mut s_str: Vec = ensure_no_nuls(s)?.encode_wide().collect(); + s_str.push(0); + Ok((s_str.as_ptr(), s_str)) }, None => Ok((ptr::null(), Vec::new())) } From e9e960e5af5fa8f45f391021a4a33bd37a753faa Mon Sep 17 00:00:00 2001 From: Salim Afiune Date: Tue, 1 Nov 2016 11:10:47 -0400 Subject: [PATCH 10/10] Add tests for make_command_line modifications Updating the tests to match the modifications made. The `make_command_line` fn now returns two variables, the application_name and the command_line. It will detect when the program is a batch script or not and use the `cmd.exe` as the interpreter. Signed-off-by: Salim Afiune --- src/libstd/sys/windows/process.rs | 188 ++++++++++++++++++++++++------ 1 file changed, 154 insertions(+), 34 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index eb35e4de93dcb..752aefa7699da 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -532,45 +532,165 @@ fn as_vec_u16(str: Option<&OsString>) -> io::Result<(*const u16, Vec)> { #[cfg(test)] mod tests { - use ffi::{OsStr, OsString}; - use super::make_command_line; + use ffi::OsString; + use path::Path; + use fs::canonicalize; + use env::join_paths; + + fn gen_env() -> HashMap { + let mut env: HashMap = HashMap::new(); + env.insert(OsString::from("HOMEDRIVE"), OsString::from("C:")); + let p1 = canonicalize("./src/test/run-pass/process_command/fixtures/bin").unwrap(); + let p2 = canonicalize("./src/test/run-pass/process_command/fixtures").unwrap(); + let p3 = canonicalize("./src/test/run-pass/process_command").unwrap(); + let paths = vec![p1, p2, p3]; + let path = join_paths(paths).unwrap(); + env.insert(OsString::from("PATH"), OsString::from(&path)); + env.insert(OsString::from("USERNAME"), OsString::from("rust")); + env.insert(OsString::from("COMSPEC"), OsString::from("C:\\Windows\\system32\\cmd.exe")); + return env + } #[test] - fn test_make_command_line_with_out_env() { - fn test_wrapper(prog: &str, args: &[&str]) -> String { - let command_line = &make_command_line(OsStr::new(prog), + mod make_command_line_without_env { + use ffi::{OsStr, OsString}; + use super::make_command_line; + + fn fn_wrapper(prog: &str, args: &[&str]) -> (String, String) { + let (app_name, command_line) = &make_command_line(OsStr::new(prog), &args.iter() .map(|a| OsString::from(a)) .collect::>(), None) - .unwrap(); - String::from_utf16(command_line).unwrap() - } - - assert_eq!( - test_wrapper("prog", &["aaa", "bbb", "ccc"]), - "prog aaa bbb ccc" - ); - - assert_eq!( - test_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa"]), - "\"C:\\Program Files\\blah\\blah.exe\" aaa" - ); - assert_eq!( - test_wrapper("C:\\Program Files\\test", &["aa\"bb"]), - "\"C:\\Program Files\\test\" aa\\\"bb" - ); - assert_eq!( - test_wrapper("echo", &["a b c"]), - "echo \"a b c\"" - ); - assert_eq!( - test_wrapper("echo", &["\" \\\" \\", "\\"]), - "echo \"\\\" \\\\\\\" \\\\\" \\" - ); - assert_eq!( - test_wrapper("\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}", &[]), - "\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}" - ); + .unwrap(); + ( + String::from_utf16(app_name.unwrap_or(&[]).unwrap(), + String::from_utf16(command_line).unwrap() + ) + } + fn simple_prog_with_args() { + let (app, cmd) = fn_wrapper("prog", &["aaa", "bbb", "ccc"]); + assert!(app.is_empty()); + assert_eq!(cmd, "prog aaa bbb ccc"); + } + fn exe_with_args() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa"]); + assert!(app.is_empty()); + assert_eq!(cmd, "\"C:\\Program Files\\blah\\blah.exe\" aaa"); + } + fn prog_without_ext() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\test", &["aa\"bb"]); + assert!(app.is_empty()); + assert_eq!(cmd, "\"C:\\Program Files\\test\" aa\\\"bb"); + } + fn simple_echo() { + let (app, cmd) = fn_wrapper("echo", &["a b c"]); + assert!(app.is_empty()); + assert_eq!(cmd, "echo \"a b c\""); + } + fn command_line_with_special_chars() { + let (app, cmd) = fn_wrapper("echo", &["\" \\\" \\", "\\"]); + assert!(app.is_empty()); + assert_eq!(cmd, "echo \"\\\" \\\\\\\" \\\\\" \\"); + + let (app, cmd) = fn_wrapper("\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}", &[]); + assert!(app.is_empty()); + assert_eq!(cmd, "\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}"); + } + fn batch_file() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\blah\\blah.bat", &["bbb"]); + assert!(!app.is_empty()); + assert_eq!(app, "cmd.exe"); + assert_eq!(cmd, "cmd /c \"C:\\Program Files\\blah\\blah.exe\" bbb"); + } + fn cmd_file() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\cmd program\\cool.cmd", &["install"]); + assert!(!app.is_empty()); + assert_eq!(app, "cmd.exe"); + assert_eq!(cmd, "cmd /c \"C:\\Program Files\\cmd program\\cool.cmd\" install"); + } + fn vbs_file_is_not_a_batch_script() { + let (app, cmd) = fn_wrapper( + "C:\\Users\\afiune\\automate.vbs", + &["all", "the", "things"] + ); + assert!(!app.is_empty()); + assert_eq!(app, "cmd.exe"); + assert_eq!(cmd, "cmd /c \"C:\\Users\\afiune\\automate.vbs\" all the things"); + } + } + + #[test] + mod test_make_command_line_with_env() { + use super::make_command_line; + use ffi::{OsStr, OsString}; + use super::gen_env; + + fn fn_wrapper(prog: &str, args: &[&str]) -> (String, String) { + let env = gen_env(); + let (app_name, command_line) = &make_command_line(OsStr::new(prog), + &args.iter() + .map(|a| OsString::from(a)) + .collect::>(), + Some(&env)) + .unwrap(); + ( + String::from_utf16(app_name.unwrap_or(&[]).unwrap(), + String::from_utf16(command_line).unwrap() + ) + } + fn simple_prog_with_args() { + let (app, cmd) = fn_wrapper("prog.cmd", &["aaa", "bbb", "ccc"]); + assert_eq!(app, "C:\\Windows\\system32\\cmd.exe"); + assert_eq!(cmd, "cmd /c \"prog.cmd\" aaa bbb ccc"); + } + fn exe_with_args() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa"]); + assert!(app.is_empty()); + assert_eq!(cmd, "\"C:\\Program Files\\blah\\blah.exe\" aaa"); + } + fn prog_without_ext() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\test", &["aa\"bb"]); + assert!(app.is_empty()); + assert_eq!(cmd, "\"C:\\Program Files\\test\" aa\\\"bb"); + } + fn simple_echo() { + let (app, cmd) = fn_wrapper("echo", &["a b c"]); + assert!(app.is_empty()); + assert_eq!(cmd, "echo \"a b c\""); + } + fn command_line_with_special_chars() { + let (app, cmd) = fn_wrapper("echo", &["\" \\\" \\", "\\"]); + assert!(app.is_empty()); + assert_eq!(cmd, "echo \"\\\" \\\\\\\" \\\\\" \\"); + + let (app, cmd) = fn_wrapper("\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}", &[]); + assert!(app.is_empty()); + assert_eq!(cmd, "\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}"); + } + fn batch_file() { + let (app, cmd) = fn_wrapper("C:\\Program Files\\blah\\blah.bat", &["bbb"]); + assert!(!app.is_empty()); + assert_eq!(app, "C:\\Windows\\system32\\cmd.exe"); + assert_eq!(cmd, "cmd /c \"C:\\Program Files\\blah\\blah.exe\" bbb"); + } + fn cmd_file() { + let (app, cmd) = fn_wrapper( + "C:\\Program Files\\cmd program\\cool.cmd", + &["install"] + ); + assert!(!app.is_empty()); + assert_eq!(app, "C:\\Windows\\system32\\cmd.exe"); + assert_eq!(cmd, "cmd /c \"C:\\Program Files\\cmd program\\cool.cmd\" install"); + } + fn vbs_file_is_not_a_batch_script() { + let (app, cmd) = fn_wrapper( + "C:\\Users\\afiune\\automate.vbs", + &["all", "the", "things"] + ); + assert!(!app.is_empty()); + assert_eq!(app, "C:\\Windows\\system32\\cmd.exe"); + assert_eq!(cmd, "cmd /c \"C:\\Users\\afiune\\automate.vbs\" all the things"); + } } }