Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net: refactor named pipe builders to not use bitfields #5477

Merged
merged 1 commit into from
Feb 19, 2023
Merged

net: refactor named pipe builders to not use bitfields #5477

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 86 additions & 108 deletions tokio/src/net/windows/named_pipe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1645,28 +1645,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 @@ -1687,8 +1683,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,
Comment on lines -1690 to -1691
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PIPE_ACCESS_DUPLEX option seems to have disappeared?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PIPE_ACCESS_DUPLEX is PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND.

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 @@ -1705,14 +1707,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 @@ -1808,7 +1803,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 @@ -1906,7 +1901,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 @@ -1974,11 +1969,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 @@ -2060,7 +2051,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 @@ -2070,7 +2061,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 @@ -2080,11 +2071,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 @@ -2095,11 +2082,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 @@ -2245,10 +2228,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 @@ -2270,7 +2289,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 @@ -2291,7 +2311,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 @@ -2305,7 +2326,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 @@ -2316,7 +2337,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 @@ -2434,13 +2455,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 @@ -2453,13 +2485,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 @@ -2582,53 +2610,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);
}
}