Exclude NUL characters from OSStrings returned by getsockopt (#2557)

* Exclude NUL characters from OSStrings returned by getsockopt

On FreeBSD, getsockopt appends a single NUL to the returned string.  On
Linux, it pads the entire buffer with NULs.  But both of those behaviors
may be version-dependent.  Adjust GetOsString to handle any number of
NUL characters.

* fixup: fix MSRV breakage

* Fixup: expand the comment

Author: asomers

changelog/2557.fixed.md

@@ -0,0 +1 @@
+Properly exclude NUL characters from `OSString`s returned by `getsockopt`.

src/sys/socket/sockopt.rs

@@ -7,8 +7,8 @@ use crate::{errno::Errno, Result};
 use cfg_if::cfg_if;
 use libc::{self, c_int, c_void, socklen_t};
 #[cfg(apple_targets)]
-use std::ffi::{CStr, CString};
-use std::ffi::{OsStr, OsString};
+use std::ffi::CString;
+use std::ffi::{CStr, OsStr, OsString};
 use std::mem::{self, MaybeUninit};
 use std::os::unix::ffi::OsStrExt;
 #[cfg(linux_android)]
@@ -1745,7 +1745,15 @@ impl<T: AsMut<[u8]>> Get<OsString> for GetOsString<T> {
     unsafe fn assume_init(self) -> OsString {
         let len = self.len as usize;
         let mut v = unsafe { self.val.assume_init() };
-        OsStr::from_bytes(&v.as_mut()[0..len]).to_owned()
+        if let Ok(cs) = CStr::from_bytes_until_nul(&v.as_mut()[0..len]) {
+            // It's legal for the kernel to return any number of NULs at the
+            // end of the string.  C applications don't care, after all.
+            OsStr::from_bytes(cs.to_bytes())
+        } else {
+            // Even zero NULs is possible.
+            OsStr::from_bytes(&v.as_mut()[0..len])
+        }
+        .to_owned()
     }
 }
 

test/sys/test_sockopt.rs

@@ -249,16 +249,16 @@ fn test_so_type_unknown() {
 }
 
 // The CI doesn't supported getsockopt and setsockopt on emulated processors.
-// It's believed that a QEMU issue, the tests run ok on a fully emulated system.
-// Current CI just run the binary with QEMU but the Kernel remains the same as the host.
+// It's believed to be a QEMU issue; the tests run ok on a fully emulated
+// system.  Current CI just runs the binary with QEMU but the kernel remains the
+// same as the host.
 // So the syscall doesn't work properly unless the kernel is also emulated.
 #[test]
-#[cfg(all(
-    any(target_arch = "x86", target_arch = "x86_64"),
-    any(target_os = "freebsd", target_os = "linux")
-))]
+#[cfg(any(target_os = "freebsd", target_os = "linux"))]
+#[cfg_attr(qemu, ignore)]
 fn test_tcp_congestion() {
     use std::ffi::OsString;
+    use std::os::unix::ffi::OsStrExt;
 
     let fd = socket(
         AddressFamily::Inet,
@@ -269,6 +269,14 @@ fn test_tcp_congestion() {
     .unwrap();
 
     let val = getsockopt(&fd, sockopt::TcpCongestion).unwrap();
+    let bytes = val.as_os_str().as_bytes();
+    for b in bytes.iter() {
+        assert_ne!(
+            *b, 0,
+            "OsString should contain no embedded NULs: {:?}",
+            val
+        );
+    }
     setsockopt(&fd, sockopt::TcpCongestion, &val).unwrap();
 
     setsockopt(