foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00
Code Issues Packages 1 Wiki Activity

fix(ext/io): Fix NUL termination error in windows named pipes (#23379)

Browse source 
Due to a terminating NUL that was placed in a `r#` string, we were not
actually NUL-terminating pipe names on Windows. While this has no
security implications due to the random nature of the prefix, it would
occasionally cause random failures when the trailing garbage would make
the pipe name invalid.
This commit is contained in:
Matt Mastracci 2024-04-15 14:10:09 -06:00 committed by GitHub
parent a080acc1b4
commit 7e4ee02e2e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 28 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
cli/tools/test/mod.rs
View file

@ -1346,18 +1346,8 @@ async fn test_specifiers(
})
});
// TODO(mmastrac): Temporarily limit concurrency in windows testing to avoid named pipe issue:
// *** Unexpected server pipe failure '"\\\\.\\pipe\\deno_pipe_e30f45c9df61b1e4.1198.222\\0"': 3
// This is likely because we're hitting some sort of invisible resource limit
// This limit is both in cli/lsp/testing/execution.rs and cli/tools/test/mod.rs
let concurrent = if cfg!(windows) {
std::cmp::min(concurrent_jobs.get(), 4)
} else {
concurrent_jobs.get()
};
let join_stream = stream::iter(join_handles)
.buffer_unordered(concurrent)
.buffer_unordered(concurrent_jobs.get())
.collect::<Vec<Result<Result<(), AnyError>, tokio::task::JoinError>>>();
let handler = spawn(async move { report_tests(receiver, reporter).await.0 });

31
ext/io/winpipe.rs
View file

@ -5,7 +5,6 @@ use std::io;
use std::os::windows::io::RawHandle;
use std::sync::atomic::AtomicU32;
use std::sync::atomic::Ordering;
use std::time::Duration;
use winapi::shared::minwindef::DWORD;
use winapi::um::errhandlingapi::GetLastError;
use winapi::um::fileapi::CreateFileA;
@ -31,12 +30,6 @@ use winapi::um::winnt::GENERIC_WRITE;
/// well as offering a complex NTAPI solution if we decide to try to make these pipes truely
/// anonymous: https://stackoverflow.com/questions/60645/overlapped-i-o-on-anonymous-pipe
pub fn create_named_pipe() -> io::Result<(RawHandle, RawHandle)> {
// Silently retry up to 10 times.
for _ in 0..10 {
if let Ok(res) = create_named_pipe_inner() {
return Ok(res);
}
}
create_named_pipe_inner()
}
@ -44,7 +37,7 @@ fn create_named_pipe_inner() -> io::Result<(RawHandle, RawHandle)> {
static NEXT_ID: AtomicU32 = AtomicU32::new(0);
// Create an extremely-likely-unique pipe name from randomness, identity and a serial counter.
let pipe_name = format!(
r#"\\.\pipe\deno_pipe_{:x}.{:x}.{:x}\0"#,
concat!(r#"\\.\pipe\deno_pipe_{:x}.{:x}.{:x}"#, "\0"),
thread_rng().next_u64(),
std::process::id(),
NEXT_ID.fetch_add(1, Ordering::SeqCst),
@ -89,8 +82,6 @@ fn create_named_pipe_inner() -> io::Result<(RawHandle, RawHandle)> {
return Err(io::Error::last_os_error());
}
// The pipe might not be ready yet in rare cases, so we loop for a bit
for i in 0..10 {
// SAFETY: Create the pipe client with non-inheritable handle
let client_handle = unsafe {
CreateFileA(
@ -104,23 +95,9 @@ fn create_named_pipe_inner() -> io::Result<(RawHandle, RawHandle)> {
)
};
// There is a very rare case where the pipe is not ready to open. If we get `ERROR_PATH_NOT_FOUND`,
// we spin and try again in 1-10ms.
if client_handle == INVALID_HANDLE_VALUE {
// SAFETY: Getting last error for diagnostics
let error = unsafe { GetLastError() };
if error == winapi::shared::winerror::ERROR_FILE_NOT_FOUND
|| error == winapi::shared::winerror::ERROR_PATH_NOT_FOUND
{
// Exponential backoff, but don't sleep longer than 10ms
eprintln!(
"*** Unexpected client pipe not found failure '{pipe_name:?}': {:x}",
error
);
std::thread::sleep(Duration::from_millis(10.min(2_u64.pow(i) + 1)));
continue;
}
// This should not happen, so we would like to get some better diagnostics here.
eprintln!(
"*** Unexpected client pipe failure '{pipe_name:?}': {:x}",
@ -134,11 +111,7 @@ fn create_named_pipe_inner() -> io::Result<(RawHandle, RawHandle)> {
return Err(err);
}
return Ok((server_handle, client_handle));
}
// We failed to open the pipe despite sleeping
Err(std::io::ErrorKind::NotFound.into())
Ok((server_handle, client_handle))
}
#[cfg(test)]