From 9b44868d6d9751d174552d06c6551d4a492340a9 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Fri, 3 Jul 2026 15:16:17 -0600 Subject: [PATCH] fix(firecracker): read API responses by framing, not EOF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Firecracker's API server holds connections open regardless of the request's Connection: close (measured on v1.6.0 and v1.12.0: responses carry Connection: keep-alive and the server never half-closes), so read_to_end blocks until the socket read timeout on every request. wait_for_socket_ready then burns its whole window on one probe and create_vm fails with 'api socket not ready' against a healthy VMM — every real-Firecracker code path is unusable. Read the response by its HTTP/1.1 framing instead: headers to the blank line (capped at 64 KiB), then exactly Content-Length body bytes (absent means empty body; transfer-encoded responses are rejected loudly). --- src/adapters/firecracker.rs | 136 +++++++++++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 2 deletions(-) diff --git a/src/adapters/firecracker.rs b/src/adapters/firecracker.rs index 72ed2c8..758c467 100644 --- a/src/adapters/firecracker.rs +++ b/src/adapters/firecracker.rs @@ -74,6 +74,91 @@ fn mem_backend_from_env_value(value: Option<&str>) -> MemBackend { } } +/// Read one HTTP/1.1 response from `stream` by its framing: headers up to +/// the blank line, then exactly `Content-Length` body bytes (0 when the +/// header is absent — Firecracker's micro_http always sends it on non-empty +/// bodies). Returns the raw bytes (headers + body). +/// +/// Reading to EOF is not an option here: Firecracker's API server holds the +/// connection open regardless of `Connection: close`, so an EOF-terminated +/// read blocks until the socket read timeout on every request. +fn read_http_response(stream: &mut UnixStream) -> std::io::Result> { + // Cap header growth so a misbehaving server cannot balloon memory; FC + // response headers are well under 1 KiB. + const MAX_HEADER_BYTES: usize = 64 * 1024; + + let mut response: Vec = Vec::with_capacity(1024); + let mut chunk = [0u8; 4096]; + let header_end = loop { + if let Some(pos) = find_subslice(&response, b"\r\n\r\n") { + break pos + 4; + } + if response.len() > MAX_HEADER_BYTES { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "response headers exceed 64 KiB without terminator", + )); + } + let n = stream.read(&mut chunk)?; + if n == 0 { + return Err(std::io::Error::new( + std::io::ErrorKind::UnexpectedEof, + "connection closed before response headers completed", + )); + } + response.extend_from_slice(&chunk[..n]); + }; + + let headers_text = String::from_utf8_lossy(&response[..header_end]); + if headers_text + .lines() + .any(|l| l.to_ascii_lowercase().starts_with("transfer-encoding:")) + { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "transfer-encoded responses are not supported", + )); + } + let content_length = headers_text + .lines() + .find_map(|l| { + let (name, value) = l.split_once(':')?; + name.trim() + .eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse::()) + }) + .transpose() + .map_err(|e| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("invalid Content-Length: {e}"), + ) + })? + .unwrap_or(0); + + let total = header_end.checked_add(content_length).ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidData, "Content-Length overflow") + })?; + while response.len() < total { + let n = stream.read(&mut chunk)?; + if n == 0 { + return Err(std::io::Error::new( + std::io::ErrorKind::UnexpectedEof, + "connection closed mid-body", + )); + } + response.extend_from_slice(&chunk[..n]); + } + response.truncate(total); + Ok(response) +} + +fn find_subslice(haystack: &[u8], needle: &[u8]) -> Option { + haystack + .windows(needle.len()) + .position(|window| window == needle) +} + #[derive(Debug, Clone)] struct VmRecord { status: VmStatus, @@ -1035,8 +1120,14 @@ impl FirecrackerVmProvider { )) })?; - let mut response = Vec::new(); - stream.read_to_end(&mut response).map_err(|e| { + // Firecracker's API server keeps the connection alive even when the + // request says `Connection: close` (measured on v1.6 and v1.12: the + // response carries `Connection: keep-alive` and the server never + // half-closes). `read_to_end` therefore never sees EOF and blocks + // until the read timeout, failing every call — read the response by + // its framing (headers, then exactly `Content-Length` body bytes) + // instead of waiting for a close that never comes. + let response = read_http_response(&mut stream).map_err(|e| { VmRuntimeError::Unsupported(format!( "failed reading firecracker response {method} {endpoint}: {e}" )) @@ -1715,6 +1806,47 @@ mod tests { use crate::jailer::{Jailer, JailerConfig}; use crate::model::{RateLimiter, TokenBucket}; + /// Named bug: Firecracker's API server keeps the connection alive even + /// for `Connection: close` requests, so an EOF-terminated read + /// (`read_to_end`) blocks until the socket read timeout on every call — + /// `create_vm` then fails with "api socket not ready" against a + /// perfectly healthy VMM. The response must be read by its framing. + #[test] + fn read_http_response_returns_without_server_close() { + let (mut client, mut server) = UnixStream::pair().expect("socketpair"); + let response = + b"HTTP/1.1 200 \r\nServer: Firecracker API\r\nConnection: keep-alive\r\nContent-Type: application/json\r\nContent-Length: 13\r\n\r\n{\"state\":\"a\"}"; + server.write_all(response).expect("write response"); + server.flush().expect("flush"); + // Deliberately NOT closing/dropping `server`: the read must complete + // on framing alone. The timeout only bounds a regression — with the + // read_to_end implementation this test hangs here and fails. + client + .set_read_timeout(Some(Duration::from_secs(2))) + .expect("timeout"); + let got = read_http_response(&mut client).expect("framed read completes without EOF"); + assert_eq!(got, response.to_vec()); + drop(server); + } + + /// Body split across reads with no Content-Length on empty-body + /// responses (Firecracker's 204-style action replies). + #[test] + fn read_http_response_handles_missing_content_length() { + let (mut client, mut server) = UnixStream::pair().expect("socketpair"); + server + .write_all( + b"HTTP/1.1 204 \r\nServer: Firecracker API\r\nConnection: keep-alive\r\n\r\n", + ) + .expect("write response"); + client + .set_read_timeout(Some(Duration::from_secs(2))) + .expect("timeout"); + let got = read_http_response(&mut client).expect("headers-only response"); + assert!(got.ends_with(b"\r\n\r\n")); + drop(server); + } + fn test_config(root: &Path) -> FirecrackerConfig { FirecrackerConfig { binary_path: PathBuf::from("/usr/local/bin/firecracker"),