Auto merge of #95246 - ChrisDenton:command-args, r=joshtriplett

Windows Command: Don't run batch files using verbatim paths

Fixes #95178

Note that the first commit does some minor refactoring (moving command line argument building to args.rs). The actual changes are in the second.

Author: bors

library/std/src/process/tests.rs

@@ -435,3 +435,24 @@ fn run_bat_script() {
     assert!(output.status.success());
     assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "Hello, fellow Rustaceans!");
 }
+
+// See issue #95178
+#[test]
+#[cfg(windows)]
+fn run_canonical_bat_script() {
+    let tempdir = crate::sys_common::io::test::tmpdir();
+    let script_path = tempdir.join("hello.cmd");
+
+    crate::fs::write(&script_path, "@echo Hello, %~1!").unwrap();
+
+    // Try using a canonical path
+    let output = Command::new(&script_path.canonicalize().unwrap())
+        .arg("fellow Rustaceans")
+        .stdout(crate::process::Stdio::piped())
+        .spawn()
+        .unwrap()
+        .wait_with_output()
+        .unwrap();
+    assert!(output.status.success());
+    assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), "Hello, fellow Rustaceans!");
+}

library/std/src/sys/windows/args.rs

@@ -8,12 +8,14 @@ mod tests;
 
 use crate::ffi::OsString;
 use crate::fmt;
+use crate::io;
 use crate::marker::PhantomData;
 use crate::num::NonZeroU16;
 use crate::os::windows::prelude::*;
 use crate::path::PathBuf;
 use crate::ptr::NonNull;
 use crate::sys::c;
+use crate::sys::process::ensure_no_nuls;
 use crate::sys::windows::os::current_exe;
 use crate::vec;
 
@@ -234,3 +236,160 @@ impl Iterator for WStrUnits<'_> {
         }
     }
 }
+
+#[derive(Debug)]
+pub(crate) enum Arg {
+    /// Add quotes (if needed)
+    Regular(OsString),
+    /// Append raw string without quoting
+    Raw(OsString),
+}
+
+enum Quote {
+    // Every arg is quoted
+    Always,
+    // Whitespace and empty args are quoted
+    Auto,
+    // Arg appended without any changes (#29494)
+    Never,
+}
+
+pub(crate) fn append_arg(cmd: &mut Vec<u16>, arg: &Arg, force_quotes: bool) -> io::Result<()> {
+    let (arg, quote) = match arg {
+        Arg::Regular(arg) => (arg, if force_quotes { Quote::Always } else { Quote::Auto }),
+        Arg::Raw(arg) => (arg, Quote::Never),
+    };
+
+    // If an argument has 0 characters then we need to quote it to ensure
+    // that it actually gets passed through on the command line or otherwise
+    // it will be dropped entirely when parsed on the other end.
+    ensure_no_nuls(arg)?;
+    let arg_bytes = arg.bytes();
+    let (quote, escape) = match quote {
+        Quote::Always => (true, true),
+        Quote::Auto => {
+            (arg_bytes.iter().any(|c| *c == b' ' || *c == b'\t') || arg_bytes.is_empty(), true)
+        }
+        Quote::Never => (false, false),
+    };
+    if quote {
+        cmd.push('"' as u16);
+    }
+
+    let mut backslashes: usize = 0;
+    for x in arg.encode_wide() {
+        if escape {
+            if x == '\\' as u16 {
+                backslashes += 1;
+            } else {
+                if x == '"' as u16 {
+                    // Add n+1 backslashes to total 2n+1 before internal '"'.
+                    cmd.extend((0..=backslashes).map(|_| '\\' as u16));
+                }
+                backslashes = 0;
+            }
+        }
+        cmd.push(x);
+    }
+
+    if quote {
+        // Add n backslashes to total 2n before ending '"'.
+        cmd.extend((0..backslashes).map(|_| '\\' as u16));
+        cmd.push('"' as u16);
+    }
+    Ok(())
+}
+
+pub(crate) fn make_bat_command_line(
+    script: &[u16],
+    args: &[Arg],
+    force_quotes: bool,
+) -> io::Result<Vec<u16>> {
+    // Set the start of the command line to `cmd.exe /c "`
+    // It is necessary to surround the command in an extra pair of quotes,
+    // hence the trailing quote here. It will be closed after all arguments
+    // have been added.
+    let mut cmd: Vec<u16> = "cmd.exe /c \"".encode_utf16().collect();
+
+    // Push the script name surrounded by its quote pair.
+    cmd.push(b'"' as u16);
+    // Windows file names cannot contain a `"` character or end with `\\`.
+    // If the script name does then return an error.
+    if script.contains(&(b'"' as u16)) || script.last() == Some(&(b'\\' as u16)) {
+        return Err(io::const_io_error!(
+            io::ErrorKind::InvalidInput,
+            "Windows file names may not contain `\"` or end with `\\`"
+        ));
+    }
+    cmd.extend_from_slice(script.strip_suffix(&[0]).unwrap_or(script));
+    cmd.push(b'"' as u16);
+
+    // Append the arguments.
+    // FIXME: This needs tests to ensure that the arguments are properly
+    // reconstructed by the batch script by default.
+    for arg in args {
+        cmd.push(' ' as u16);
+        append_arg(&mut cmd, arg, force_quotes)?;
+    }
+
+    // Close the quote we left opened earlier.
+    cmd.push(b'"' as u16);
+
+    Ok(cmd)
+}
+
+/// Takes a path and tries to return a non-verbatim path.
+///
+/// This is necessary because cmd.exe does not support verbatim paths.
+pub(crate) fn to_user_path(mut path: Vec<u16>) -> io::Result<Vec<u16>> {
+    use crate::ptr;
+    use crate::sys::windows::fill_utf16_buf;
+
+    // UTF-16 encoded code points, used in parsing and building UTF-16 paths.
+    // All of these are in the ASCII range so they can be cast directly to `u16`.
+    const SEP: u16 = b'\\' as _;
+    const QUERY: u16 = b'?' as _;
+    const COLON: u16 = b':' as _;
+    const U: u16 = b'U' as _;
+    const N: u16 = b'N' as _;
+    const C: u16 = b'C' as _;
+
+    // Early return if the path is too long to remove the verbatim prefix.
+    const LEGACY_MAX_PATH: usize = 260;
+    if path.len() > LEGACY_MAX_PATH {
+        return Ok(path);
+    }
+
+    match &path[..] {
+        // `\\?\C:\...` => `C:\...`
+        [SEP, SEP, QUERY, SEP, _, COLON, SEP, ..] => unsafe {
+            let lpfilename = path[4..].as_ptr();
+            fill_utf16_buf(
+                |buffer, size| c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()),
+                |full_path: &[u16]| {
+                    if full_path == &path[4..path.len() - 1] { full_path.into() } else { path }
+                },
+            )
+        },
+        // `\\?\UNC\...` => `\\...`
+        [SEP, SEP, QUERY, SEP, U, N, C, SEP, ..] => unsafe {
+            // Change the `C` in `UNC\` to `\` so we can get a slice that starts with `\\`.
+            path[6] = b'\\' as u16;
+            let lpfilename = path[6..].as_ptr();
+            fill_utf16_buf(
+                |buffer, size| c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()),
+                |full_path: &[u16]| {
+                    if full_path == &path[6..path.len() - 1] {
+                        full_path.into()
+                    } else {
+                        // Restore the 'C' in "UNC".
+                        path[6] = b'C' as u16;
+                        path
+                    }
+                },
+            )
+        },
+        // For everything else, leave the path unchanged.
+        _ => Ok(path),
+    }
+}

library/std/src/sys/windows/process.rs

@@ -17,6 +17,7 @@ use crate::os::windows::ffi::{OsStrExt, OsStringExt};
 use crate::os::windows::io::{AsRawHandle, FromRawHandle, IntoRawHandle};
 use crate::path::{Path, PathBuf};
 use crate::ptr;
+use crate::sys::args::{self, Arg};
 use crate::sys::c;
 use crate::sys::c::NonZeroDWORD;
 use crate::sys::cvt;
@@ -27,7 +28,7 @@ use crate::sys::pipe::{self, AnonPipe};
 use crate::sys::stdio;
 use crate::sys_common::mutex::StaticMutex;
 use crate::sys_common::process::{CommandEnv, CommandEnvs};
-use crate::sys_common::{AsInner, IntoInner};
+use crate::sys_common::IntoInner;
 
 use libc::{c_void, EXIT_FAILURE, EXIT_SUCCESS};
 
@@ -147,7 +148,7 @@ impl AsRef<OsStr> for EnvKey {
     }
 }
 
-fn ensure_no_nuls<T: AsRef<OsStr>>(str: T) -> io::Result<T> {
+pub(crate) fn ensure_no_nuls<T: AsRef<OsStr>>(str: T) -> io::Result<T> {
     if str.as_ref().encode_wide().any(|b| b == 0) {
         Err(io::const_io_error!(ErrorKind::InvalidInput, "nul byte found in provided data"))
     } else {
@@ -182,14 +183,6 @@ pub struct StdioPipes {
     pub stderr: Option<AnonPipe>,
 }
 
-#[derive(Debug)]
-enum Arg {
-    /// Add quotes (if needed)
-    Regular(OsString),
-    /// Append raw string without quoting
-    Raw(OsString),
-}
-
 impl Command {
     pub fn new(program: &OsStr) -> Command {
         Command {
@@ -275,8 +268,19 @@ impl Command {
             program.len().checked_sub(5).and_then(|i| program.get(i..)),
             Some([46, 98 | 66, 97 | 65, 116 | 84, 0] | [46, 99 | 67, 109 | 77, 100 | 68, 0])
         );
-        let mut cmd_str =
-            make_command_line(&program, &self.args, self.force_quotes_enabled, is_batch_file)?;
+        let (program, mut cmd_str) = if is_batch_file {
+            (
+                command_prompt()?,
+                args::make_bat_command_line(
+                    &args::to_user_path(program)?,
+                    &self.args,
+                    self.force_quotes_enabled,
+                )?,
+            )
+        } else {
+            let cmd_str = make_command_line(&self.program, &self.args, self.force_quotes_enabled)?;
+            (program, cmd_str)
+        };
         cmd_str.push(0); // add null terminator
 
         // stolen from the libuv code.
@@ -730,96 +734,36 @@ fn zeroed_process_information() -> c::PROCESS_INFORMATION {
     }
 }
 
-enum Quote {
-    // Every arg is quoted
-    Always,
-    // Whitespace and empty args are quoted
-    Auto,
-    // Arg appended without any changes (#29494)
-    Never,
-}
-
 // 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: &[u16],
-    args: &[Arg],
-    force_quotes: bool,
-    is_batch_file: bool,
-) -> io::Result<Vec<u16>> {
+fn make_command_line(argv0: &OsStr, args: &[Arg], force_quotes: bool) -> io::Result<Vec<u16>> {
     // Encode the command and arguments in a command line string such
     // that the spawned process may recover them using CommandLineToArgvW.
     let mut cmd: Vec<u16> = Vec::new();
 
-    // CreateFileW has special handling for .bat and .cmd files, which means we
-    // need to add an extra pair of quotes surrounding the whole command line
-    // so they are properly passed on to the script.
-    // See issue #91991.
-    if is_batch_file {
-        cmd.push(b'"' as u16);
-    }
-
     // Always quote the program name so CreateProcess to avoid ambiguity when
     // the child process parses its arguments.
     // Note that quotes aren't escaped here because they can't be used in arg0.
     // But that's ok because file paths can't contain quotes.
     cmd.push(b'"' as u16);
-    cmd.extend_from_slice(prog.strip_suffix(&[0]).unwrap_or(prog));
+    cmd.extend(argv0.encode_wide());
     cmd.push(b'"' as u16);
 
     for arg in args {
         cmd.push(' ' as u16);
-        let (arg, quote) = match arg {
-            Arg::Regular(arg) => (arg, if force_quotes { Quote::Always } else { Quote::Auto }),
-            Arg::Raw(arg) => (arg, Quote::Never),
-        };
-        append_arg(&mut cmd, arg, quote)?;
-    }
-    if is_batch_file {
-        cmd.push(b'"' as u16);
-    }
-    return Ok(cmd);
-
-    fn append_arg(cmd: &mut Vec<u16>, arg: &OsStr, quote: Quote) -> io::Result<()> {
-        // If an argument has 0 characters then we need to quote it to ensure
-        // that it actually gets passed through on the command line or otherwise
-        // it will be dropped entirely when parsed on the other end.
-        ensure_no_nuls(arg)?;
-        let arg_bytes = &arg.as_inner().inner.as_inner();
-        let (quote, escape) = match quote {
-            Quote::Always => (true, true),
-            Quote::Auto => {
-                (arg_bytes.iter().any(|c| *c == b' ' || *c == b'\t') || arg_bytes.is_empty(), true)
-            }
-            Quote::Never => (false, false),
-        };
-        if quote {
-            cmd.push('"' as u16);
-        }
-
-        let mut backslashes: usize = 0;
-        for x in arg.encode_wide() {
-            if escape {
-                if x == '\\' as u16 {
-                    backslashes += 1;
-                } else {
-                    if x == '"' as u16 {
-                        // Add n+1 backslashes to total 2n+1 before internal '"'.
-                        cmd.extend((0..=backslashes).map(|_| '\\' as u16));
-                    }
-                    backslashes = 0;
-                }
-            }
-            cmd.push(x);
-        }
-
-        if quote {
-            // Add n backslashes to total 2n before ending '"'.
-            cmd.extend((0..backslashes).map(|_| '\\' as u16));
-            cmd.push('"' as u16);
-        }
-        Ok(())
+        args::append_arg(&mut cmd, arg, force_quotes)?;
     }
+    Ok(cmd)
+}
+
+// Get `cmd.exe` for use with bat scripts, encoded as a UTF-16 string.
+fn command_prompt() -> io::Result<Vec<u16>> {
+    let mut system: Vec<u16> = super::fill_utf16_buf(
+        |buf, size| unsafe { c::GetSystemDirectoryW(buf, size) },
+        |buf| buf.into(),
+    )?;
+    system.extend("\\cmd.exe".encode_utf16().chain([0]));
+    Ok(system)
 }
 
 fn make_envp(maybe_env: Option<BTreeMap<EnvKey, OsString>>) -> io::Result<(*mut c_void, Vec<u16>)> {

library/std/src/sys/windows/process/tests.rs

@@ -3,12 +3,11 @@ use super::Arg;
 use crate::env;
 use crate::ffi::{OsStr, OsString};
 use crate::process::Command;
-use crate::sys::to_u16s;
 
 #[test]
 fn test_raw_args() {
     let command_line = &make_command_line(
-        &to_u16s("quoted exe").unwrap(),
+        OsStr::new("quoted exe"),
         &[
             Arg::Regular(OsString::from("quote me")),
             Arg::Raw(OsString::from("quote me *not*")),
@@ -17,7 +16,6 @@ fn test_raw_args() {
             Arg::Regular(OsString::from("optional-quotes")),
         ],
         false,
-        false,
     )
     .unwrap();
     assert_eq!(
@@ -30,10 +28,9 @@ fn test_raw_args() {
 fn test_make_command_line() {
     fn test_wrapper(prog: &str, args: &[&str], force_quotes: bool) -> String {
         let command_line = &make_command_line(
-            &to_u16s(prog).unwrap(),
+            OsStr::new(prog),
             &args.iter().map(|a| Arg::Regular(OsString::from(a))).collect::<Vec<_>>(),
             force_quotes,
-            false,
         )
         .unwrap();
         String::from_utf16(command_line).unwrap()

src/test/ui-fulldeps/issue-15149.rs

@@ -5,10 +5,11 @@
 // ignore-cross-compile
 
 use std::env;
+use std::ffi::OsStr;
 use std::fs;
+use std::path::PathBuf;
 use std::process;
 use std::str;
-use std::path::PathBuf;
 
 fn main() {
     // If we're the child, make sure we were invoked correctly
@@ -18,8 +19,8 @@ fn main() {
         // checking that it ends_with the executable name. This
         // is needed because of Windows, which has a different behavior.
         // See #15149 for more info.
-        return assert!(args[0].ends_with(&format!("mytest{}",
-                                                  env::consts::EXE_SUFFIX)));
+        let my_path = env::current_exe().unwrap();
+        return assert_eq!(my_path.file_stem(), Some(OsStr::new("mytest")));
     }
 
     test();
@@ -28,14 +29,13 @@ fn main() {
 fn test() {
     // If we're the parent, copy our own binary to a new directory.
     let my_path = env::current_exe().unwrap();
-    let my_dir  = my_path.parent().unwrap();
+    let my_dir = my_path.parent().unwrap();
 
     let child_dir = PathBuf::from(env::var_os("RUST_TEST_TMPDIR").unwrap());
     let child_dir = child_dir.join("issue-15140-child");
     fs::create_dir_all(&child_dir).unwrap();
 
-    let child_path = child_dir.join(&format!("mytest{}",
-                                             env::consts::EXE_SUFFIX));
+    let child_path = child_dir.join(&format!("mytest{}", env::consts::EXE_SUFFIX));
     fs::copy(&my_path, &child_path).unwrap();
 
     // Append the new directory to our own PATH.
@@ -45,12 +45,13 @@ fn test() {
         env::join_paths(paths).unwrap()
     };
 
-    let child_output = process::Command::new("mytest").env("PATH", &path)
-                                                      .arg("child")
-                                                      .output().unwrap();
+    let child_output =
+        process::Command::new("mytest").env("PATH", &path).arg("child").output().unwrap();
 
-    assert!(child_output.status.success(),
-            "child assertion failed\n child stdout:\n {}\n child stderr:\n {}",
-            str::from_utf8(&child_output.stdout).unwrap(),
-            str::from_utf8(&child_output.stderr).unwrap());
+    assert!(
+        child_output.status.success(),
+        "child assertion failed\n child stdout:\n {}\n child stderr:\n {}",
+        str::from_utf8(&child_output.stdout).unwrap(),
+        str::from_utf8(&child_output.stderr).unwrap()
+    );
 }