From 913b1177f310957299a46c1dae29546bb96b6552 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Fri, 6 Jan 2023 17:45:13 +0100 Subject: [PATCH] net: refactor named pipe builders to not use bitfields This fixes #5359. --- tokio/src/net/windows/named_pipe.rs | 194 ++++++++++++---------------- 1 file changed, 86 insertions(+), 108 deletions(-) diff --git a/tokio/src/net/windows/named_pipe.rs b/tokio/src/net/windows/named_pipe.rs index 2107c1cdfce..356d5e4bd26 100644 --- a/tokio/src/net/windows/named_pipe.rs +++ b/tokio/src/net/windows/named_pipe.rs @@ -1645,19 +1645,6 @@ 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. @@ -1665,8 +1652,17 @@ macro_rules! bool_flag { /// 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, @@ -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, + 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, @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -2245,10 +2228,46 @@ impl ServerOptions { ) -> io::Result { 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, @@ -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, } @@ -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, @@ -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 } @@ -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 } @@ -2434,13 +2455,24 @@ impl ClientOptions { ) -> io::Result { 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, @@ -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()); @@ -2582,53 +2610,3 @@ unsafe fn named_pipe_info(handle: RawHandle) -> io::Result { 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); - } -}