From a730d86940081ad044cdfbc1285c1db6d3048392 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Thu, 20 Jan 2022 17:03:01 +0100 Subject: [PATCH] Receive multiple file descriptors with a Request Extending the existing code to support receiving more than one file descriptor per request. The micro-http crate might be used in a context where multiple file descriptors are associated with one request, hence the need to update the micro-http crate. A concrete example from Cloud Hypervisor is to be able to pass multiple TAP file descriptors at once when adding a new network interface. This way it can hotplug a multiqueue device. Signed-off-by: Sebastien Boeuf --- Cargo.lock | 6 +- coverage_config.json | 2 +- src/connection.rs | 160 ++++++++++++++++++++++++++++++++++++------- src/request.rs | 10 +-- 4 files changed, 145 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 18450e6..00f3cf2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "bitflags" version = "1.2.1" @@ -22,9 +24,9 @@ dependencies = [ [[package]] name = "vmm-sys-util" -version = "0.8.0" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01cf11afbc4ebc0d5c7a7748a77d19e2042677fc15faa2f4ccccb27c18a60605" +checksum = "733537bded03aaa93543f785ae997727b30d1d9f4a03b7861d23290474242e11" dependencies = [ "bitflags", "libc", diff --git a/coverage_config.json b/coverage_config.json index ade818e..4f21e4e 100644 --- a/coverage_config.json +++ b/coverage_config.json @@ -1 +1 @@ -{"coverage_score": 93.1, "exclude_path": "", "crate_features": ""} +{"coverage_score": 93.2, "exclude_path": "", "crate_features": ""} diff --git a/src/connection.rs b/src/connection.rs index 99b3b24..a7386f6 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -4,6 +4,7 @@ use std::collections::VecDeque; use std::fs::File; use std::io::{Read, Write}; +use std::os::unix::io::FromRawFd; use crate::common::ascii::{CR, CRLF_LEN, LF}; use crate::common::Body; @@ -15,6 +16,7 @@ use crate::server::MAX_PAYLOAD_SIZE; use vmm_sys_util::sock_ctrl_msg::ScmSocket; const BUFFER_SIZE: usize = 1024; +const SCM_MAX_FD: usize = 253; /// Describes the state machine of an HTTP connection. enum ConnectionState { @@ -52,9 +54,9 @@ pub struct HttpConnection { /// A buffer containing the bytes of a response that is currently /// being sent. response_buffer: Option>, - /// The latest file that has been received and which must be associated + /// The list of files that has been received and which must be associated /// with the pending request. - file: Option, + files: Vec, /// Optional payload max size. payload_max_size: usize, } @@ -73,7 +75,7 @@ impl HttpConnection { parsed_requests: VecDeque::new(), response_queue: VecDeque::new(), response_buffer: None, - file: None, + files: Vec::new(), payload_max_size: MAX_PAYLOAD_SIZE, } } @@ -123,7 +125,7 @@ impl HttpConnection { self.state = ConnectionState::WaitingForRequestLine; self.body_bytes_to_be_read = 0; let mut pending_request = self.pending_request.take().unwrap(); - pending_request.file = self.file.take(); + pending_request.files = self.files.drain(..).collect(); self.parsed_requests.push_back(pending_request); } }; @@ -143,15 +145,11 @@ impl HttpConnection { } // Append new bytes to what we already have in the buffer. // The slice access is safe, the index is checked above. - let (bytes_read, file) = self - .stream - .recv_with_fd(&mut self.buffer[self.read_cursor..]) - .map_err(ConnectionError::StreamReadError)?; + let (bytes_read, new_files) = self.recv_with_fds()?; - // Update the internal file that must be associated with the request. - if file.is_some() { - self.file = file; - } + // Update the internal list of files that must be associated with the + // request. + self.files.extend(new_files); // If the read returned 0 then the client has closed the connection. if bytes_read == 0 { @@ -162,6 +160,43 @@ impl HttpConnection { .ok_or(ConnectionError::ParseError(RequestError::Overflow)) } + /// Receive data along with optional files descriptors. + /// It is a wrapper around the same function from vmm-sys-util. + /// + /// # Errors + /// `StreamError` is returned if any error occurred while reading the stream. + fn recv_with_fds(&mut self) -> Result<(usize, Vec), ConnectionError> { + let buf = &mut self.buffer[self.read_cursor..]; + // We must allocate the maximum number of receivable file descriptors + // if don't want to miss any of them. Allocating a too small number + // would lead to the incapacity of receiving the file descriptors. + let mut fds = [0; SCM_MAX_FD]; + let mut iovecs = [libc::iovec { + iov_base: buf.as_mut_ptr() as *mut libc::c_void, + iov_len: buf.len(), + }]; + + // Safe because we have mutably borrowed buf and it's safe to write + // arbitrary data to a slice. + let (read_count, fd_count) = unsafe { + self.stream + .recv_with_fds(&mut iovecs, &mut fds) + .map_err(ConnectionError::StreamReadError)? + }; + + Ok(( + read_count, + fds.iter() + .take(fd_count) + .map(|fd| { + // Safe because all fds are owned by us after they have been + // received through the socket. + unsafe { File::from_raw_fd(*fd) } + }) + .collect(), + )) + } + /// Parses bytes in `buffer` for a valid request line. /// Returns `false` if there are no more bytes to be parsed in the buffer. /// @@ -197,7 +232,7 @@ impl HttpConnection { .map_err(ConnectionError::ParseError)?, headers: Headers::default(), body: None, - file: None, + files: Vec::new(), }); self.state = ConnectionState::WaitingForHeaders; Ok(true) @@ -517,13 +552,17 @@ impl HttpConnection { #[cfg(test)] mod tests { + use std::io::{Seek, SeekFrom}; use std::net::Shutdown; + use std::os::unix::io::IntoRawFd; use std::os::unix::net::UnixStream; use super::*; use crate::common::{Method, Version}; use crate::server::MAX_PAYLOAD_SIZE; + use vmm_sys_util::tempfile::TempFile; + #[test] fn test_try_read_expect() { // Test request with `Expect` header. @@ -548,7 +587,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(26, true, true), body: Some(Body::new(b"this is not\n\r\na json \nbody".to_vec())), - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); @@ -585,7 +624,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(26, true, true), body: Some(Body::new(b"this is not\n\r\na json \nbody".to_vec())), - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); } @@ -619,7 +658,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(26, true, true), body: Some(Body::new(b"this is not\n\r\na json \nbody".to_vec())), - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); } @@ -684,7 +723,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(1400, true, true), body: Some(Body::new(request_body)), - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); @@ -755,7 +794,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(0, true, true), body: None, - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); } @@ -777,7 +816,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(0, false, false), body: None, - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); } @@ -806,7 +845,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(0, false, false), body: None, - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); @@ -825,7 +864,7 @@ mod tests { ), headers: Headers::new(0, false, false), body: None, - file: None, + files: Vec::new(), }; assert_eq!(request, expected_request); } @@ -853,7 +892,7 @@ mod tests { request_line: RequestLine::new(Method::Patch, "http://localhost/home", Version::Http11), headers: Headers::new(26, false, true), body: Some(Body::new(b"this is not\n\r\na json \nbody".to_vec())), - file: None, + files: Vec::new(), }; conn.try_read().unwrap(); @@ -864,7 +903,7 @@ mod tests { request_line: RequestLine::new(Method::Put, "http://farhost/away", Version::Http11), headers: Headers::new(23, false, false), body: Some(Body::new(b"this is another request".to_vec())), - file: None, + files: Vec::new(), }; assert_eq!(request_first, expected_request_first); assert_eq!(request_second, expected_request_second); @@ -999,6 +1038,77 @@ mod tests { ); } + #[test] + fn test_read_bytes_with_files() { + let (sender, receiver) = UnixStream::pair().unwrap(); + receiver.set_nonblocking(true).expect("Can't modify socket"); + let mut conn = HttpConnection::new(receiver); + + // Create 3 files, edit the content and rewind back to the start. + let mut file1 = TempFile::new().unwrap().into_file(); + let mut file2 = TempFile::new().unwrap().into_file(); + let mut file3 = TempFile::new().unwrap().into_file(); + file1.write(b"foo").unwrap(); + file1.seek(SeekFrom::Start(0)).unwrap(); + file2.write(b"bar").unwrap(); + file2.seek(SeekFrom::Start(0)).unwrap(); + file3.write(b"foobar").unwrap(); + file3.seek(SeekFrom::Start(0)).unwrap(); + + // Send 2 file descriptors along with 3 bytes of data. + assert_eq!( + sender.send_with_fds( + &[[1, 2, 3].as_ref()], + &[file1.into_raw_fd(), file2.into_raw_fd()] + ), + Ok(3) + ); + + // Check we receive the right amount of data along with the right + // amount of file descriptors. + assert_eq!(conn.read_bytes(), Ok(3)); + assert_eq!(conn.files.len(), 2); + + // Check the content of the data received + assert_eq!(conn.buffer[0], 1); + assert_eq!(conn.buffer[1], 2); + assert_eq!(conn.buffer[2], 3); + + // Check the file descriptors are usable by checking the content that + // can be read. + let mut buf = [0; 10]; + assert_eq!(conn.files[0].read(&mut buf).unwrap(), 3); + assert_eq!(&buf[..3], b"foo"); + assert_eq!(conn.files[1].read(&mut buf).unwrap(), 3); + assert_eq!(&buf[..3], b"bar"); + + // Send the 3rd file descriptor along with 1 byte of data. + assert_eq!( + sender.send_with_fds(&[[10].as_ref()], &[file3.into_raw_fd()]), + Ok(1) + ); + + // Check the amount of data along with the amount of file descriptors + // are updated. + assert_eq!(conn.read_bytes(), Ok(1)); + assert_eq!(conn.files.len(), 3); + + // Check the content of the new data received + assert_eq!(conn.buffer[0], 10); + + // Check the latest file descriptor is usable by checking the content + // that can be read. + let mut buf = [0; 10]; + assert_eq!(conn.files[2].read(&mut buf).unwrap(), 6); + assert_eq!(&buf[..6], b"foobar"); + + sender.shutdown(Shutdown::Write).unwrap(); + assert_eq!( + conn.read_bytes().unwrap_err(), + ConnectionError::ConnectionClosed + ); + } + #[test] fn test_shift_buffer_left() { let (_, receiver) = UnixStream::pair().unwrap(); @@ -1095,7 +1205,7 @@ mod tests { request_line: RequestLine::new(Method::Get, "http://foo/bar", Version::Http11), headers: Headers::new(0, true, true), body: None, - file: None, + files: Vec::new(), }); assert_eq!( conn.parse_headers(&mut 0, BUFFER_SIZE).unwrap_err(), @@ -1153,7 +1263,7 @@ mod tests { request_line: RequestLine::new(Method::Get, "http://foo/bar", Version::Http11), headers: Headers::new(0, true, true), body: None, - file: None, + files: Vec::new(), }); conn.body_vec = vec![0xde, 0xad, 0xbe, 0xef]; assert_eq!( diff --git a/src/request.rs b/src/request.rs index dbf10fa..146ca3f 100644 --- a/src/request.rs +++ b/src/request.rs @@ -159,8 +159,8 @@ pub struct Request { pub headers: Headers, /// The body of the request. pub body: Option, - /// The optional file associated with the request. - pub file: Option, + /// The optional files associated with the request. + pub files: Vec, } impl Request { @@ -220,7 +220,7 @@ impl Request { request_line, headers: Headers::default(), body: None, - file: None, + files: Vec::new(), }), Some(headers_end) => { // Parse the request headers. @@ -280,7 +280,7 @@ impl Request { request_line, headers, body, - file: None, + files: Vec::new(), }) } // If we can't find a CR LF CR LF even though the request should have headers @@ -449,7 +449,7 @@ mod tests { uri: Uri::new("http://localhost/home"), }, body: None, - file: None, + files: Vec::new(), headers: Headers::default(), }; let request_bytes = b"GET http://localhost/home HTTP/1.0\r\n\