Rollup merge of rust-lang#96391 - ChrisDenton:command-non-verbatim, r=joshtriplett

matthiaskrgr · web-flow · commit 184b0b8a0f13 · 2023-03-20T09:46:47.000+01:00
Windows: make `Command` prefer non-verbatim paths

When spawning Commands, the path we use can end up being queried using `env::current_exe` (or the equivalent in other languages). Not all applications handle these paths properly therefore we should have a stronger preference for non-verbatim paths when spawning processes.
diff --git a/library/std/src/sys/windows/args.rs b/library/std/src/sys/windows/args.rs
@@ -11,10 +11,11 @@ use crate::fmt;
 use crate::io;
 use crate::num::NonZeroU16;
 use crate::os::windows::prelude::*;
-use crate::path::PathBuf;
-use crate::sys::c;
+use crate::path::{Path, PathBuf};
+use crate::sys::path::get_long_path;
 use crate::sys::process::ensure_no_nuls;
 use crate::sys::windows::os::current_exe;
+use crate::sys::{c, to_u16s};
 use crate::sys_common::wstr::WStrUnits;
 use crate::vec;
 
@@ -311,7 +312,7 @@ pub(crate) fn make_bat_command_line(
 /// 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>> {
+pub(crate) fn to_user_path(path: &Path) -> io::Result<Vec<u16>> {
     use crate::ptr;
     use crate::sys::windows::fill_utf16_buf;
 
@@ -324,6 +325,8 @@ pub(crate) fn to_user_path(mut path: Vec<u16>) -> io::Result<Vec<u16>> {
     const N: u16 = b'N' as _;
     const C: u16 = b'C' as _;
 
+    let mut path = to_u16s(path)?;
+
     // 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 {
@@ -337,7 +340,13 @@ pub(crate) fn to_user_path(mut path: Vec<u16>) -> io::Result<Vec<u16>> {
             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 }
+                    if full_path == &path[4..path.len() - 1] {
+                        let mut path: Vec<u16> = full_path.into();
+                        path.push(0);
+                        path
+                    } else {
+                        path
+                    }
                 },
             )
         },
@@ -350,7 +359,9 @@ pub(crate) fn to_user_path(mut path: Vec<u16>) -> io::Result<Vec<u16>> {
                 |buffer, size| c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()),
                 |full_path: &[u16]| {
                     if full_path == &path[6..path.len() - 1] {
-                        full_path.into()
+                        let mut path: Vec<u16> = full_path.into();
+                        path.push(0);
+                        path
                     } else {
                         // Restore the 'C' in "UNC".
                         path[6] = b'C' as u16;
@@ -360,6 +371,6 @@ pub(crate) fn to_user_path(mut path: Vec<u16>) -> io::Result<Vec<u16>> {
             )
         },
         // For everything else, leave the path unchanged.
-        _ => Ok(path),
+        _ => get_long_path(path, false),
     }
 }
diff --git a/library/std/src/sys/windows/path.rs b/library/std/src/sys/windows/path.rs
@@ -220,6 +220,19 @@ fn parse_next_component(path: &OsStr, verbatim: bool) -> (&OsStr, &OsStr) {
 ///
 /// This path may or may not have a verbatim prefix.
 pub(crate) fn maybe_verbatim(path: &Path) -> io::Result<Vec<u16>> {
+    let path = to_u16s(path)?;
+    get_long_path(path, true)
+}
+
+/// Get a normalized absolute path that can bypass path length limits.
+///
+/// Setting prefer_verbatim to true suggests a stronger preference for verbatim
+/// paths even when not strictly necessary. This allows the Windows API to avoid
+/// repeating our work. However, if the path may be given back to users or
+/// passed to other application then it's preferable to use non-verbatim paths
+/// when possible. Non-verbatim paths are better understood by users and handled
+/// by more software.
+pub(crate) fn get_long_path(mut path: Vec<u16>, prefer_verbatim: bool) -> io::Result<Vec<u16>> {
     // Normally the MAX_PATH is 260 UTF-16 code units (including the NULL).
     // However, for APIs such as CreateDirectory[1], the limit is 248.
     //
@@ -243,7 +256,6 @@ pub(crate) fn maybe_verbatim(path: &Path) -> io::Result<Vec<u16>> {
     // \\?\UNC\
     const UNC_PREFIX: &[u16] = &[SEP, SEP, QUERY, SEP, U, N, C, SEP];
 
-    let mut path = to_u16s(path)?;
     if path.starts_with(VERBATIM_PREFIX) || path.starts_with(NT_PREFIX) || path == &[0] {
         // Early return for paths that are already verbatim or empty.
         return Ok(path);
@@ -275,29 +287,34 @@ pub(crate) fn maybe_verbatim(path: &Path) -> io::Result<Vec<u16>> {
         |mut absolute| {
             path.clear();
 
-            // Secondly, add the verbatim prefix. This is easier here because we know the
-            // path is now absolute and fully normalized (e.g. `/` has been changed to `\`).
-            let prefix = match absolute {
-                // C:\ => \\?\C:\
-                [_, COLON, SEP, ..] => VERBATIM_PREFIX,
-                // \\.\ => \\?\
-                [SEP, SEP, DOT, SEP, ..] => {
-                    absolute = &absolute[4..];
-                    VERBATIM_PREFIX
-                }
-                // Leave \\?\ and \??\ as-is.
-                [SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => &[],
-                // \\ => \\?\UNC\
-                [SEP, SEP, ..] => {
-                    absolute = &absolute[2..];
-                    UNC_PREFIX
-                }
-                // Anything else we leave alone.
-                _ => &[],
-            };
-
-            path.reserve_exact(prefix.len() + absolute.len() + 1);
-            path.extend_from_slice(prefix);
+            // Only prepend the prefix if needed.
+            if prefer_verbatim || absolute.len() + 1 >= LEGACY_MAX_PATH {
+                // Secondly, add the verbatim prefix. This is easier here because we know the
+                // path is now absolute and fully normalized (e.g. `/` has been changed to `\`).
+                let prefix = match absolute {
+                    // C:\ => \\?\C:\
+                    [_, COLON, SEP, ..] => VERBATIM_PREFIX,
+                    // \\.\ => \\?\
+                    [SEP, SEP, DOT, SEP, ..] => {
+                        absolute = &absolute[4..];
+                        VERBATIM_PREFIX
+                    }
+                    // Leave \\?\ and \??\ as-is.
+                    [SEP, SEP, QUERY, SEP, ..] | [SEP, QUERY, QUERY, SEP, ..] => &[],
+                    // \\ => \\?\UNC\
+                    [SEP, SEP, ..] => {
+                        absolute = &absolute[2..];
+                        UNC_PREFIX
+                    }
+                    // Anything else we leave alone.
+                    _ => &[],
+                };
+
+                path.reserve_exact(prefix.len() + absolute.len() + 1);
+                path.extend_from_slice(prefix);
+            } else {
+                path.reserve_exact(absolute.len() + 1);
+            }
             path.extend_from_slice(absolute);
             path.push(0);
         },
diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs
@@ -266,11 +266,7 @@ impl Command {
         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,
-                )?,
+                args::make_bat_command_line(&program, &self.args, self.force_quotes_enabled)?,
             )
         } else {
             let cmd_str = make_command_line(&self.program, &self.args, self.force_quotes_enabled)?;
@@ -410,7 +406,7 @@ fn resolve_exe<'a>(
         if has_exe_suffix {
             // The application name is a path to a `.exe` file.
             // Let `CreateProcessW` figure out if it exists or not.
-            return path::maybe_verbatim(Path::new(exe_path));
+            return args::to_user_path(Path::new(exe_path));
         }
         let mut path = PathBuf::from(exe_path);
 
@@ -422,7 +418,7 @@ fn resolve_exe<'a>(
             // It's ok to use `set_extension` here because the intent is to
             // remove the extension that was just added.
             path.set_extension("");
-            return path::maybe_verbatim(&path);
+            return args::to_user_path(&path);
         }
     } else {
         ensure_no_nuls(exe_path)?;
@@ -510,7 +506,7 @@ where
 /// Check if a file exists without following symlinks.
 fn program_exists(path: &Path) -> Option<Vec<u16>> {
     unsafe {
-        let path = path::maybe_verbatim(path).ok()?;
+        let path = args::to_user_path(path).ok()?;
         // Getting attributes using `GetFileAttributesW` does not follow symlinks
         // and it will almost always be successful if the link exists.
         // There are some exceptions for special system files (e.g. the pagefile)
