From afe4ed79f7696484d81934c14769ca93ae51dd17 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 15:33:44 +0100 Subject: [PATCH 01/20] Update Cargo.toml Changes the URLs to the repo's new home: https://github.com/rust-lang/socket2-rs. Add keywords, categories and documentation link. --- Cargo.toml | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ec69b0e9..5db3590f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,21 +1,34 @@ [package] -name = "socket2" -version = "0.4.0-dev" -authors = ["Alex Crichton "] -license = "MIT/Apache-2.0" -readme = "README.md" -repository = "https://github.com/alexcrichton/socket2-rs" -homepage = "https://github.com/alexcrichton/socket2-rs" +name = "socket2" +version = "0.4.0-dev" +authors = ["Alex Crichton "] +license = "MIT/Apache-2.0" +readme = "README.md" +repository = "https://github.com/rust-lang/socket2-rs" +homepage = "https://github.com/rust-lang/socket2-rs" +documentation = "https://docs.rs/socket2" description = """ Utilities for handling networking sockets with a maximal amount of configuration possible intended. """ -edition = "2018" +keywords = ["io", "socket", "network"] +categories = ["api-bindings", "network-programming", "web-programming"] +edition = "2018" +include = [ + "Cargo.toml", + "LICENSE-APACHE", + "LICENSE-MIT", + "README.md", + "src/**/*.rs", +] [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] +[package.metadata.playground] +features = ["all"] + [target."cfg(windows)".dependencies.winapi] version = "0.3.3" features = ["handleapi", "ws2def", "ws2ipdef", "ws2tcpip", "minwindef"] From f9e738f785b799b578988989f36b4e5ca0626a1d Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 15:38:53 +0100 Subject: [PATCH 02/20] Put Socket::set_nosigpipe behind the all feature It's only available on Apple platforms. --- src/sys/unix.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/sys/unix.rs b/src/sys/unix.rs index 0c984fc7..42615477 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -331,6 +331,7 @@ pub(crate) fn accept(fd: SysSocket) -> io::Result<(SysSocket, SockAddr)> { }) } +/// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. /// @@ -381,8 +382,12 @@ impl crate::Socket { fcntl_add(self.inner, libc::FD_CLOEXEC) } - /// Sets `SO_NOSIGPIPE` to one. - #[cfg(target_vendor = "apple")] + /// Sets `SO_NOSIGPIPE` on the socket. + /// + /// # Notes + /// + /// Only supported on Apple platforms (`target_vendor = "apple"`). + #[cfg(all(feature = "all", target_vendor = "apple"))] pub fn set_nosigpipe(&self) -> io::Result<()> { unsafe { setsockopt(self.inner, libc::SOL_SOCKET, libc::SO_NOSIGPIPE, 1i32) } } @@ -399,7 +404,7 @@ fn fcntl_add(fd: SysSocket, flag: c_int) -> io::Result<()> { } } -#[cfg(target_vendor = "apple")] +#[cfg(all(feature = "all", target_vendor = "apple"))] unsafe fn setsockopt(fd: SysSocket, opt: c_int, val: c_int, payload: T) -> io::Result<()> where T: Copy, From 8e05ced354dda13ca35b9cc7dc2b0cfd3d49128c Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 15:54:39 +0100 Subject: [PATCH 03/20] Refactor Socket::local_addr --- src/socket.rs | 11 +++++++++-- src/sys/unix.rs | 25 +++++++++++-------------- src/sys/windows.rs | 32 +++++++++++++++----------------- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index b31392a3..a6477e46 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -198,9 +198,16 @@ impl Socket { sys::accept(self.inner).map(|(inner, addr)| (Socket { inner }, addr)) } - /// Returns the socket address of the local half of this TCP connection. + /// Returns the socket address of the local half of this socket. + /// + /// # Notes + /// + /// Depending on the OS this may return an error if the socket is not + /// [bound]. + /// + /// [bound]: Socket::bind pub fn local_addr(&self) -> io::Result { - self.inner().local_addr() + sys::getsockname(self.inner) } /// Returns the socket address of the remote peer of this TCP connection. diff --git a/src/sys/unix.rs b/src/sys/unix.rs index 42615477..568828f5 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -9,6 +9,7 @@ #[cfg(not(target_os = "redox"))] use std::io::{IoSlice, IoSliceMut}; use std::io::{Read, Write}; +use std::mem::{self, size_of_val}; use std::net::Shutdown; use std::net::{self, Ipv4Addr, Ipv6Addr}; #[cfg(feature = "all")] @@ -21,7 +22,7 @@ use std::path::Path; use std::ptr; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; -use std::{cmp, fmt, io, mem}; +use std::{cmp, fmt, io}; use libc::{self, c_void, in6_addr, in_addr, ssize_t}; @@ -324,13 +325,20 @@ pub(crate) fn listen(fd: SysSocket, backlog: i32) -> io::Result<()> { pub(crate) fn accept(fd: SysSocket) -> io::Result<(SysSocket, SockAddr)> { // Safety: zeroed `sockaddr_storage` is valid. let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; - let mut len = mem::size_of_val(&storage) as socklen_t; + let mut len = size_of_val(&storage) as socklen_t; syscall!(accept(fd, &mut storage as *mut _ as *mut _, &mut len)).map(|fd| { let addr = unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }; (fd, addr) }) } +pub(crate) fn getsockname(fd: SysSocket) -> io::Result { + let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; + let mut len = size_of_val(&storage) as libc::socklen_t; + syscall!(getsockname(fd, &mut storage as *mut _ as *mut _, &mut len,)) + .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) +} + /// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. @@ -426,17 +434,6 @@ pub struct Socket { } impl Socket { - pub fn local_addr(&self) -> io::Result { - let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; - let mut len = mem::size_of_val(&storage) as libc::socklen_t; - syscall!(getsockname( - self.fd, - &mut storage as *mut _ as *mut _, - &mut len, - ))?; - Ok(unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) - } - pub fn peer_addr(&self) -> io::Result { let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; let mut len = mem::size_of_val(&storage) as libc::socklen_t; @@ -1052,7 +1049,7 @@ impl fmt::Debug for Socket { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("Socket"); f.field("fd", &self.fd); - if let Ok(addr) = self.local_addr() { + if let Ok(addr) = getsockname(self.fd) { f.field("local_addr", &addr); } if let Ok(addr) = self.peer_addr() { diff --git a/src/sys/windows.rs b/src/sys/windows.rs index 207780cb..3df2a5b8 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -10,7 +10,7 @@ use std::cmp; use std::fmt; use std::io; use std::io::{IoSlice, IoSliceMut, Read, Write}; -use std::mem; +use std::mem::{self, size_of_val}; use std::net::Shutdown; use std::net::{self, Ipv4Addr, Ipv6Addr}; use std::os::windows::prelude::*; @@ -169,7 +169,7 @@ pub(crate) fn listen(socket: SysSocket, backlog: i32) -> io::Result<()> { pub(crate) fn accept(socket: SysSocket) -> io::Result<(SysSocket, SockAddr)> { // Safety: zeroed `SOCKADDR_STORAGE` is valid. let mut storage: SOCKADDR_STORAGE = unsafe { mem::zeroed() }; - let mut len = mem::size_of_val(&storage) as c_int; + let mut len = size_of_val(&storage) as c_int; syscall!( accept(socket, &mut storage as *mut _ as *mut _, &mut len), PartialEq::eq, @@ -181,6 +181,18 @@ pub(crate) fn accept(socket: SysSocket) -> io::Result<(SysSocket, SockAddr)> { }) } +pub(crate) fn getsockname(socket: SysSocket) -> io::Result { + // Safety: zeroed `SOCKADDR_STORAGE` is valid. + let mut storage: SOCKADDR_STORAGE = unsafe { mem::zeroed() }; + let mut len = size_of_val(&storage) as c_int; + syscall!( + getsockname(socket, &mut storage as *mut _ as *mut _, &mut len), + PartialEq::eq, + sock::SOCKET_ERROR + ) + .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) +} + impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. pub fn set_no_inherit(&self) -> io::Result<()> { @@ -199,20 +211,6 @@ pub struct Socket { } impl Socket { - pub fn local_addr(&self) -> io::Result { - unsafe { - let mut storage: SOCKADDR_STORAGE = mem::zeroed(); - let mut len = mem::size_of_val(&storage) as c_int; - if sock::getsockname(self.socket, &mut storage as *mut _ as *mut _, &mut len) != 0 { - return Err(last_error()); - } - Ok(SockAddr::from_raw_parts( - &storage as *const _ as *const _, - len, - )) - } - } - pub fn peer_addr(&self) -> io::Result { unsafe { let mut storage: SOCKADDR_STORAGE = mem::zeroed(); @@ -894,7 +892,7 @@ impl fmt::Debug for Socket { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut f = f.debug_struct("Socket"); f.field("socket", &self.socket); - if let Ok(addr) = self.local_addr() { + if let Ok(addr) = getsockname(self.socket) { f.field("local_addr", &addr); } if let Ok(addr) = self.peer_addr() { From 053735f8f5c1b1c1441a11fe2e1d3e97cf4fa950 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 15:57:07 +0100 Subject: [PATCH 04/20] Refactor Socket::peer_addr --- src/socket.rs | 10 ++++++++-- src/sys/unix.rs | 20 ++++++++------------ src/sys/windows.rs | 28 +++++++++++++--------------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index a6477e46..f730e4ab 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -210,9 +210,15 @@ impl Socket { sys::getsockname(self.inner) } - /// Returns the socket address of the remote peer of this TCP connection. + /// Returns the socket address of the remote peer of this socket. + /// + /// # Notes + /// + /// This returns an error if the socket is not [`connect`ed]. + /// + /// [`connect`ed]: Socket::connect pub fn peer_addr(&self) -> io::Result { - self.inner().peer_addr() + sys::getpeername(self.inner) } /// Creates a new independently owned handle to the underlying socket. diff --git a/src/sys/unix.rs b/src/sys/unix.rs index 568828f5..9fc1ada4 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -339,6 +339,13 @@ pub(crate) fn getsockname(fd: SysSocket) -> io::Result { .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) } +pub(crate) fn getpeername(fd: SysSocket) -> io::Result { + let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; + let mut len = size_of_val(&storage) as libc::socklen_t; + syscall!(getpeername(fd, &mut storage as *mut _ as *mut _, &mut len,)) + .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) +} + /// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. @@ -434,17 +441,6 @@ pub struct Socket { } impl Socket { - pub fn peer_addr(&self) -> io::Result { - let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; - let mut len = mem::size_of_val(&storage) as libc::socklen_t; - syscall!(getpeername( - self.fd, - &mut storage as *mut _ as *mut _, - &mut len, - ))?; - Ok(unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) - } - pub fn try_clone(&self) -> io::Result { // implementation lifted from libstd #[cfg(any(target_os = "android", target_os = "haiku"))] @@ -1052,7 +1048,7 @@ impl fmt::Debug for Socket { if let Ok(addr) = getsockname(self.fd) { f.field("local_addr", &addr); } - if let Ok(addr) = self.peer_addr() { + if let Ok(addr) = getpeername(self.fd) { f.field("peer_addr", &addr); } f.finish() diff --git a/src/sys/windows.rs b/src/sys/windows.rs index 3df2a5b8..28a1806d 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -193,6 +193,18 @@ pub(crate) fn getsockname(socket: SysSocket) -> io::Result { .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) } +pub(crate) fn getpeername(socket: SysSocket) -> io::Result { + // Safety: zeroed `SOCKADDR_STORAGE` is valid. + let mut storage: SOCKADDR_STORAGE = unsafe { mem::zeroed() }; + let mut len = size_of_val(&storage) as c_int; + syscall!( + getpeername(socket, &mut storage as *mut _ as *mut _, &mut len), + PartialEq::eq, + sock::SOCKET_ERROR + ) + .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) +} + impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. pub fn set_no_inherit(&self) -> io::Result<()> { @@ -211,20 +223,6 @@ pub struct Socket { } impl Socket { - pub fn peer_addr(&self) -> io::Result { - unsafe { - let mut storage: SOCKADDR_STORAGE = mem::zeroed(); - let mut len = mem::size_of_val(&storage) as c_int; - if sock::getpeername(self.socket, &mut storage as *mut _ as *mut _, &mut len) != 0 { - return Err(last_error()); - } - Ok(SockAddr::from_raw_parts( - &storage as *const _ as *const _, - len, - )) - } - } - pub fn try_clone(&self) -> io::Result { unsafe { let mut info: sock::WSAPROTOCOL_INFOW = mem::zeroed(); @@ -895,7 +893,7 @@ impl fmt::Debug for Socket { if let Ok(addr) = getsockname(self.socket) { f.field("local_addr", &addr); } - if let Ok(addr) = self.peer_addr() { + if let Ok(addr) = getpeername(self.socket) { f.field("peer_addr", &addr); } f.finish() From 0c002e8b9dc4c72d761f9ceb35ca7b1f9b99ff9c Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 16:58:24 +0100 Subject: [PATCH 05/20] Put Socket::set_no_inherit behind the all feature --- src/sys/windows.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/sys/windows.rs b/src/sys/windows.rs index 28a1806d..2a5afc2c 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -22,11 +22,15 @@ use winapi::ctypes::{c_char, c_ulong}; use winapi::shared::in6addr::*; use winapi::shared::inaddr::*; use winapi::shared::minwindef::DWORD; +#[cfg(feature = "all")] use winapi::shared::ntdef::HANDLE; use winapi::shared::ws2def::{self, *}; use winapi::shared::ws2ipdef::*; +#[cfg(feature = "all")] use winapi::um::handleapi::SetHandleInformation; use winapi::um::processthreadsapi::GetCurrentProcessId; +#[cfg(feature = "all")] +use winapi::um::winbase; use winapi::um::winbase::INFINITE; use winapi::um::winsock2 as sock; @@ -205,12 +209,18 @@ pub(crate) fn getpeername(socket: SysSocket) -> io::Result { .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) } +/// Windows only API. impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. + #[cfg(feature = "all")] pub fn set_no_inherit(&self) -> io::Result<()> { - let r = unsafe { SetHandleInformation(self.inner as HANDLE, HANDLE_FLAG_INHERIT, 0) }; - if r == 0 { - Err(last_error()) + // NOTE: can't use `syscall!` because it expects the function in the + // `sock::` path. + let res = + unsafe { SetHandleInformation(self.inner as HANDLE, winbase::HANDLE_FLAG_INHERIT, 0) }; + if res == 0 { + // Zero means error. + Err(io::Error::last_os_error()) } else { Ok(()) } From e887d430fca9e2e5847d6dbeba7f138a38c6decf Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 17:00:10 +0100 Subject: [PATCH 06/20] Refactor Socket::try_clone --- src/socket.rs | 19 +++++++++----- src/sys/unix.rs | 44 ++++--------------------------- src/sys/windows.rs | 65 +++++++++++++++++++--------------------------- 3 files changed, 43 insertions(+), 85 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index f730e4ab..97e24f29 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -223,14 +223,19 @@ impl Socket { /// Creates a new independently owned handle to the underlying socket. /// - /// The returned `TcpStream` is a reference to the same stream that this - /// object references. Both handles will read and write the same stream of - /// data, and options set on one stream will be propagated to the other - /// stream. + /// # Notes + /// + /// On Unix this uses `F_DUPFD_CLOEXEC` and thus sets the `FD_CLOEXEC` on + /// the returned socket. + /// + /// On Windows this uses `WSA_FLAG_NO_HANDLE_INHERIT` setting inheriting to + /// false. + /// + /// On Windows this can **not** be used function cannot be used on a + /// QOS-enabled socket, see + /// https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsaduplicatesocketw. pub fn try_clone(&self) -> io::Result { - self.inner() - .try_clone() - .map(|s| Socket { inner: s.inner() }) + sys::try_clone(self.inner).map(|inner| Socket { inner }) } /// Get the value of the `SO_ERROR` option on this socket. diff --git a/src/sys/unix.rs b/src/sys/unix.rs index 9fc1ada4..57a93411 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -20,7 +20,6 @@ use std::os::unix::net::{UnixDatagram, UnixListener, UnixStream}; use std::path::Path; #[cfg(feature = "all")] use std::ptr; -use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use std::{cmp, fmt, io}; @@ -346,6 +345,10 @@ pub(crate) fn getpeername(fd: SysSocket) -> io::Result { .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) } +pub(crate) fn try_clone(fd: SysSocket) -> io::Result { + syscall!(fcntl(fd, libc::F_DUPFD_CLOEXEC, 0)) +} + /// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. @@ -408,6 +411,7 @@ impl crate::Socket { } } +/// Add `flag` to the current set flags of `F_GETFD`. fn fcntl_add(fd: SysSocket, flag: c_int) -> io::Result<()> { let previous = syscall!(fcntl(fd, libc::F_GETFD))?; let new = previous | flag; @@ -441,35 +445,6 @@ pub struct Socket { } impl Socket { - pub fn try_clone(&self) -> io::Result { - // implementation lifted from libstd - #[cfg(any(target_os = "android", target_os = "haiku"))] - use libc::F_DUPFD as F_DUPFD_CLOEXEC; - #[cfg(not(any(target_os = "android", target_os = "haiku")))] - use libc::F_DUPFD_CLOEXEC; - - static CLOEXEC_FAILED: AtomicBool = AtomicBool::new(false); - if !CLOEXEC_FAILED.load(Ordering::Relaxed) { - match syscall!(fcntl(self.fd, F_DUPFD_CLOEXEC, 0)) { - Ok(fd) => { - let fd = unsafe { Socket::from_raw_fd(fd) }; - if cfg!(target_os = "linux") { - set_cloexec(fd.as_raw_fd())?; - } - return Ok(fd); - } - Err(ref e) if e.raw_os_error() == Some(libc::EINVAL) => { - CLOEXEC_FAILED.store(true, Ordering::Relaxed); - } - Err(e) => return Err(e), - } - } - let fd = syscall!(fcntl(self.fd, libc::F_DUPFD, 0))?; - let fd = unsafe { Socket::from_raw_fd(fd) }; - set_cloexec(fd.as_raw_fd())?; - Ok(fd) - } - pub fn take_error(&self) -> io::Result> { unsafe { let raw: c_int = self.getsockopt(libc::SOL_SOCKET, libc::SO_ERROR)?; @@ -1191,15 +1166,6 @@ fn max_len() -> usize { } } -fn set_cloexec(fd: c_int) -> io::Result<()> { - let previous = syscall!(fcntl(fd, libc::F_GETFD))?; - let new = previous | libc::FD_CLOEXEC; - if new != previous { - syscall!(fcntl(fd, libc::F_SETFD, new))?; - } - Ok(()) -} - fn dur2timeval(dur: Option) -> io::Result { match dur { Some(dur) => { diff --git a/src/sys/windows.rs b/src/sys/windows.rs index 2a5afc2c..6af06546 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -10,7 +10,7 @@ use std::cmp; use std::fmt; use std::io; use std::io::{IoSlice, IoSliceMut, Read, Write}; -use std::mem::{self, size_of_val}; +use std::mem::{self, size_of_val, MaybeUninit}; use std::net::Shutdown; use std::net::{self, Ipv4Addr, Ipv6Addr}; use std::os::windows::prelude::*; @@ -36,13 +36,11 @@ use winapi::um::winsock2 as sock; use crate::{RecvFlags, SockAddr}; -const HANDLE_FLAG_INHERIT: DWORD = 0x00000001; const MSG_PEEK: c_int = 0x2; const SD_BOTH: c_int = 2; const SD_RECEIVE: c_int = 0; const SD_SEND: c_int = 1; const SIO_KEEPALIVE_VALS: DWORD = 0x98000004; -const WSA_FLAG_OVERLAPPED: DWORD = 0x01; pub use winapi::ctypes::c_int; @@ -151,7 +149,7 @@ pub(crate) fn socket(family: c_int, ty: c_int, protocol: c_int) -> io::Result io::Result { .map(|_| unsafe { SockAddr::from_raw_parts(&storage as *const _ as *const _, len) }) } +pub(crate) fn try_clone(socket: SysSocket) -> io::Result { + let mut info: MaybeUninit = MaybeUninit::uninit(); + syscall!( + WSADuplicateSocketW(socket, GetCurrentProcessId(), info.as_mut_ptr()), + PartialEq::eq, + sock::SOCKET_ERROR + )?; + // Safety: `WSADuplicateSocketW` intialised `info` for us. + let mut info = unsafe { info.assume_init() }; + + syscall!( + WSASocketW( + info.iAddressFamily, + info.iSocketType, + info.iProtocol, + &mut info, + 0, + sock::WSA_FLAG_OVERLAPPED | sock::WSA_FLAG_NO_HANDLE_INHERIT, + ), + PartialEq::eq, + sock::INVALID_SOCKET + ) +} + /// Windows only API. impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. @@ -233,30 +255,6 @@ pub struct Socket { } impl Socket { - pub fn try_clone(&self) -> io::Result { - unsafe { - let mut info: sock::WSAPROTOCOL_INFOW = mem::zeroed(); - let r = sock::WSADuplicateSocketW(self.socket, GetCurrentProcessId(), &mut info); - if r != 0 { - return Err(io::Error::last_os_error()); - } - let socket = sock::WSASocketW( - info.iAddressFamily, - info.iSocketType, - info.iProtocol, - &mut info, - 0, - WSA_FLAG_OVERLAPPED, - ); - let socket = match socket { - sock::INVALID_SOCKET => return Err(last_error()), - n => Socket::from_raw_socket(n as RawSocket), - }; - socket.set_no_inherit()?; - Ok(socket) - } - } - pub fn take_error(&self) -> io::Result> { unsafe { let raw: c_int = self.getsockopt(SOL_SOCKET, SO_ERROR)?; @@ -844,17 +842,6 @@ impl Socket { } } - fn set_no_inherit(&self) -> io::Result<()> { - unsafe { - let r = SetHandleInformation(self.socket as HANDLE, HANDLE_FLAG_INHERIT, 0); - if r == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } - } - } - pub fn inner(self) -> SysSocket { self.socket } From 05bae6df96097e5b7654d91a78e04761b768d174 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 17:07:07 +0100 Subject: [PATCH 07/20] Add Type::no_inherit Sets the WSA_FLAG_NO_HANDLE_INHERIT flag on socket creation on Windows. --- src/socket.rs | 7 ++++--- src/sys/windows.rs | 29 +++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index 97e24f29..5ed08690 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -58,6 +58,10 @@ use crate::{Domain, Protocol, SockAddr, Type}; /// #[cfg(any(target_os = "android", target_os = "dragonfly", target_os = "freebsd", target_os = "linux", target_os = "netbsd", target_os = "openbsd"))] /// let ty = ty.cloexec(); /// +/// // On windows set `WSA_FLAG_NO_HANDLE_INHERIT`. +/// #[cfg(windows)] +/// let ty = ty.no_inherit(); +/// /// let socket = Socket::new(domain, ty, Some(protocol))?; /// /// // On platforms that don't support `SOCK_CLOEXEC`, use `FD_CLOEXEC`. @@ -68,9 +72,6 @@ use crate::{Domain, Protocol, SockAddr, Type}; /// #[cfg(target_vendor = "apple")] /// socket.set_nosigpipe()?; /// -/// // On windows set `HANDLE_FLAG_INHERIT`. -/// #[cfg(windows)] -/// socket.set_no_inherit()?; /// # drop(socket); /// # Ok(()) /// # } diff --git a/src/sys/windows.rs b/src/sys/windows.rs index 6af06546..59635003 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -10,7 +10,7 @@ use std::cmp; use std::fmt; use std::io; use std::io::{IoSlice, IoSliceMut, Read, Write}; -use std::mem::{self, size_of_val, MaybeUninit}; +use std::mem::{self, size_of, size_of_val, MaybeUninit}; use std::net::Shutdown; use std::net::{self, Ipv4Addr, Ipv6Addr}; use std::os::windows::prelude::*; @@ -34,7 +34,7 @@ use winapi::um::winbase; use winapi::um::winbase::INFINITE; use winapi::um::winsock2 as sock; -use crate::{RecvFlags, SockAddr}; +use crate::{RecvFlags, SockAddr, Type}; const MSG_PEEK: c_int = 0x2; const SD_BOTH: c_int = 2; @@ -89,6 +89,19 @@ impl_debug!( ws2def::AF_UNSPEC, // = 0. ); +/// Windows only API. +impl Type { + /// Our custom flag to set `WSA_FLAG_NO_HANDLE_INHERIT` on socket creation. + /// Trying to mimic `Type::cloexec` on windows. + const NO_INHERIT: c_int = 1 << (size_of::()); + + /// Set `WSA_FLAG_NO_HANDLE_INHERIT` on the socket. + #[cfg(feature = "all")] + pub const fn no_inherit(self) -> Type { + Type(self.0 | Type::NO_INHERIT) + } +} + impl_debug!( crate::Type, ws2def::SOCK_STREAM, @@ -139,9 +152,17 @@ fn last_error() -> io::Error { // TODO: rename to `Socket` once the struct `Socket` is no longer used. pub(crate) type SysSocket = sock::SOCKET; -pub(crate) fn socket(family: c_int, ty: c_int, protocol: c_int) -> io::Result { +pub(crate) fn socket(family: c_int, mut ty: c_int, protocol: c_int) -> io::Result { init(); + // Check if we set our custom flag. + let flags = if ty & Type::NO_INHERIT != 0 { + ty = ty & !Type::NO_INHERIT; + sock::WSA_FLAG_NO_HANDLE_INHERIT + } else { + 0 + }; + syscall!( WSASocketW( family, @@ -149,7 +170,7 @@ pub(crate) fn socket(family: c_int, ty: c_int, protocol: c_int) -> io::Result Date: Mon, 9 Nov 2020 17:26:28 +0100 Subject: [PATCH 08/20] Refactor Socket::take_error --- src/socket.rs | 2 +- src/sys/unix.rs | 44 ++++++++++++++++++++++++++++++-------------- src/sys/windows.rs | 35 ++++++++++++++++++++++++----------- 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index 5ed08690..69d8ba91 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -245,7 +245,7 @@ impl Socket { /// the field in the process. This can be useful for checking errors between /// calls. pub fn take_error(&self) -> io::Result> { - self.inner().take_error() + sys::take_error(self.inner) } /// Moves this TCP stream into or out of nonblocking mode. diff --git a/src/sys/unix.rs b/src/sys/unix.rs index 57a93411..a4747d05 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -9,7 +9,7 @@ #[cfg(not(target_os = "redox"))] use std::io::{IoSlice, IoSliceMut}; use std::io::{Read, Write}; -use std::mem::{self, size_of_val}; +use std::mem::{self, size_of_val, MaybeUninit}; use std::net::Shutdown; use std::net::{self, Ipv4Addr, Ipv6Addr}; #[cfg(feature = "all")] @@ -349,6 +349,14 @@ pub(crate) fn try_clone(fd: SysSocket) -> io::Result { syscall!(fcntl(fd, libc::F_DUPFD_CLOEXEC, 0)) } +pub(crate) fn take_error(fd: SysSocket) -> io::Result> { + match unsafe { getsockopt::(fd, libc::SOL_SOCKET, libc::SO_ERROR) } { + Ok(0) => Ok(None), + Ok(errno) => Ok(Some(io::Error::from_raw_os_error(errno))), + Err(err) => Err(err), + } +} + /// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. @@ -423,6 +431,25 @@ fn fcntl_add(fd: SysSocket, flag: c_int) -> io::Result<()> { } } +/// Caller must ensure `T` is the correct type for `opt` and `val`. +unsafe fn getsockopt(fd: SysSocket, opt: c_int, val: c_int) -> io::Result { + let mut payload: MaybeUninit = MaybeUninit::uninit(); + let mut len = mem::size_of::() as libc::socklen_t; + syscall!(getsockopt( + fd, + opt, + val, + payload.as_mut_ptr().cast(), + &mut len, + )) + .map(|_| { + debug_assert_eq!(len as usize, mem::size_of::()); + // Safety: `getsockopt` initialised `payload` for us. + payload.assume_init() + }) +} + +/// Caller must ensure `T` is the correct type for `opt` and `val`. #[cfg(all(feature = "all", target_vendor = "apple"))] unsafe fn setsockopt(fd: SysSocket, opt: c_int, val: c_int, payload: T) -> io::Result<()> where @@ -435,8 +462,8 @@ where val, payload, mem::size_of::() as libc::socklen_t, - ))?; - Ok(()) + )) + .map(|_| ()) } #[repr(transparent)] // Required during rewriting. @@ -445,17 +472,6 @@ pub struct Socket { } impl Socket { - pub fn take_error(&self) -> io::Result> { - unsafe { - let raw: c_int = self.getsockopt(libc::SOL_SOCKET, libc::SO_ERROR)?; - if raw == 0 { - Ok(None) - } else { - Ok(Some(io::Error::from_raw_os_error(raw as i32))) - } - } - } - pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { let previous = syscall!(fcntl(self.fd, libc::F_GETFL))?; let new = if nonblocking { diff --git a/src/sys/windows.rs b/src/sys/windows.rs index 59635003..b3252064 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -72,6 +72,7 @@ pub(crate) use winapi::um::ws2tcpip::socklen_t; /// Helper macro to execute a system call that returns an `io::Result`. macro_rules! syscall { ($fn: ident ( $($arg: expr),* $(,)* ), $err_test: path, $err_value: expr) => {{ + #[allow(unused_unsafe)] let res = unsafe { sock::$fn($($arg, )*) }; if $err_test(&res, &$err_value) { Err(io::Error::last_os_error()) @@ -252,6 +253,29 @@ pub(crate) fn try_clone(socket: SysSocket) -> io::Result { ) } +pub(crate) fn take_error(socket: SysSocket) -> io::Result> { + match unsafe { getsockopt::(socket, SOL_SOCKET, SO_ERROR) } { + Ok(0) => Ok(None), + Ok(errno) => Ok(Some(io::Error::from_raw_os_error(errno))), + Err(err) => Err(err), + } +} + +unsafe fn getsockopt(socket: SysSocket, opt: c_int, val: c_int) -> io::Result { + let mut payload: MaybeUninit = MaybeUninit::uninit(); + let mut len = mem::size_of::() as c_int; + syscall!( + getsockopt(socket, opt, val, payload.as_mut_ptr().cast(), &mut len,), + PartialEq::eq, + sock::SOCKET_ERROR + ) + .map(|_| { + debug_assert_eq!(len as usize, mem::size_of::()); + // Safety: `getsockopt` initialised `payload` for us. + payload.assume_init() + }) +} + /// Windows only API. impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. @@ -276,17 +300,6 @@ pub struct Socket { } impl Socket { - pub fn take_error(&self) -> io::Result> { - unsafe { - let raw: c_int = self.getsockopt(SOL_SOCKET, SO_ERROR)?; - if raw == 0 { - Ok(None) - } else { - Ok(Some(io::Error::from_raw_os_error(raw as i32))) - } - } - } - pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { unsafe { let mut nonblocking = nonblocking as c_ulong; From e024a110a1e6ab034b61d35ee5b5cff4ec3da845 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 17:41:56 +0100 Subject: [PATCH 09/20] Refactor Socket::set_nonblocking --- src/socket.rs | 10 +++++++--- src/sys/unix.rs | 41 ++++++++++++++++++++++++----------------- src/sys/windows.rs | 31 +++++++++++++++++-------------- tests/socket.rs | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 34 deletions(-) create mode 100644 tests/socket.rs diff --git a/src/socket.rs b/src/socket.rs index 69d8ba91..74a8e90b 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -250,10 +250,14 @@ impl Socket { /// Moves this TCP stream into or out of nonblocking mode. /// - /// On Unix this corresponds to calling fcntl, and on Windows this - /// corresponds to calling ioctlsocket. + /// # Notes + /// + /// On Unix this corresponds to calling `fcntl` (un)setting `O_NONBLOCK`. + /// + /// On Windows this corresponds to calling `ioctlsocket` (un)setting + /// `FIONBIO`. pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { - self.inner().set_nonblocking(nonblocking) + sys::set_nonblocking(self.inner, nonblocking) } /// Shuts down the read, write, or both halves of this connection. diff --git a/src/sys/unix.rs b/src/sys/unix.rs index a4747d05..cfabc6f8 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -357,6 +357,14 @@ pub(crate) fn take_error(fd: SysSocket) -> io::Result> { } } +pub(crate) fn set_nonblocking(fd: SysSocket, nonblocking: bool) -> io::Result<()> { + if nonblocking { + fcntl_add(fd, libc::F_GETFL, libc::F_SETFL, libc::O_NONBLOCK) + } else { + fcntl_remove(fd, libc::F_GETFL, libc::F_SETFL, libc::O_NONBLOCK) + } +} + /// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. @@ -405,7 +413,7 @@ impl crate::Socket { /// /// On supported platforms you can use [`Protocol::cloexec`]. pub fn set_cloexec(&self) -> io::Result<()> { - fcntl_add(self.inner, libc::FD_CLOEXEC) + fcntl_add(self.inner, libc::F_GETFD, libc::F_SETFD, libc::FD_CLOEXEC) } /// Sets `SO_NOSIGPIPE` on the socket. @@ -420,11 +428,23 @@ impl crate::Socket { } /// Add `flag` to the current set flags of `F_GETFD`. -fn fcntl_add(fd: SysSocket, flag: c_int) -> io::Result<()> { - let previous = syscall!(fcntl(fd, libc::F_GETFD))?; +fn fcntl_add(fd: SysSocket, get_cmd: c_int, set_cmd: c_int, flag: c_int) -> io::Result<()> { + let previous = syscall!(fcntl(fd, get_cmd))?; let new = previous | flag; if new != previous { - syscall!(fcntl(fd, libc::F_SETFD, new)).map(|_| ()) + syscall!(fcntl(fd, set_cmd, new)).map(|_| ()) + } else { + // Flag was already set. + Ok(()) + } +} + +/// Remove `flag` to the current set flags of `F_GETFD`. +fn fcntl_remove(fd: SysSocket, get_cmd: c_int, set_cmd: c_int, flag: c_int) -> io::Result<()> { + let previous = syscall!(fcntl(fd, get_cmd))?; + let new = previous & !flag; + if new != previous { + syscall!(fcntl(fd, set_cmd, new)).map(|_| ()) } else { // Flag was already set. Ok(()) @@ -472,19 +492,6 @@ pub struct Socket { } impl Socket { - pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { - let previous = syscall!(fcntl(self.fd, libc::F_GETFL))?; - let new = if nonblocking { - previous | libc::O_NONBLOCK - } else { - previous & !libc::O_NONBLOCK - }; - if new != previous { - syscall!(fcntl(self.fd, libc::F_SETFL, new))?; - } - Ok(()) - } - pub fn shutdown(&self, how: Shutdown) -> io::Result<()> { let how = match how { Shutdown::Write => libc::SHUT_WR, diff --git a/src/sys/windows.rs b/src/sys/windows.rs index b3252064..eea16599 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -18,7 +18,7 @@ use std::ptr; use std::sync::Once; use std::time::Duration; -use winapi::ctypes::{c_char, c_ulong}; +use winapi::ctypes::{c_char, c_long, c_ulong}; use winapi::shared::in6addr::*; use winapi::shared::inaddr::*; use winapi::shared::minwindef::DWORD; @@ -32,7 +32,7 @@ use winapi::um::processthreadsapi::GetCurrentProcessId; #[cfg(feature = "all")] use winapi::um::winbase; use winapi::um::winbase::INFINITE; -use winapi::um::winsock2 as sock; +use winapi::um::winsock2::{self as sock, u_long}; use crate::{RecvFlags, SockAddr, Type}; @@ -261,6 +261,12 @@ pub(crate) fn take_error(socket: SysSocket) -> io::Result> { } } +pub(crate) fn set_nonblocking(socket: SysSocket, nonblocking: bool) -> io::Result<()> { + let mut nonblocking = nonblocking as u_long; + ioctlsocket(socket, sock::FIONBIO, &mut nonblocking) +} + +/// Caller must ensure `T` is the correct type for `opt` and `val`. unsafe fn getsockopt(socket: SysSocket, opt: c_int, val: c_int) -> io::Result { let mut payload: MaybeUninit = MaybeUninit::uninit(); let mut len = mem::size_of::() as c_int; @@ -276,6 +282,15 @@ unsafe fn getsockopt(socket: SysSocket, opt: c_int, val: c_int) -> io::Result }) } +fn ioctlsocket(socket: SysSocket, cmd: c_long, payload: &mut u_long) -> io::Result<()> { + syscall!( + ioctlsocket(socket, cmd, payload), + PartialEq::eq, + sock::SOCKET_ERROR + ) + .map(|_| ()) +} + /// Windows only API. impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. @@ -300,18 +315,6 @@ pub struct Socket { } impl Socket { - pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { - unsafe { - let mut nonblocking = nonblocking as c_ulong; - let r = sock::ioctlsocket(self.socket, sock::FIONBIO as c_int, &mut nonblocking); - if r == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } - } - } - pub fn shutdown(&self, how: Shutdown) -> io::Result<()> { let how = match how { Shutdown::Write => SD_SEND, diff --git a/tests/socket.rs b/tests/socket.rs new file mode 100644 index 00000000..8810bacc --- /dev/null +++ b/tests/socket.rs @@ -0,0 +1,33 @@ +#[cfg(unix)] +use std::os::unix::io::AsRawFd; + +use socket2::{Domain, Socket, Type}; + +#[test] +fn set_nonblocking() { + let socket = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); + assert_non_blocking(&socket, false); + + socket.set_nonblocking(true).unwrap(); + assert_non_blocking(&socket, true); + + socket.set_nonblocking(false).unwrap(); + assert_non_blocking(&socket, false); +} + +/// Assert that `NONBLOCK` is set on `socket`. +#[cfg(unix)] +#[track_caller] +pub fn assert_non_blocking(socket: &S, want: bool) +where + S: AsRawFd, +{ + let flags = unsafe { libc::fcntl(socket.as_raw_fd(), libc::F_GETFL) }; + assert_eq!(flags & libc::O_NONBLOCK != 0, want, "non-blocking option"); +} + +#[cfg(windows)] +#[track_caller] +pub fn assert_non_blocking(_: &S, _: bool) { + // No way to get this information... +} From dc7b1ad0e22cad762bdef8b686dd5feab75fa97d Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Mon, 9 Nov 2020 17:45:47 +0100 Subject: [PATCH 10/20] Refactor Socket::shutdown --- src/socket.rs | 2 +- src/sys/unix.rs | 19 +++++++++---------- src/sys/windows.rs | 27 ++++++++++----------------- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index 74a8e90b..efaed726 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -265,7 +265,7 @@ impl Socket { /// This function will cause all pending and future I/O on the specified /// portions to return immediately with an appropriate value. pub fn shutdown(&self, how: Shutdown) -> io::Result<()> { - self.inner().shutdown(how) + sys::shutdown(self.inner, how) } /// Receives data on the socket from the remote address to which it is diff --git a/src/sys/unix.rs b/src/sys/unix.rs index cfabc6f8..ea6e76ea 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -365,6 +365,15 @@ pub(crate) fn set_nonblocking(fd: SysSocket, nonblocking: bool) -> io::Result<() } } +pub(crate) fn shutdown(fd: SysSocket, how: Shutdown) -> io::Result<()> { + let how = match how { + Shutdown::Write => libc::SHUT_WR, + Shutdown::Read => libc::SHUT_RD, + Shutdown::Both => libc::SHUT_RDWR, + }; + syscall!(shutdown(fd, how)).map(|_| ()) +} + /// Unix only API. impl crate::Socket { /// Accept a new incoming connection from this listener. @@ -492,16 +501,6 @@ pub struct Socket { } impl Socket { - pub fn shutdown(&self, how: Shutdown) -> io::Result<()> { - let how = match how { - Shutdown::Write => libc::SHUT_WR, - Shutdown::Read => libc::SHUT_RD, - Shutdown::Both => libc::SHUT_RDWR, - }; - syscall!(shutdown(self.fd, how))?; - Ok(()) - } - pub fn recv(&self, buf: &mut [u8], flags: c_int) -> io::Result { let n = syscall!(recv( self.fd, diff --git a/src/sys/windows.rs b/src/sys/windows.rs index eea16599..d9d13c10 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -32,14 +32,11 @@ use winapi::um::processthreadsapi::GetCurrentProcessId; #[cfg(feature = "all")] use winapi::um::winbase; use winapi::um::winbase::INFINITE; -use winapi::um::winsock2::{self as sock, u_long}; +use winapi::um::winsock2::{self as sock, u_long, SD_BOTH, SD_RECEIVE, SD_SEND}; use crate::{RecvFlags, SockAddr, Type}; const MSG_PEEK: c_int = 0x2; -const SD_BOTH: c_int = 2; -const SD_RECEIVE: c_int = 0; -const SD_SEND: c_int = 1; const SIO_KEEPALIVE_VALS: DWORD = 0x98000004; pub use winapi::ctypes::c_int; @@ -266,6 +263,15 @@ pub(crate) fn set_nonblocking(socket: SysSocket, nonblocking: bool) -> io::Resul ioctlsocket(socket, sock::FIONBIO, &mut nonblocking) } +pub(crate) fn shutdown(socket: SysSocket, how: Shutdown) -> io::Result<()> { + let how = match how { + Shutdown::Write => SD_SEND, + Shutdown::Read => SD_RECEIVE, + Shutdown::Both => SD_BOTH, + }; + syscall!(shutdown(socket, how), PartialEq::eq, sock::SOCKET_ERROR).map(|_| ()) +} + /// Caller must ensure `T` is the correct type for `opt` and `val`. unsafe fn getsockopt(socket: SysSocket, opt: c_int, val: c_int) -> io::Result { let mut payload: MaybeUninit = MaybeUninit::uninit(); @@ -315,19 +321,6 @@ pub struct Socket { } impl Socket { - pub fn shutdown(&self, how: Shutdown) -> io::Result<()> { - let how = match how { - Shutdown::Write => SD_SEND, - Shutdown::Read => SD_RECEIVE, - Shutdown::Both => SD_BOTH, - }; - if unsafe { sock::shutdown(self.socket, how) == 0 } { - Ok(()) - } else { - Err(last_error()) - } - } - pub fn recv(&self, buf: &mut [u8], flags: c_int) -> io::Result { unsafe { let n = { From 27f93eb7f0681cb257506c3032ce8cac58b4064e Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 11:47:09 +0100 Subject: [PATCH 11/20] Remove web-programming from Cargo's categories --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5db3590f..a589533d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ Utilities for handling networking sockets with a maximal amount of configuration possible intended. """ keywords = ["io", "socket", "network"] -categories = ["api-bindings", "network-programming", "web-programming"] +categories = ["api-bindings", "network-programming"] edition = "2018" include = [ "Cargo.toml", From ea9d7308219835aba96f508a74060241d2a7b63a Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 12:39:51 +0100 Subject: [PATCH 12/20] Rename Type::non_blocking to nonblocking To match Socket::set_nonblocking. Also add a test for it. --- src/sys/unix.rs | 2 +- tests/socket.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/sys/unix.rs b/src/sys/unix.rs index ea6e76ea..2e4e79fa 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -145,7 +145,7 @@ impl Type { target_os = "openbsd" ) ))] - pub const fn non_blocking(self) -> Type { + pub const fn nonblocking(self) -> Type { Type(self.0 | libc::SOCK_NONBLOCK) } diff --git a/tests/socket.rs b/tests/socket.rs index 8810bacc..1ff5e5ba 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -15,6 +15,24 @@ fn set_nonblocking() { assert_non_blocking(&socket, false); } +#[cfg(all( + feature = "all", + any( + target_os = "android", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd", + target_os = "openbsd" + ) +))] +#[test] +fn type_nonblocking() { + let ty = Type::Stream.nonblocking(); + let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); + assert_non_blocking(&socket, true); +} + /// Assert that `NONBLOCK` is set on `socket`. #[cfg(unix)] #[track_caller] From 282b6b81c58fa0f612cd891a69cb604b928393c4 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 12:41:00 +0100 Subject: [PATCH 13/20] Add note about atomicity of setting options on Socket --- src/socket.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/socket.rs b/src/socket.rs index efaed726..58b4e35c 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -41,6 +41,14 @@ use crate::{Domain, Protocol, SockAddr, Type}; /// [`TcpStream`]: std::net::TcpStream /// [`UdpSocket`]: std::net::UdpSocket /// +/// # Notes +/// +/// Some methods that set options on `Socket` require two system calls to set +/// there options without overwriting previously set options. We do this by +/// first getting the current settings, applying the desired changes and than +/// updating the settings. This means that the operation is **not** atomic. This +/// can lead to a data race when two threads are changing options in parallel. +/// /// # Examples /// /// Creating a new socket setting all advisable flags. @@ -234,7 +242,7 @@ impl Socket { /// /// On Windows this can **not** be used function cannot be used on a /// QOS-enabled socket, see - /// https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsaduplicatesocketw. + /// . pub fn try_clone(&self) -> io::Result { sys::try_clone(self.inner).map(|inner| Socket { inner }) } From c346cf636ce660c8f19dc6145540493e8cd0149f Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 12:43:19 +0100 Subject: [PATCH 14/20] Rename assert_nonblocking test functions --- tests/socket.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/socket.rs b/tests/socket.rs index 1ff5e5ba..07bfc766 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -6,13 +6,13 @@ use socket2::{Domain, Socket, Type}; #[test] fn set_nonblocking() { let socket = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); - assert_non_blocking(&socket, false); + assert_nonblocking(&socket, false); socket.set_nonblocking(true).unwrap(); - assert_non_blocking(&socket, true); + assert_nonblocking(&socket, true); socket.set_nonblocking(false).unwrap(); - assert_non_blocking(&socket, false); + assert_nonblocking(&socket, false); } #[cfg(all( @@ -30,13 +30,13 @@ fn set_nonblocking() { fn type_nonblocking() { let ty = Type::Stream.nonblocking(); let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); - assert_non_blocking(&socket, true); + assert_nonblocking(&socket, true); } /// Assert that `NONBLOCK` is set on `socket`. #[cfg(unix)] #[track_caller] -pub fn assert_non_blocking(socket: &S, want: bool) +pub fn assert_nonblocking(socket: &S, want: bool) where S: AsRawFd, { @@ -46,6 +46,6 @@ where #[cfg(windows)] #[track_caller] -pub fn assert_non_blocking(_: &S, _: bool) { +pub fn assert_nonblocking(_: &S, _: bool) { // No way to get this information... } From 7f65390d43007f5101a091f58b4644a2f611461e Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 12:48:00 +0100 Subject: [PATCH 15/20] Change Socket::set_cloexec to accept a bool Matching the API of Socket::set_nonblocking. --- src/sys/unix.rs | 8 ++++++-- tests/socket.rs | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/sys/unix.rs b/src/sys/unix.rs index 2e4e79fa..ecde634d 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -421,8 +421,12 @@ impl crate::Socket { /// # Notes /// /// On supported platforms you can use [`Protocol::cloexec`]. - pub fn set_cloexec(&self) -> io::Result<()> { - fcntl_add(self.inner, libc::F_GETFD, libc::F_SETFD, libc::FD_CLOEXEC) + pub fn set_cloexec(&self, close_on_exec: bool) -> io::Result<()> { + if close_on_exec { + fcntl_add(self.inner, libc::F_GETFD, libc::F_SETFD, libc::FD_CLOEXEC) + } else { + fcntl_remove(self.inner, libc::F_GETFD, libc::F_SETFD, libc::FD_CLOEXEC) + } } /// Sets `SO_NOSIGPIPE` on the socket. diff --git a/tests/socket.rs b/tests/socket.rs index 07bfc766..1bdbe943 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -49,3 +49,26 @@ where pub fn assert_nonblocking(_: &S, _: bool) { // No way to get this information... } + +#[cfg(unix)] +#[test] +fn set_cloexec() { + let socket = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); + assert_close_on_exec(&socket, false); + + socket.set_cloexec(true).unwrap(); + assert_close_on_exec(&socket, true); + + socket.set_cloexec(false).unwrap(); + assert_close_on_exec(&socket, false); +} + +/// Assert that `CLOEXEC` is set on `socket`. +#[cfg(unix)] +pub fn assert_close_on_exec(socket: &S, want: bool) +where + S: AsRawFd, +{ + let flags = unsafe { libc::fcntl(socket.as_raw_fd(), libc::F_GETFD) }; + assert_eq!(flags & libc::FD_CLOEXEC != 0, want, "CLOEXEC option"); +} From 05a3a82f820168420f6c21d6e9cc305f06fdd349 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 12:50:09 +0100 Subject: [PATCH 16/20] Add test for Type::cloexec --- tests/socket.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/socket.rs b/tests/socket.rs index 1bdbe943..72a96929 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -63,6 +63,24 @@ fn set_cloexec() { assert_close_on_exec(&socket, false); } +#[cfg(all( + feature = "all", + any( + target_os = "android", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd", + target_os = "openbsd" + ) +))] +#[test] +fn type_cloexec() { + let ty = Type::Stream.cloexec(); + let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); + assert_close_on_exec(&socket, true); +} + /// Assert that `CLOEXEC` is set on `socket`. #[cfg(unix)] pub fn assert_close_on_exec(socket: &S, want: bool) From 878e6b4093921cb1b6bb4ed99a6aa3c01a73867b Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 13:09:20 +0100 Subject: [PATCH 17/20] Change Socket::set_no_inherit signature To match Socket::set_cloexec. Also add a test for it and Type::no_inherit. --- src/sys/windows.rs | 17 +++++++------- tests/socket.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/sys/windows.rs b/src/sys/windows.rs index d9d13c10..fa109be1 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -22,14 +22,11 @@ use winapi::ctypes::{c_char, c_long, c_ulong}; use winapi::shared::in6addr::*; use winapi::shared::inaddr::*; use winapi::shared::minwindef::DWORD; -#[cfg(feature = "all")] use winapi::shared::ntdef::HANDLE; use winapi::shared::ws2def::{self, *}; use winapi::shared::ws2ipdef::*; -#[cfg(feature = "all")] use winapi::um::handleapi::SetHandleInformation; use winapi::um::processthreadsapi::GetCurrentProcessId; -#[cfg(feature = "all")] use winapi::um::winbase; use winapi::um::winbase::INFINITE; use winapi::um::winsock2::{self as sock, u_long, SD_BOTH, SD_RECEIVE, SD_SEND}; @@ -91,7 +88,7 @@ impl_debug!( impl Type { /// Our custom flag to set `WSA_FLAG_NO_HANDLE_INHERIT` on socket creation. /// Trying to mimic `Type::cloexec` on windows. - const NO_INHERIT: c_int = 1 << (size_of::()); + const NO_INHERIT: c_int = 1 << ((size_of::() * 8) - 1); // Last bit. /// Set `WSA_FLAG_NO_HANDLE_INHERIT` on the socket. #[cfg(feature = "all")] @@ -300,12 +297,16 @@ fn ioctlsocket(socket: SysSocket, cmd: c_long, payload: &mut u_long) -> io::Resu /// Windows only API. impl crate::Socket { /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`. - #[cfg(feature = "all")] - pub fn set_no_inherit(&self) -> io::Result<()> { + pub fn set_no_inherit(&self, no_inherit: bool) -> io::Result<()> { // NOTE: can't use `syscall!` because it expects the function in the // `sock::` path. - let res = - unsafe { SetHandleInformation(self.inner as HANDLE, winbase::HANDLE_FLAG_INHERIT, 0) }; + let res = unsafe { + SetHandleInformation( + self.inner as HANDLE, + winbase::HANDLE_FLAG_INHERIT, + !no_inherit as _, + ) + }; if res == 0 { // Zero means error. Err(io::Error::last_os_error()) diff --git a/tests/socket.rs b/tests/socket.rs index 72a96929..598e86fd 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -1,5 +1,16 @@ +#[cfg(windows)] +use std::io; #[cfg(unix)] use std::os::unix::io::AsRawFd; +#[cfg(windows)] +use std::os::windows::io::AsRawSocket; + +#[cfg(windows)] +use winapi::shared::minwindef::DWORD; +#[cfg(windows)] +use winapi::um::handleapi::GetHandleInformation; +#[cfg(windows)] +use winapi::um::winbase::HANDLE_FLAG_INHERIT; use socket2::{Domain, Socket, Type}; @@ -28,7 +39,7 @@ fn set_nonblocking() { ))] #[test] fn type_nonblocking() { - let ty = Type::Stream.nonblocking(); + let ty = Type::STREAM.nonblocking(); let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); assert_nonblocking(&socket, true); } @@ -76,13 +87,14 @@ fn set_cloexec() { ))] #[test] fn type_cloexec() { - let ty = Type::Stream.cloexec(); + let ty = Type::STREAM.cloexec(); let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); assert_close_on_exec(&socket, true); } /// Assert that `CLOEXEC` is set on `socket`. #[cfg(unix)] +#[track_caller] pub fn assert_close_on_exec(socket: &S, want: bool) where S: AsRawFd, @@ -90,3 +102,43 @@ where let flags = unsafe { libc::fcntl(socket.as_raw_fd(), libc::F_GETFD) }; assert_eq!(flags & libc::FD_CLOEXEC != 0, want, "CLOEXEC option"); } + +#[cfg(windows)] +#[test] +fn set_no_inherit() { + let socket = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); + assert_flag_inherit(&socket, false); + + socket.set_no_inherit(true).unwrap(); + assert_flag_inherit(&socket, true); + + socket.set_no_inherit(false).unwrap(); + assert_flag_inherit(&socket, false); +} + +#[cfg(all(feature = "all", windows))] +#[test] +fn type_no_inherit() { + let ty = Type::STREAM.no_inherit(); + let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); + assert_flag_inherit(&socket, true); +} + +/// Assert that `FLAG_INHERIT` is set on `socket`. +#[cfg(windows)] +#[track_caller] +pub fn assert_flag_inherit(socket: &S, want: bool) +where + S: AsRawSocket, +{ + let mut flags: DWORD = 0; + if unsafe { GetHandleInformation(socket.as_raw_socket() as _, &mut flags) } == 0 { + let err = io::Error::last_os_error(); + panic!("unexpected error: {}", err); + } + assert_eq!( + flags & HANDLE_FLAG_INHERIT != 0, + want, + "FLAG_INHERIT option" + ); +} From 77969dccf90b54e14ef9d4a69631dc9c98b087fc Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 13:12:47 +0100 Subject: [PATCH 18/20] Fix Socket example --- src/socket.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/socket.rs b/src/socket.rs index 58b4e35c..c13ee032 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -74,7 +74,7 @@ use crate::{Domain, Protocol, SockAddr, Type}; /// /// // On platforms that don't support `SOCK_CLOEXEC`, use `FD_CLOEXEC`. /// #[cfg(all(not(windows), not(any(target_os = "android", target_os = "dragonfly", target_os = "freebsd", target_os = "linux", target_os = "netbsd", target_os = "openbsd"))))] -/// socket.set_cloexec()?; +/// socket.set_cloexec(true)?; /// /// // On macOS and iOS set `NOSIGPIPE`. /// #[cfg(target_vendor = "apple")] From 75aa2ecd72b476a2a82fad2e23efd0827101faff Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 13:19:37 +0100 Subject: [PATCH 19/20] Fix set_no_inherit test --- tests/socket.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/socket.rs b/tests/socket.rs index 598e86fd..df878e68 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -107,13 +107,13 @@ where #[test] fn set_no_inherit() { let socket = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); - assert_flag_inherit(&socket, false); + assert_flag_no_inherit(&socket, false); socket.set_no_inherit(true).unwrap(); - assert_flag_inherit(&socket, true); + assert_flag_no_inherit(&socket, true); socket.set_no_inherit(false).unwrap(); - assert_flag_inherit(&socket, false); + assert_flag_no_inherit(&socket, false); } #[cfg(all(feature = "all", windows))] @@ -121,13 +121,13 @@ fn set_no_inherit() { fn type_no_inherit() { let ty = Type::STREAM.no_inherit(); let socket = Socket::new(Domain::IPV4, ty, None).unwrap(); - assert_flag_inherit(&socket, true); + assert_flag_no_inherit(&socket, true); } -/// Assert that `FLAG_INHERIT` is set on `socket`. +/// Assert that `FLAG_INHERIT` is not set on `socket`. #[cfg(windows)] #[track_caller] -pub fn assert_flag_inherit(socket: &S, want: bool) +pub fn assert_flag_no_inherit(socket: &S, want: bool) where S: AsRawSocket, { @@ -138,7 +138,7 @@ where } assert_eq!( flags & HANDLE_FLAG_INHERIT != 0, - want, + !want, "FLAG_INHERIT option" ); } From adb14245fd4dc0f6486f4064b470401f433d15a8 Mon Sep 17 00:00:00 2001 From: Thomas de Zeeuw Date: Wed, 11 Nov 2020 13:32:51 +0100 Subject: [PATCH 20/20] Change Socket::set_nosigpipe to accept a bool Same as Socket::set_nonblocking. Also add test for it. --- src/socket.rs | 2 +- src/sys/unix.rs | 31 ++++++++++++++++++++++--------- tests/socket.rs | 42 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/socket.rs b/src/socket.rs index c13ee032..5c47b3d8 100644 --- a/src/socket.rs +++ b/src/socket.rs @@ -78,7 +78,7 @@ use crate::{Domain, Protocol, SockAddr, Type}; /// /// // On macOS and iOS set `NOSIGPIPE`. /// #[cfg(target_vendor = "apple")] -/// socket.set_nosigpipe()?; +/// socket.set_nosigpipe(true)?; /// /// # drop(socket); /// # Ok(()) diff --git a/src/sys/unix.rs b/src/sys/unix.rs index ecde634d..2b02309c 100644 --- a/src/sys/unix.rs +++ b/src/sys/unix.rs @@ -9,7 +9,7 @@ #[cfg(not(target_os = "redox"))] use std::io::{IoSlice, IoSliceMut}; use std::io::{Read, Write}; -use std::mem::{self, size_of_val, MaybeUninit}; +use std::mem::{self, size_of, size_of_val, MaybeUninit}; use std::net::Shutdown; use std::net::{self, Ipv4Addr, Ipv6Addr}; #[cfg(feature = "all")] @@ -435,8 +435,15 @@ impl crate::Socket { /// /// Only supported on Apple platforms (`target_vendor = "apple"`). #[cfg(all(feature = "all", target_vendor = "apple"))] - pub fn set_nosigpipe(&self) -> io::Result<()> { - unsafe { setsockopt(self.inner, libc::SOL_SOCKET, libc::SO_NOSIGPIPE, 1i32) } + pub fn set_nosigpipe(&self, nosigpipe: bool) -> io::Result<()> { + unsafe { + setsockopt::( + self.inner, + libc::SOL_SOCKET, + libc::SO_NOSIGPIPE, + nosigpipe as _, + ) + } } } @@ -467,7 +474,7 @@ fn fcntl_remove(fd: SysSocket, get_cmd: c_int, set_cmd: c_int, flag: c_int) -> i /// Caller must ensure `T` is the correct type for `opt` and `val`. unsafe fn getsockopt(fd: SysSocket, opt: c_int, val: c_int) -> io::Result { let mut payload: MaybeUninit = MaybeUninit::uninit(); - let mut len = mem::size_of::() as libc::socklen_t; + let mut len = size_of::() as libc::socklen_t; syscall!(getsockopt( fd, opt, @@ -476,7 +483,7 @@ unsafe fn getsockopt(fd: SysSocket, opt: c_int, val: c_int) -> io::Result &mut len, )) .map(|_| { - debug_assert_eq!(len as usize, mem::size_of::()); + debug_assert_eq!(len as usize, size_of::()); // Safety: `getsockopt` initialised `payload` for us. payload.assume_init() }) @@ -484,10 +491,7 @@ unsafe fn getsockopt(fd: SysSocket, opt: c_int, val: c_int) -> io::Result /// Caller must ensure `T` is the correct type for `opt` and `val`. #[cfg(all(feature = "all", target_vendor = "apple"))] -unsafe fn setsockopt(fd: SysSocket, opt: c_int, val: c_int, payload: T) -> io::Result<()> -where - T: Copy, -{ +unsafe fn setsockopt(fd: SysSocket, opt: c_int, val: c_int, payload: T) -> io::Result<()> { let payload = &payload as *const T as *const c_void; syscall!(setsockopt( fd, @@ -499,6 +503,15 @@ where .map(|_| ()) } +/* + setsockopt::( + self.inner, + libc::SOL_SOCKET, + libc::SO_NOSIGPIPE, + nosigpipe as _, + ) +*/ + #[repr(transparent)] // Required during rewriting. pub struct Socket { fd: SysSocket, diff --git a/tests/socket.rs b/tests/socket.rs index df878e68..5b10a8ea 100644 --- a/tests/socket.rs +++ b/tests/socket.rs @@ -1,4 +1,4 @@ -#[cfg(windows)] +#[cfg(any(windows, all(feature = "all", target_vendor = "apple")))] use std::io; #[cfg(unix)] use std::os::unix::io::AsRawFd; @@ -142,3 +142,43 @@ where "FLAG_INHERIT option" ); } + +#[cfg(all(feature = "all", target_vendor = "apple"))] +#[test] +fn set_nosigpipe() { + let socket = Socket::new(Domain::IPV4, Type::STREAM, None).unwrap(); + assert_flag_no_sigpipe(&socket, false); + + socket.set_nosigpipe(true).unwrap(); + assert_flag_no_sigpipe(&socket, true); + + socket.set_nosigpipe(false).unwrap(); + assert_flag_no_sigpipe(&socket, false); +} + +/// Assert that `SO_NOSIGPIPE` is set on `socket`. +#[cfg(all(feature = "all", target_vendor = "apple"))] +#[track_caller] +pub fn assert_flag_no_sigpipe(socket: &S, want: bool) +where + S: AsRawFd, +{ + use std::mem::size_of; + let mut flags: libc::c_int = 0; + let mut length = size_of::() as libc::socklen_t; + let res = unsafe { + libc::getsockopt( + socket.as_raw_fd(), + libc::SOL_SOCKET, + libc::SO_NOSIGPIPE, + &mut flags as *mut _ as *mut _, + &mut length, + ) + }; + if res != 0 { + let err = io::Error::last_os_error(); + panic!("unexpected error: {}", err); + } + assert_eq!(length as usize, size_of::()); + assert_eq!(flags, want as _, "non-blocking option"); +}