Skip to content

Commit

Permalink
net: refactor named pipe builders to not use bitfields (tokio-rs#5477)
Browse files Browse the repository at this point in the history
  • Loading branch information
mhils authored and amab8901 committed Feb 27, 2023
1 parent 480f2de commit 4af4da3
Showing 1 changed file with 86 additions and 108 deletions.
194 changes: 86 additions & 108 deletions tokio/src/net/windows/named_pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1729,28 +1729,24 @@ impl AsRawHandle for NamedPipeClient {
}
}

// Helper to set a boolean flag as a bitfield.
macro_rules! bool_flag {
($f:expr, $t:expr, $flag:expr) => {{
let current = $f;

if $t {
$f = current | $flag;
} else {
$f = current & !$flag;
};
}};
}

/// A builder structure for construct a named pipe with named pipe-specific
/// options. This is required to use for named pipe servers who wants to modify
/// pipe-related options.
///
/// See [`ServerOptions::create`].
#[derive(Debug, Clone)]
pub struct ServerOptions {
open_mode: u32,
pipe_mode: u32,
// dwOpenMode
access_inbound: bool,
access_outbound: bool,
first_pipe_instance: bool,
write_dac: bool,
write_owner: bool,
access_system_security: bool,
// dwPipeMode
pipe_mode: PipeMode,
reject_remote_clients: bool,
// other options
max_instances: u32,
out_buffer_size: u32,
in_buffer_size: u32,
Expand All @@ -1771,8 +1767,14 @@ impl ServerOptions {
/// ```
pub fn new() -> ServerOptions {
ServerOptions {
open_mode: windows_sys::PIPE_ACCESS_DUPLEX | windows_sys::FILE_FLAG_OVERLAPPED,
pipe_mode: windows_sys::PIPE_TYPE_BYTE | windows_sys::PIPE_REJECT_REMOTE_CLIENTS,
access_inbound: true,
access_outbound: true,
first_pipe_instance: false,
write_dac: false,
write_owner: false,
access_system_security: false,
pipe_mode: PipeMode::Byte,
reject_remote_clients: true,
max_instances: windows_sys::PIPE_UNLIMITED_INSTANCES,
out_buffer_size: 65536,
in_buffer_size: 65536,
Expand All @@ -1789,14 +1791,7 @@ impl ServerOptions {
///
/// [`dwPipeMode`]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea
pub fn pipe_mode(&mut self, pipe_mode: PipeMode) -> &mut Self {
let is_msg = matches!(pipe_mode, PipeMode::Message);
// Pipe mode is implemented as a bit flag 0x4. Set is message and unset
// is byte.
bool_flag!(
self.pipe_mode,
is_msg,
windows_sys::PIPE_TYPE_MESSAGE | windows_sys::PIPE_READMODE_MESSAGE
);
self.pipe_mode = pipe_mode;
self
}

Expand Down Expand Up @@ -1892,7 +1887,7 @@ impl ServerOptions {
/// # Ok(()) }
/// ```
pub fn access_inbound(&mut self, allowed: bool) -> &mut Self {
bool_flag!(self.open_mode, allowed, windows_sys::PIPE_ACCESS_INBOUND);
self.access_inbound = allowed;
self
}

Expand Down Expand Up @@ -1990,7 +1985,7 @@ impl ServerOptions {
/// # Ok(()) }
/// ```
pub fn access_outbound(&mut self, allowed: bool) -> &mut Self {
bool_flag!(self.open_mode, allowed, windows_sys::PIPE_ACCESS_OUTBOUND);
self.access_outbound = allowed;
self
}

Expand Down Expand Up @@ -2058,11 +2053,7 @@ impl ServerOptions {
/// [`create`]: ServerOptions::create
/// [`FILE_FLAG_FIRST_PIPE_INSTANCE`]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea#pipe_first_pipe_instance
pub fn first_pipe_instance(&mut self, first: bool) -> &mut Self {
bool_flag!(
self.open_mode,
first,
windows_sys::FILE_FLAG_FIRST_PIPE_INSTANCE
);
self.first_pipe_instance = first;
self
}

Expand Down Expand Up @@ -2144,7 +2135,7 @@ impl ServerOptions {
///
/// [`WRITE_DAC`]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea
pub fn write_dac(&mut self, requested: bool) -> &mut Self {
bool_flag!(self.open_mode, requested, windows_sys::WRITE_DAC);
self.write_dac = requested;
self
}

Expand All @@ -2154,7 +2145,7 @@ impl ServerOptions {
///
/// [`WRITE_OWNER`]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea
pub fn write_owner(&mut self, requested: bool) -> &mut Self {
bool_flag!(self.open_mode, requested, windows_sys::WRITE_OWNER);
self.write_owner = requested;
self
}

Expand All @@ -2164,11 +2155,7 @@ impl ServerOptions {
///
/// [`ACCESS_SYSTEM_SECURITY`]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea
pub fn access_system_security(&mut self, requested: bool) -> &mut Self {
bool_flag!(
self.open_mode,
requested,
windows_sys::ACCESS_SYSTEM_SECURITY
);
self.access_system_security = requested;
self
}

Expand All @@ -2179,11 +2166,7 @@ impl ServerOptions {
///
/// [`PIPE_REJECT_REMOTE_CLIENTS`]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createnamedpipea#pipe_reject_remote_clients
pub fn reject_remote_clients(&mut self, reject: bool) -> &mut Self {
bool_flag!(
self.pipe_mode,
reject,
windows_sys::PIPE_REJECT_REMOTE_CLIENTS
);
self.reject_remote_clients = reject;
self
}

Expand Down Expand Up @@ -2329,10 +2312,46 @@ impl ServerOptions {
) -> io::Result<NamedPipeServer> {
let addr = encode_addr(addr);

let pipe_mode = {
let mut mode = if matches!(self.pipe_mode, PipeMode::Message) {
windows_sys::PIPE_TYPE_MESSAGE | windows_sys::PIPE_READMODE_MESSAGE
} else {
windows_sys::PIPE_TYPE_BYTE | windows_sys::PIPE_READMODE_BYTE
};
if self.reject_remote_clients {
mode |= windows_sys::PIPE_REJECT_REMOTE_CLIENTS;
} else {
mode |= windows_sys::PIPE_ACCEPT_REMOTE_CLIENTS;
}
mode
};
let open_mode = {
let mut mode = windows_sys::FILE_FLAG_OVERLAPPED;
if self.access_inbound {
mode |= windows_sys::PIPE_ACCESS_INBOUND;
}
if self.access_outbound {
mode |= windows_sys::PIPE_ACCESS_OUTBOUND;
}
if self.first_pipe_instance {
mode |= windows_sys::FILE_FLAG_FIRST_PIPE_INSTANCE;
}
if self.write_dac {
mode |= windows_sys::WRITE_DAC;
}
if self.write_owner {
mode |= windows_sys::WRITE_OWNER;
}
if self.access_system_security {
mode |= windows_sys::ACCESS_SYSTEM_SECURITY;
}
mode
};

let h = windows_sys::CreateNamedPipeW(
addr.as_ptr(),
self.open_mode,
self.pipe_mode,
open_mode,
pipe_mode,
self.max_instances,
self.out_buffer_size,
self.in_buffer_size,
Expand All @@ -2354,7 +2373,8 @@ impl ServerOptions {
/// See [`ClientOptions::open`].
#[derive(Debug, Clone)]
pub struct ClientOptions {
desired_access: u32,
generic_read: bool,
generic_write: bool,
security_qos_flags: u32,
pipe_mode: PipeMode,
}
Expand All @@ -2375,7 +2395,8 @@ impl ClientOptions {
/// ```
pub fn new() -> Self {
Self {
desired_access: windows_sys::GENERIC_READ | windows_sys::GENERIC_WRITE,
generic_read: true,
generic_write: true,
security_qos_flags: windows_sys::SECURITY_IDENTIFICATION
| windows_sys::SECURITY_SQOS_PRESENT,
pipe_mode: PipeMode::Byte,
Expand All @@ -2389,7 +2410,7 @@ impl ClientOptions {
/// [`GENERIC_READ`]: https://docs.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights
/// [`CreateFile`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew
pub fn read(&mut self, allowed: bool) -> &mut Self {
bool_flag!(self.desired_access, allowed, windows_sys::GENERIC_READ);
self.generic_read = allowed;
self
}

Expand All @@ -2400,7 +2421,7 @@ impl ClientOptions {
/// [`GENERIC_WRITE`]: https://docs.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights
/// [`CreateFile`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew
pub fn write(&mut self, allowed: bool) -> &mut Self {
bool_flag!(self.desired_access, allowed, windows_sys::GENERIC_WRITE);
self.generic_write = allowed;
self
}

Expand Down Expand Up @@ -2518,13 +2539,24 @@ impl ClientOptions {
) -> io::Result<NamedPipeClient> {
let addr = encode_addr(addr);

let desired_access = {
let mut access = 0;
if self.generic_read {
access |= windows_sys::GENERIC_READ;
}
if self.generic_write {
access |= windows_sys::GENERIC_WRITE;
}
access
};

// NB: We could use a platform specialized `OpenOptions` here, but since
// we have access to windows_sys it ultimately doesn't hurt to use
// `CreateFile` explicitly since it allows the use of our already
// well-structured wide `addr` to pass into CreateFileW.
let h = windows_sys::CreateFileW(
addr.as_ptr(),
self.desired_access,
desired_access,
0,
attrs as *mut _,
windows_sys::OPEN_EXISTING,
Expand All @@ -2537,13 +2569,9 @@ impl ClientOptions {
}

if matches!(self.pipe_mode, PipeMode::Message) {
let mut mode = windows_sys::PIPE_READMODE_MESSAGE;
let result = windows_sys::SetNamedPipeHandleState(
h,
&mut mode,
ptr::null_mut(),
ptr::null_mut(),
);
let mode = windows_sys::PIPE_READMODE_MESSAGE;
let result =
windows_sys::SetNamedPipeHandleState(h, &mode, ptr::null_mut(), ptr::null_mut());

if result == 0 {
return Err(io::Error::last_os_error());
Expand Down Expand Up @@ -2666,53 +2694,3 @@ unsafe fn named_pipe_info(handle: RawHandle) -> io::Result<PipeInfo> {
max_instances,
})
}

#[cfg(test)]
mod test {
use self::windows_sys::{
PIPE_READMODE_MESSAGE, PIPE_REJECT_REMOTE_CLIENTS, PIPE_TYPE_BYTE, PIPE_TYPE_MESSAGE,
};
use super::*;

#[test]
fn opts_default_pipe_mode() {
let opts = ServerOptions::new();
assert_eq!(opts.pipe_mode, PIPE_TYPE_BYTE | PIPE_REJECT_REMOTE_CLIENTS);
}

#[test]
fn opts_unset_reject_remote() {
let mut opts = ServerOptions::new();
opts.reject_remote_clients(false);
assert_eq!(opts.pipe_mode & PIPE_REJECT_REMOTE_CLIENTS, 0);
}

#[test]
fn opts_set_pipe_mode_maintains_reject_remote_clients() {
let mut opts = ServerOptions::new();
opts.pipe_mode(PipeMode::Byte);
assert_eq!(opts.pipe_mode, PIPE_TYPE_BYTE | PIPE_REJECT_REMOTE_CLIENTS);

opts.reject_remote_clients(false);
opts.pipe_mode(PipeMode::Byte);
assert_eq!(opts.pipe_mode, PIPE_TYPE_BYTE);

opts.reject_remote_clients(true);
opts.pipe_mode(PipeMode::Byte);
assert_eq!(opts.pipe_mode, PIPE_TYPE_BYTE | PIPE_REJECT_REMOTE_CLIENTS);

opts.reject_remote_clients(false);
opts.pipe_mode(PipeMode::Message);
assert_eq!(opts.pipe_mode, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE);

opts.reject_remote_clients(true);
opts.pipe_mode(PipeMode::Message);
assert_eq!(
opts.pipe_mode,
PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_REJECT_REMOTE_CLIENTS
);

opts.pipe_mode(PipeMode::Byte);
assert_eq!(opts.pipe_mode, PIPE_TYPE_BYTE | PIPE_REJECT_REMOTE_CLIENTS);
}
}

0 comments on commit 4af4da3

Please sign in to comment.