feat(firecracker,unix): use OsString to allow abstract names

Author: sivizius

Cargo.toml

@@ -4,7 +4,7 @@ version = "0.6.0"
 edition = "2021"
 license = "MIT"
 description = "A Hyper client library allowing access to Unix, VSock and Firecracker sockets"
-rust-version = "1.63"
+rust-version = "1.74"
 repository = "https://github.com/rust-firecracker/hyper-client-sockets"
 readme = "README.md"
 keywords = ["hyper", "client", "sockets"]

src/async_io/vsock.rs

@@ -50,7 +50,7 @@ impl AsyncVsockIo {
                 // TODO: Once https://github.com/rust-lang/rust/issues/63569 is stable, use `assume_init_mut`:
                 let buffer = unsafe { &mut *(cursor.as_mut() as *mut [MaybeUninit<u8>] as *mut [u8]) };
                 let amount = self.0.get_ref().read(buffer);
-                try_advance_cursor(cursor, self.0.get_ref().read(buffer))
+                try_advance_cursor(cursor, amount)
             }
             other => Some(other),
         }

src/uri/firecracker.rs

@@ -1,5 +1,7 @@
 use std::{
+    ffi::OsString,
     io::Result as IoResult,
+    os::unix::ffi::OsStringExt as _,
     path::{Path, PathBuf},
 };
 
@@ -27,26 +29,33 @@ impl FirecrackerUri for Uri {
         guest_port: u32,
         url: impl AsRef<str>,
     ) -> Result<Uri, InvalidUri> {
-        let host = host_socket_path.as_ref().to_string_lossy().to_string();
-        let authority = encode(format!("{host}:{guest_port}"));
+        let host = encode(host_socket_path.as_ref().as_os_str().as_encoded_bytes());
+        let guest_port = encode(guest_port.to_string());
+        let authority = format!("{host}{:02x}{guest_port}", b':');
         let path_and_query = url.as_ref().trim_start_matches('/');
         let uri_str = format!("fc://{authority}/{path_and_query}");
         uri_str.parse()
     }
 
     fn parse_firecracker(&self) -> IoResult<(PathBuf, u32)> {
         if self.scheme_str() == Some("fc") {
-            let host = self.host().ok_or_else(|| io_input_err("URI host must be present"))?;
-            let hex_decoded = Vec::from_hex(host).map_err(|_| io_input_err("URI host must be hex"))?;
-            let full_str = String::from_utf8_lossy(&hex_decoded).into_owned();
-            let splits = full_str
-                .split_once(':')
-                .ok_or_else(|| io_input_err("URI host could not be split in halves with a ."))?;
-            let host_socket_path = PathBuf::from(splits.0);
-            let guest_port = splits
+            let host_hex = self.host().ok_or_else(|| io_input_err("URI host must be present"))?;
+            let mut host_octets =
+                Vec::from_hex(host_hex).map_err(|_| io_input_err("URI host must be hexadecimal encoded"))?;
+
+            let colon_pos = host_octets
+                .iter()
+                .rposition(|octet| *octet == b':')
+                .ok_or_else(|| io_input_err("URI host does not encode port"))?;
+
+            let guest_port = String::from_utf8(host_octets.split_off(colon_pos))
+                .map_err(|_| io_input_err("URI guest port is not valid UTF8"))?
+                .split_at(1)
                 .1
                 .parse::<u32>()
-                .map_err(|_| io_input_err("URI guest port could not converted to u32"))?;
+                .map_err(|_| io_input_err("URI guest port could not be parsed"))?;
+
+            let host_socket_path = OsString::from_vec(host_octets).into();
 
             Ok((host_socket_path, guest_port))
         } else {

src/uri/tests.rs

@@ -6,7 +6,7 @@ use vsock::VsockAddr;
 use crate::uri::{FirecrackerUri, UnixUri, VsockUri};
 
 #[test]
-fn unix_uri_should_be_constructed_correctly() {
+fn unix_uri_with_pathname_should_be_constructed_correctly() {
     let uri_str = format!("unix://{}/route", hex::encode("/tmp/socket.sock"));
     assert_eq!(
         Uri::unix("/tmp/socket.sock", "/route").unwrap(),
@@ -15,14 +15,32 @@ fn unix_uri_should_be_constructed_correctly() {
 }
 
 #[test]
-fn unix_uri_should_be_deconstructed_correctly() {
+fn unix_uri_with_abstract_name_should_be_constructed_correctly() {
+    let uri_str = format!("unix://00{}/route", hex::encode("/tmp/socket.sock"));
+    assert_eq!(
+        Uri::unix("\0/tmp/socket.sock", "/route").unwrap(),
+        uri_str.parse::<Uri>().unwrap()
+    );
+}
+
+#[test]
+fn unix_uri_with_pathname_should_be_deconstructed_correctly() {
     let uri = format!("unix://{}/route", hex::encode("/tmp/socket.sock"));
     assert_eq!(
         uri.parse::<Uri>().unwrap().parse_unix().unwrap(),
         PathBuf::from("/tmp/socket.sock")
     );
 }
 
+#[test]
+fn unix_uri_with_abstract_name_should_be_deconstructed_correctly() {
+    let uri = format!("unix://00{}/route", hex::encode("/tmp/socket.sock"));
+    assert_eq!(
+        uri.parse::<Uri>().unwrap().parse_unix().unwrap(),
+        PathBuf::from("\0/tmp/socket.sock")
+    );
+}
+
 #[test]
 fn vsock_uri_should_be_constructed_correctly() {
     let uri = format!("vsock://{}/route", hex::encode("10.20"));
@@ -41,7 +59,7 @@ fn vsock_uri_should_be_deconstructed_correctly() {
 }
 
 #[test]
-fn firecracker_uri_should_be_constructed_correctly() {
+fn firecracker_uri_with_pathname_should_be_constructed_correctly() {
     let uri_str = format!("fc://{}/route", hex::encode("/tmp/socket.sock:1000"));
     assert_eq!(
         Uri::firecracker("/tmp/socket.sock", 1000, "/route").unwrap(),
@@ -50,11 +68,30 @@ fn firecracker_uri_should_be_constructed_correctly() {
 }
 
 #[test]
-fn firecracker_uri_should_be_deconstructed_correctly() {
+fn firecracker_uri_with_abstract_name_should_be_constructed_correctly() {
+    let uri_str = format!("fc://00{}/route", hex::encode("/tmp/socket.sock:1000"));
+    assert_eq!(
+        Uri::firecracker("\0/tmp/socket.sock", 1000, "/route").unwrap(),
+        uri_str.parse::<Uri>().unwrap()
+    );
+}
+
+#[test]
+fn firecracker_uri_with_pathname_should_be_deconstructed_correctly() {
     let uri = format!("fc://{}/route", hex::encode("/tmp/socket.sock:1000"))
         .parse::<Uri>()
         .unwrap();
     let (socket_path, port) = uri.parse_firecracker().unwrap();
     assert_eq!(socket_path, PathBuf::from("/tmp/socket.sock"));
     assert_eq!(port, 1000);
 }
+
+#[test]
+fn firecracker_uri_with_abstract_name_should_be_deconstructed_correctly() {
+    let uri = format!("fc://00{}/route", hex::encode("/tmp/socket.sock:1000"))
+        .parse::<Uri>()
+        .unwrap();
+    let (socket_path, port) = uri.parse_firecracker().unwrap();
+    assert_eq!(socket_path, PathBuf::from("\0/tmp/socket.sock"));
+    assert_eq!(port, 1000);
+}

src/uri/unix.rs

@@ -1,5 +1,7 @@
 use std::{
+    ffi::OsString,
     io::Result as IoResult,
+    os::unix::ffi::OsStringExt as _,
     path::{Path, PathBuf},
 };
 
@@ -19,7 +21,7 @@ pub trait UnixUri {
 
 impl UnixUri for Uri {
     fn unix(socket_path: impl AsRef<Path>, url: impl AsRef<str>) -> Result<Uri, InvalidUri> {
-        let authority = encode(socket_path.as_ref().to_string_lossy().to_string());
+        let authority = encode(socket_path.as_ref().as_os_str().as_encoded_bytes());
         let path_and_query = url.as_ref().trim_start_matches('/');
         let uri_str = format!("unix://{authority}/{path_and_query}");
         uri_str.parse()
@@ -29,8 +31,8 @@ impl UnixUri for Uri {
         if self.scheme_str() == Some("unix") {
             match self.host() {
                 Some(host) => {
-                    let bytes = Vec::from_hex(host).map_err(|_| io_input_err("URI host must be hex"))?;
-                    Ok(PathBuf::from(String::from_utf8_lossy(&bytes).into_owned()))
+                    let octets = Vec::from_hex(host).map_err(|_| io_input_err("URI host must be hexadecimal"))?;
+                    Ok(OsString::from_vec(octets).into())
                 }
                 None => Err(io_input_err("URI host must be present")),
             }

tests/async_io.rs

@@ -2,7 +2,7 @@ use std::{future::Future, sync::Arc};
 
 use async_executor::Executor;
 use bytes::Bytes;
-use common::{check_response, serve_firecracker, serve_unix, serve_vsock};
+use common::{check_response, serve_firecracker, serve_unix, serve_unix_abstract, serve_vsock};
 use http::{Request, Uri};
 use http_body_util::Full;
 use hyper::client::conn::http1::handshake;
@@ -18,7 +18,7 @@ use smol_hyper::rt::SmolExecutor;
 mod common;
 
 #[test]
-fn async_io_unix_raw_connectivity() {
+fn async_io_unix_raw_connectivity_with_pathname() {
     run(|executor| async move {
         let socket_path = serve_unix();
         let io = AsyncIoBackend::connect_to_unix_socket(&socket_path).await.unwrap();
@@ -32,6 +32,21 @@ fn async_io_unix_raw_connectivity() {
     });
 }
 
+#[test]
+fn async_io_unix_raw_connectivity_with_abstract_name() {
+    run(|executor| async move {
+        let socket_path = serve_unix_abstract();
+        let io = AsyncIoBackend::connect_to_unix_socket(&socket_path).await.unwrap();
+        let (mut send_request, conn) = handshake::<_, Full<Bytes>>(io).await.unwrap();
+        executor.spawn(conn).detach();
+        let response = send_request
+            .send_request(Request::new(Full::new(Bytes::new())))
+            .await
+            .unwrap();
+        check_response(response).await;
+    });
+}
+
 #[test]
 fn async_io_unix_pooled_connectivity() {
     run(|executor| async move {

tests/common.rs

@@ -37,6 +37,29 @@ pub fn serve_unix() -> PathBuf {
     socket_path
 }
 
+#[allow(unused)]
+pub fn serve_unix_abstract() -> PathBuf {
+    let socket_path = PathBuf::from("\0test").with_extension(Uuid::new_v4().to_string());
+
+    let cloned_socket_path = socket_path.clone();
+    in_tokio_thread(async move {
+        let listener = UnixListener::bind(cloned_socket_path).unwrap();
+
+        loop {
+            let (stream, _) = listener.accept().await.unwrap();
+            tokio::spawn(async move {
+                http1::Builder::new()
+                    .serve_connection(TokioIo::new(stream), service_fn(responder))
+                    .await
+                    .unwrap();
+            });
+        }
+    });
+
+    std::thread::sleep(Duration::from_millis(1));
+    socket_path
+}
+
 #[allow(unused)]
 pub fn serve_vsock() -> VsockAddr {
     let port = fastrand::u32(15000..=65536);

tests/tokio.rs

@@ -1,5 +1,5 @@
 use bytes::Bytes;
-use common::{check_response, serve_firecracker, serve_unix, serve_vsock};
+use common::{check_response, serve_firecracker, serve_unix, serve_unix_abstract, serve_vsock};
 use http::{Request, Uri};
 use http_body_util::Full;
 use hyper::client::conn::http1::handshake;
@@ -14,7 +14,7 @@ use hyper_util::{client::legacy::Client, rt::TokioExecutor};
 mod common;
 
 #[tokio::test]
-async fn tokio_unix_raw_connectivity() {
+async fn tokio_unix_raw_connectivity_with_pathname() {
     let socket_path = serve_unix();
     let io = TokioBackend::connect_to_unix_socket(&socket_path).await.unwrap();
     let (mut send_request, conn) = handshake::<_, Full<Bytes>>(io).await.unwrap();
@@ -26,6 +26,19 @@ async fn tokio_unix_raw_connectivity() {
     check_response(response).await;
 }
 
+#[tokio::test]
+async fn tokio_unix_raw_connectivity_with_abstract_name() {
+    let socket_path = serve_unix_abstract();
+    let io = TokioBackend::connect_to_unix_socket(&socket_path).await.unwrap();
+    let (mut send_request, conn) = handshake::<_, Full<Bytes>>(io).await.unwrap();
+    tokio::spawn(conn);
+    let response = send_request
+        .send_request(Request::new(Full::new(Bytes::new())))
+        .await
+        .unwrap();
+    check_response(response).await;
+}
+
 #[tokio::test]
 async fn tokio_unix_pooled_connectivity() {
     let socket_path = serve_unix();