diff options
Diffstat (limited to 'library/std/src/sys/windows')
| -rw-r--r-- | library/std/src/sys/windows/args.rs | 159 | ||||
| -rw-r--r-- | library/std/src/sys/windows/c.rs | 6 | ||||
| -rw-r--r-- | library/std/src/sys/windows/handle.rs | 4 | ||||
| -rw-r--r-- | library/std/src/sys/windows/mod.rs | 12 | ||||
| -rw-r--r-- | library/std/src/sys/windows/path.rs | 120 | ||||
| -rw-r--r-- | library/std/src/sys/windows/path/tests.rs | 20 | ||||
| -rw-r--r-- | library/std/src/sys/windows/pipe.rs | 90 | ||||
| -rw-r--r-- | library/std/src/sys/windows/process.rs | 154 | ||||
| -rw-r--r-- | library/std/src/sys/windows/process/tests.rs | 7 | ||||
| -rw-r--r-- | library/std/src/sys/windows/thread_parker.rs | 20 |
10 files changed, 433 insertions, 159 deletions
diff --git a/library/std/src/sys/windows/args.rs b/library/std/src/sys/windows/args.rs index 3919025b080..c5918103fec 100644 --- a/library/std/src/sys/windows/args.rs +++ b/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), + } +} diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 5f14edaf067..0692da1d795 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -1022,6 +1022,12 @@ extern "system" { bWaitAll: BOOL, dwMilliseconds: DWORD, ) -> DWORD; + pub fn CreatePipe( + hReadPipe: *mut HANDLE, + hWritePipe: *mut HANDLE, + lpPipeAttributes: *const SECURITY_ATTRIBUTES, + nSize: DWORD, + ) -> BOOL; pub fn CreateNamedPipeW( lpName: LPCWSTR, dwOpenMode: DWORD, diff --git a/library/std/src/sys/windows/handle.rs b/library/std/src/sys/windows/handle.rs index ef9a8bd6900..3b609825a79 100644 --- a/library/std/src/sys/windows/handle.rs +++ b/library/std/src/sys/windows/handle.rs @@ -221,6 +221,10 @@ impl Handle { Ok(Self(self.0.duplicate(access, inherit, options)?)) } + pub(crate) fn set_inheritable(&self) -> io::Result<()> { + self.0.set_inheritable() + } + /// Performs a synchronous read. /// /// If the handle is opened for asynchronous I/O then this abort the process. diff --git a/library/std/src/sys/windows/mod.rs b/library/std/src/sys/windows/mod.rs index 31c7208bbf1..4e9d408291d 100644 --- a/library/std/src/sys/windows/mod.rs +++ b/library/std/src/sys/windows/mod.rs @@ -156,7 +156,13 @@ pub fn unrolled_find_u16s(needle: u16, haystack: &[u16]) -> Option<usize> { pub fn to_u16s<S: AsRef<OsStr>>(s: S) -> crate::io::Result<Vec<u16>> { fn inner(s: &OsStr) -> crate::io::Result<Vec<u16>> { - let mut maybe_result: Vec<u16> = s.encode_wide().collect(); + // Most paths are ASCII, so reserve capacity for as much as there are bytes + // in the OsStr plus one for the null-terminating character. We are not + // wasting bytes here as paths created by this function are primarily used + // in an ephemeral fashion. + let mut maybe_result = Vec::with_capacity(s.len() + 1); + maybe_result.extend(s.encode_wide()); + if unrolled_find_u16s(0, &maybe_result).is_some() { return Err(crate::io::const_io_error!( ErrorKind::InvalidInput, @@ -190,6 +196,10 @@ where { // Start off with a stack buf but then spill over to the heap if we end up // needing more space. + // + // This initial size also works around `GetFullPathNameW` returning + // incorrect size hints for some short paths: + // https://github.com/dylni/normpath/issues/5 let mut stack_buf = [0u16; 512]; let mut heap_buf = Vec::new(); unsafe { diff --git a/library/std/src/sys/windows/path.rs b/library/std/src/sys/windows/path.rs index e54fcaed495..a0f82207099 100644 --- a/library/std/src/sys/windows/path.rs +++ b/library/std/src/sys/windows/path.rs @@ -50,37 +50,101 @@ pub(crate) fn append_suffix(path: PathBuf, suffix: &OsStr) -> PathBuf { path.into() } +struct PrefixParser<'a, const LEN: usize> { + path: &'a OsStr, + prefix: [u8; LEN], +} + +impl<'a, const LEN: usize> PrefixParser<'a, LEN> { + #[inline] + fn get_prefix(path: &OsStr) -> [u8; LEN] { + let mut prefix = [0; LEN]; + // SAFETY: Only ASCII characters are modified. + for (i, &ch) in path.bytes().iter().take(LEN).enumerate() { + prefix[i] = if ch == b'/' { b'\\' } else { ch }; + } + prefix + } + + fn new(path: &'a OsStr) -> Self { + Self { path, prefix: Self::get_prefix(path) } + } + + fn as_slice(&self) -> PrefixParserSlice<'a, '_> { + PrefixParserSlice { + path: self.path, + prefix: &self.prefix[..LEN.min(self.path.len())], + index: 0, + } + } +} + +struct PrefixParserSlice<'a, 'b> { + path: &'a OsStr, + prefix: &'b [u8], + index: usize, +} + +impl<'a> PrefixParserSlice<'a, '_> { + fn strip_prefix(&self, prefix: &str) -> Option<Self> { + self.prefix[self.index..] + .starts_with(prefix.as_bytes()) + .then(|| Self { index: self.index + prefix.len(), ..*self }) + } + + fn prefix_bytes(&self) -> &'a [u8] { + &self.path.bytes()[..self.index] + } + + fn finish(self) -> &'a OsStr { + // SAFETY: The unsafety here stems from converting between &OsStr and + // &[u8] and back. This is safe to do because (1) we only look at ASCII + // contents of the encoding and (2) new &OsStr values are produced only + // from ASCII-bounded slices of existing &OsStr values. + unsafe { bytes_as_os_str(&self.path.bytes()[self.index..]) } + } +} + pub fn parse_prefix(path: &OsStr) -> Option<Prefix<'_>> { use Prefix::{DeviceNS, Disk, Verbatim, VerbatimDisk, VerbatimUNC, UNC}; - if let Some(path) = strip_prefix(path, r"\\") { + let parser = PrefixParser::<8>::new(path); + let parser = parser.as_slice(); + if let Some(parser) = parser.strip_prefix(r"\\") { // \\ - if let Some(path) = strip_prefix(path, r"?\") { + + // The meaning of verbatim paths can change when they use a different + // separator. + if let Some(parser) = parser.strip_prefix(r"?\") && !parser.prefix_bytes().iter().any(|&x| x == b'/') { // \\?\ - if let Some(path) = strip_prefix(path, r"UNC\") { + if let Some(parser) = parser.strip_prefix(r"UNC\") { // \\?\UNC\server\share + let path = parser.finish(); let (server, path) = parse_next_component(path, true); let (share, _) = parse_next_component(path, true); Some(VerbatimUNC(server, share)) } else { - let (prefix, _) = parse_next_component(path, true); + let path = parser.finish(); // in verbatim paths only recognize an exact drive prefix - if let Some(drive) = parse_drive_exact(prefix) { + if let Some(drive) = parse_drive_exact(path) { // \\?\C: Some(VerbatimDisk(drive)) } else { // \\?\prefix + let (prefix, _) = parse_next_component(path, true); Some(Verbatim(prefix)) } } - } else if let Some(path) = strip_prefix(path, r".\") { + } else if let Some(parser) = parser.strip_prefix(r".\") { // \\.\COM42 + let path = parser.finish(); let (prefix, _) = parse_next_component(path, false); Some(DeviceNS(prefix)) } else { + let path = parser.finish(); let (server, path) = parse_next_component(path, false); let (share, _) = parse_next_component(path, false); @@ -102,31 +166,26 @@ pub fn parse_prefix(path: &OsStr) -> Option<Prefix<'_>> { } // Parses a drive prefix, e.g. "C:" and "C:\whatever" -fn parse_drive(prefix: &OsStr) -> Option<u8> { +fn parse_drive(path: &OsStr) -> Option<u8> { // In most DOS systems, it is not possible to have more than 26 drive letters. // See <https://en.wikipedia.org/wiki/Drive_letter_assignment#Common_assignments>. fn is_valid_drive_letter(drive: &u8) -> bool { drive.is_ascii_alphabetic() } - match prefix.bytes() { + match path.bytes() { [drive, b':', ..] if is_valid_drive_letter(drive) => Some(drive.to_ascii_uppercase()), _ => None, } } // Parses a drive prefix exactly, e.g. "C:" -fn parse_drive_exact(prefix: &OsStr) -> Option<u8> { +fn parse_drive_exact(path: &OsStr) -> Option<u8> { // only parse two bytes: the drive letter and the drive separator - if prefix.len() == 2 { parse_drive(prefix) } else { None } -} - -fn strip_prefix<'a>(path: &'a OsStr, prefix: &str) -> Option<&'a OsStr> { - // `path` and `prefix` are valid wtf8 and utf8 encoded slices respectively, `path[prefix.len()]` - // is thus a code point boundary and `path[prefix.len()..]` is a valid wtf8 encoded slice. - match path.bytes().strip_prefix(prefix.as_bytes()) { - Some(path) => unsafe { Some(bytes_as_os_str(path)) }, - None => None, + if path.bytes().get(2).map(|&x| is_sep_byte(x)).unwrap_or(true) { + parse_drive(path) + } else { + None } } @@ -219,15 +278,7 @@ pub(crate) fn maybe_verbatim(path: &Path) -> io::Result<Vec<u16>> { // SAFETY: `fill_utf16_buf` ensures the `buffer` and `size` are valid. // `lpfilename` is a pointer to a null terminated string that is not // invalidated until after `GetFullPathNameW` returns successfully. - |buffer, size| unsafe { - // While the docs for `GetFullPathNameW` have the standard note - // about needing a `\\?\` path for a long lpfilename, this does not - // appear to be true in practice. - // See: - // https://stackoverflow.com/questions/38036943/getfullpathnamew-and-long-windows-file-paths - // https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html - c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()) - }, + |buffer, size| unsafe { c::GetFullPathNameW(lpfilename, size, buffer, ptr::null_mut()) }, |mut absolute| { path.clear(); @@ -263,9 +314,20 @@ pub(crate) fn maybe_verbatim(path: &Path) -> io::Result<Vec<u16>> { /// Make a Windows path absolute. pub(crate) fn absolute(path: &Path) -> io::Result<PathBuf> { - if path.as_os_str().bytes().starts_with(br"\\?\") { - return Ok(path.into()); + let path = path.as_os_str(); + let prefix = parse_prefix(path); + // Verbatim paths should not be modified. + if prefix.map(|x| x.is_verbatim()).unwrap_or(false) { + // NULs in verbatim paths are rejected for consistency. + if path.bytes().contains(&0) { + return Err(io::const_io_error!( + io::ErrorKind::InvalidInput, + "strings passed to WinAPI cannot contain NULs", + )); + } + return Ok(path.to_owned().into()); } + let path = to_u16s(path)?; let lpfilename = path.as_ptr(); fill_utf16_buf( diff --git a/library/std/src/sys/windows/path/tests.rs b/library/std/src/sys/windows/path/tests.rs index 425c2011b32..8656b04e4f4 100644 --- a/library/std/src/sys/windows/path/tests.rs +++ b/library/std/src/sys/windows/path/tests.rs @@ -94,3 +94,23 @@ fn verbatim() { // A path that contains null is not a valid path. assert!(maybe_verbatim(Path::new("\0")).is_err()); } + +fn parse_prefix(path: &str) -> Option<Prefix<'_>> { + super::parse_prefix(OsStr::new(path)) +} + +#[test] +fn test_parse_prefix_verbatim() { + let prefix = Some(Prefix::VerbatimDisk(b'C')); + assert_eq!(prefix, parse_prefix(r"\\?\C:/windows/system32/notepad.exe")); + assert_eq!(prefix, parse_prefix(r"\\?\C:\windows\system32\notepad.exe")); +} + +#[test] +fn test_parse_prefix_verbatim_device() { + let prefix = Some(Prefix::UNC(OsStr::new("?"), OsStr::new("C:"))); + assert_eq!(prefix, parse_prefix(r"//?/C:/windows/system32/notepad.exe")); + assert_eq!(prefix, parse_prefix(r"//?/C:\windows\system32\notepad.exe")); + assert_eq!(prefix, parse_prefix(r"/\?\C:\windows\system32\notepad.exe")); + assert_eq!(prefix, parse_prefix(r"\\?/C:\windows\system32\notepad.exe")); +} diff --git a/library/std/src/sys/windows/pipe.rs b/library/std/src/sys/windows/pipe.rs index 013c776c476..928bf2439c3 100644 --- a/library/std/src/sys/windows/pipe.rs +++ b/library/std/src/sys/windows/pipe.rs @@ -18,13 +18,20 @@ use crate::sys_common::IntoInner; // Anonymous pipes //////////////////////////////////////////////////////////////////////////////// -pub struct AnonPipe { - inner: Handle, +// A 64kb pipe capacity is the same as a typical Linux default. +const PIPE_BUFFER_CAPACITY: u32 = 64 * 1024; + +pub enum AnonPipe { + Sync(Handle), + Async(Handle), } impl IntoInner<Handle> for AnonPipe { fn into_inner(self) -> Handle { - self.inner + match self { + Self::Sync(handle) => handle, + Self::Async(handle) => handle, + } } } @@ -32,6 +39,35 @@ pub struct Pipes { pub ours: AnonPipe, pub theirs: AnonPipe, } +impl Pipes { + /// Create a new pair of pipes where both pipes are synchronous. + /// + /// These must not be used asynchronously. + pub fn new_synchronous( + ours_readable: bool, + their_handle_inheritable: bool, + ) -> io::Result<Self> { + unsafe { + // If `CreatePipe` succeeds, these will be our pipes. + let mut read = ptr::null_mut(); + let mut write = ptr::null_mut(); + + if c::CreatePipe(&mut read, &mut write, ptr::null(), PIPE_BUFFER_CAPACITY) == 0 { + Err(io::Error::last_os_error()) + } else { + let (ours, theirs) = if ours_readable { (read, write) } else { (write, read) }; + let ours = Handle::from_raw_handle(ours); + let theirs = Handle::from_raw_handle(theirs); + + if their_handle_inheritable { + theirs.set_inheritable()?; + } + + Ok(Pipes { ours: AnonPipe::Sync(ours), theirs: AnonPipe::Sync(theirs) }) + } + } + } +} /// Although this looks similar to `anon_pipe` in the Unix module it's actually /// subtly different. Here we'll return two pipes in the `Pipes` return value, @@ -53,9 +89,6 @@ pub struct Pipes { /// with `OVERLAPPED` instances, but also works out ok if it's only ever used /// once at a time (which we do indeed guarantee). pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Result<Pipes> { - // A 64kb pipe capacity is the same as a typical Linux default. - const PIPE_BUFFER_CAPACITY: u32 = 64 * 1024; - // Note that we specifically do *not* use `CreatePipe` here because // unfortunately the anonymous pipes returned do not support overlapped // operations. Instead, we create a "hopefully unique" name and create a @@ -156,12 +189,9 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res }; opts.security_attributes(&mut sa); let theirs = File::open(Path::new(&name), &opts)?; - let theirs = AnonPipe { inner: theirs.into_inner() }; + let theirs = AnonPipe::Sync(theirs.into_inner()); - Ok(Pipes { - ours: AnonPipe { inner: ours }, - theirs: AnonPipe { inner: theirs.into_inner() }, - }) + Ok(Pipes { ours: AnonPipe::Async(ours), theirs }) } } @@ -171,12 +201,12 @@ pub fn anon_pipe(ours_readable: bool, their_handle_inheritable: bool) -> io::Res /// This is achieved by creating a new set of pipes and spawning a thread that /// relays messages between the source and the synchronous pipe. pub fn spawn_pipe_relay( - source: &AnonPipe, + source: &Handle, ours_readable: bool, their_handle_inheritable: bool, ) -> io::Result<AnonPipe> { // We need this handle to live for the lifetime of the thread spawned below. - let source = source.duplicate()?; + let source = AnonPipe::Async(source.duplicate(0, true, c::DUPLICATE_SAME_ACCESS)?); // create a new pair of anon pipes. let Pipes { theirs, ours } = anon_pipe(ours_readable, their_handle_inheritable)?; @@ -227,19 +257,24 @@ type AlertableIoFn = unsafe extern "system" fn( impl AnonPipe { pub fn handle(&self) -> &Handle { - &self.inner + match self { + Self::Async(ref handle) => handle, + Self::Sync(ref handle) => handle, + } } pub fn into_handle(self) -> Handle { - self.inner - } - fn duplicate(&self) -> io::Result<Self> { - self.inner.duplicate(0, false, c::DUPLICATE_SAME_ACCESS).map(|inner| AnonPipe { inner }) + self.into_inner() } pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> { let result = unsafe { let len = crate::cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD; - self.alertable_io_internal(c::ReadFileEx, buf.as_mut_ptr() as _, len) + match self { + Self::Sync(ref handle) => handle.read(buf), + Self::Async(_) => { + self.alertable_io_internal(c::ReadFileEx, buf.as_mut_ptr() as _, len) + } + } }; match result { @@ -253,28 +288,33 @@ impl AnonPipe { } pub fn read_vectored(&self, bufs: &mut [IoSliceMut<'_>]) -> io::Result<usize> { - self.inner.read_vectored(bufs) + io::default_read_vectored(|buf| self.read(buf), bufs) } #[inline] pub fn is_read_vectored(&self) -> bool { - self.inner.is_read_vectored() + false } pub fn write(&self, buf: &[u8]) -> io::Result<usize> { unsafe { let len = crate::cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD; - self.alertable_io_internal(c::WriteFileEx, buf.as_ptr() as _, len) + match self { + Self::Sync(ref handle) => handle.write(buf), + Self::Async(_) => { + self.alertable_io_internal(c::WriteFileEx, buf.as_ptr() as _, len) + } + } } } pub fn write_vectored(&self, bufs: &[IoSlice<'_>]) -> io::Result<usize> { - self.inner.write_vectored(bufs) + io::default_write_vectored(|buf| self.write(buf), bufs) } #[inline] pub fn is_write_vectored(&self) -> bool { - self.inner.is_write_vectored() + false } /// Synchronizes asynchronous reads or writes using our anonymous pipe. @@ -346,7 +386,7 @@ impl AnonPipe { // Asynchronous read of the pipe. // If successful, `callback` will be called once it completes. - let result = io(self.inner.as_handle(), buf, len, &mut overlapped, callback); + let result = io(self.handle().as_handle(), buf, len, &mut overlapped, callback); if result == c::FALSE { // We can return here because the call failed. // After this we must not return until the I/O completes. diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index a0c0f5dc3ec..cc29d1a72fb 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -17,17 +17,18 @@ 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; use crate::sys::fs::{File, OpenOptions}; use crate::sys::handle::Handle; use crate::sys::path; -use crate::sys::pipe::{self, AnonPipe}; +use crate::sys::pipe::{self, AnonPipe, Pipes}; 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 { @@ -172,7 +173,7 @@ pub enum Stdio { Inherit, Null, MakePipe, - Pipe(AnonPipe), + AsyncPipe(Handle), Handle(Handle), } @@ -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. @@ -523,13 +527,33 @@ impl Stdio { }, Stdio::MakePipe => { - let ours_readable = stdio_id != c::STD_INPUT_HANDLE; - let pipes = pipe::anon_pipe(ours_readable, true)?; + // Handles that are passed to a child process must be synchronous + // because they will be read synchronously (see #95759). + // Therefore we prefer to make both ends of a pipe synchronous + // just in case our end of the pipe is passed to another process. + // + // However, we may need to read from both the child's stdout and + // stderr simultaneously when waiting for output. This requires + // async reads so as to avoid blocking either pipe. + // + // The solution used here is to make handles synchronous + // except for our side of the stdout and sterr pipes. + // If our side of those pipes do end up being given to another + // process then we use a "pipe relay" to synchronize access + // (see `Stdio::AsyncPipe` below). + let pipes = if stdio_id == c::STD_INPUT_HANDLE { + // For stdin both sides of the pipe are synchronous. + Pipes::new_synchronous(false, true)? + } else { + // For stdout/stderr our side of the pipe is async and their side is synchronous. + pipe::anon_pipe(true, true)? + }; *pipe = Some(pipes.ours); Ok(pipes.theirs.into_handle()) } - Stdio::Pipe(ref source) => { + Stdio::AsyncPipe(ref source) => { + // We need to synchronize asynchronous pipes by using a pipe relay. let ours_readable = stdio_id != c::STD_INPUT_HANDLE; pipe::spawn_pipe_relay(source, ours_readable, true).map(AnonPipe::into_handle) } @@ -558,7 +582,13 @@ impl Stdio { impl From<AnonPipe> for Stdio { fn from(pipe: AnonPipe) -> Stdio { - Stdio::Pipe(pipe) + // Note that it's very important we don't give async handles to child processes. + // Therefore if the pipe is asynchronous we must have a way to turn it synchronous. + // See #95759. + match pipe { + AnonPipe::Sync(handle) => Stdio::Handle(handle), + AnonPipe::Async(handle) => Stdio::AsyncPipe(handle), + } } } @@ -730,96 +760,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>)> { diff --git a/library/std/src/sys/windows/process/tests.rs b/library/std/src/sys/windows/process/tests.rs index cd535afb4a3..be3a0f4ed52 100644 --- a/library/std/src/sys/windows/process/tests.rs +++ b/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() diff --git a/library/std/src/sys/windows/thread_parker.rs b/library/std/src/sys/windows/thread_parker.rs index 3497da51dee..51448344475 100644 --- a/library/std/src/sys/windows/thread_parker.rs +++ b/library/std/src/sys/windows/thread_parker.rs @@ -58,6 +58,7 @@ // [4]: Windows Internals, Part 1, ISBN 9780735671300 use crate::convert::TryFrom; +use crate::pin::Pin; use crate::ptr; use crate::sync::atomic::{ AtomicI8, AtomicPtr, @@ -95,13 +96,16 @@ const NOTIFIED: i8 = 1; // Ordering::Release when writing NOTIFIED (the 'token') in unpark(), and using // Ordering::Acquire when reading this state in park() after waking up. impl Parker { - pub fn new() -> Self { - Self { state: AtomicI8::new(EMPTY) } + /// Construct the Windows parker. The UNIX parker implementation + /// requires this to happen in-place. + pub unsafe fn new(parker: *mut Parker) { + parker.write(Self { state: AtomicI8::new(EMPTY) }); } // Assumes this is only called by the thread that owns the Parker, - // which means that `self.state != PARKED`. - pub unsafe fn park(&self) { + // which means that `self.state != PARKED`. This implementation doesn't require `Pin`, + // but other implementations do. + pub unsafe fn park(self: Pin<&Self>) { // Change NOTIFIED=>EMPTY or EMPTY=>PARKED, and directly return in the // first case. if self.state.fetch_sub(1, Acquire) == NOTIFIED { @@ -132,8 +136,9 @@ impl Parker { } // Assumes this is only called by the thread that owns the Parker, - // which means that `self.state != PARKED`. - pub unsafe fn park_timeout(&self, timeout: Duration) { + // which means that `self.state != PARKED`. This implementation doesn't require `Pin`, + // but other implementations do. + pub unsafe fn park_timeout(self: Pin<&Self>, timeout: Duration) { // Change NOTIFIED=>EMPTY or EMPTY=>PARKED, and directly return in the // first case. if self.state.fetch_sub(1, Acquire) == NOTIFIED { @@ -184,7 +189,8 @@ impl Parker { } } - pub fn unpark(&self) { + // This implementation doesn't require `Pin`, but other implementations do. + pub fn unpark(self: Pin<&Self>) { // Change PARKED=>NOTIFIED, EMPTY=>NOTIFIED, or NOTIFIED=>NOTIFIED, and // wake the thread in the first case. // |