diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index 656a22b..93ddacf 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -248,7 +248,9 @@ impl ServoLiveClient { let reply = match self.ipc.exchange(request, timeout, operation) { Ok(reply) => reply, Err(error) => { - self.terminate(); + if !error.sidecar_remains_available() { + self.terminate(); + } return Err(error); } }; diff --git a/crates/ely_app/src/services/servo_live_ipc.rs b/crates/ely_app/src/services/servo_live_ipc.rs index e4e60b3..682d310 100644 --- a/crates/ely_app/src/services/servo_live_ipc.rs +++ b/crates/ely_app/src/services/servo_live_ipc.rs @@ -10,7 +10,7 @@ use super::{ ServoLiveError, ServoLiveFrame, ServoLivePermissionGrant, wire::{ LiveRequest, LiveResponse, LiveSurfaceHandle, MAX_FRAME_BYTE_COUNT, MAX_FRAME_DIMENSION, - MAX_RESPONSE_HEADER_BYTES, + MAX_REQUEST_LINE_BYTES, MAX_RESPONSE_HEADER_BYTES, }, }; @@ -82,7 +82,8 @@ fn run_io( while let Ok(message) = requests.recv() { let should_shutdown = matches!(message.request, LiveRequest::Shutdown); let response = exchange_pipes(&mut stdin, &mut stdout, &message.request); - let should_stop = should_shutdown || response.is_err(); + let should_stop = should_shutdown + || response.as_ref().is_err_and(|error| !error.sidecar_remains_available()); let _ = message.response.send(response); if should_stop { break; @@ -95,12 +96,22 @@ fn exchange_pipes( stdout: &mut BufReader, request: &LiveRequest, ) -> Result { - serde_json::to_writer(&mut *stdin, request)?; - stdin.write_all(b"\n").map_err(ServoLiveError::Command)?; + let line = serialize_request_line(request)?; + stdin.write_all(&line).map_err(ServoLiveError::Command)?; stdin.flush().map_err(ServoLiveError::Command)?; read_reply(stdout) } +fn serialize_request_line(request: &LiveRequest) -> Result, ServoLiveError> { + let mut line = serde_json::to_vec(request)?; + let bytes = line.len().saturating_add(1); + if bytes > MAX_REQUEST_LINE_BYTES { + return Err(ServoLiveError::RequestLineTooLarge { bytes, limit: MAX_REQUEST_LINE_BYTES }); + } + line.push(b'\n'); + Ok(line) +} + fn read_reply(stdout: &mut impl BufRead) -> Result { let mut line = String::new(); let bytes = Read::by_ref(stdout) @@ -280,6 +291,38 @@ mod tests { )); } + #[test] + fn request_line_preflight_accepts_the_limit_and_rejects_limit_plus_one() + -> Result<(), ServoLiveError> { + let empty = LiveRequest::Close { tab_id: String::new() }; + let fixed_bytes = serde_json::to_vec(&empty)?.len(); + let exact = + LiveRequest::Close { tab_id: "a".repeat(MAX_REQUEST_LINE_BYTES - fixed_bytes - 1) }; + + let exact_line = serialize_request_line(&exact)?; + assert_eq!(exact_line.len(), MAX_REQUEST_LINE_BYTES); + + let overflow = + LiveRequest::Close { tab_id: "a".repeat(MAX_REQUEST_LINE_BYTES - fixed_bytes) }; + let error = match serialize_request_line(&overflow) { + Err(error) => error, + Ok(_) => { + return Err(ServoLiveError::InvalidResponse { + message: "limit-plus-one request unexpectedly passed preflight", + }); + } + }; + assert!(matches!( + &error, + ServoLiveError::RequestLineTooLarge { + bytes, + limit: MAX_REQUEST_LINE_BYTES, + } if *bytes == MAX_REQUEST_LINE_BYTES + 1 + )); + assert!(error.sidecar_remains_available()); + Ok(()) + } + #[test] fn reply_parses_permission_consumption_without_a_frame() -> Result<(), ServoLiveError> { let profile_id = ely_domain::ProfileId::new(); diff --git a/crates/ely_app/src/services/servo_live_types.rs b/crates/ely_app/src/services/servo_live_types.rs index b45b952..b5d338d 100644 --- a/crates/ely_app/src/services/servo_live_types.rs +++ b/crates/ely_app/src/services/servo_live_types.rs @@ -317,6 +317,9 @@ pub(crate) enum ServoLiveError { #[error("servo live sidecar failed: {message}")] SidecarFailed { message: String }, + #[error("servo live request requires {bytes} bytes; the line limit is {limit}")] + RequestLineTooLarge { bytes: usize, limit: usize }, + #[error("servo live sidecar response header exceeded {limit} bytes")] ResponseHeaderTooLarge { limit: usize }, @@ -375,6 +378,10 @@ pub(crate) enum ServoLiveError { } impl ServoLiveError { + pub(crate) fn sidecar_remains_available(&self) -> bool { + matches!(self, Self::RequestLineTooLarge { .. }) + } + pub(crate) fn is_runtime_unavailable(&self) -> bool { match self { Self::SidecarExited @@ -403,6 +410,7 @@ impl ServoLiveError { Self::SidecarBinaryUnavailable { .. } | Self::PipeUnavailable { .. } | Self::SidecarFailed { .. } + | Self::RequestLineTooLarge { .. } | Self::SidecarCommand(_) => false, } } diff --git a/crates/ely_app/src/services/servo_live_wire.rs b/crates/ely_app/src/services/servo_live_wire.rs index 978068b..048cc1e 100644 --- a/crates/ely_app/src/services/servo_live_wire.rs +++ b/crates/ely_app/src/services/servo_live_wire.rs @@ -6,6 +6,7 @@ pub(super) const LIVE_PROTOCOL_VERSION: u32 = 3; pub(super) const MAX_FRAME_DIMENSION: u32 = 16_384; pub(super) const MAX_FRAME_BYTE_COUNT: usize = 256 * 1024 * 1024; pub(super) const MAX_RESPONSE_HEADER_BYTES: usize = 256 * 1024; +pub(super) const MAX_REQUEST_LINE_BYTES: usize = 1024 * 1024; #[derive(Serialize)] #[serde(tag = "type", rename_all = "snake_case")] @@ -110,7 +111,14 @@ fn default_device_pixel_ratio() -> f32 { mod tests { use serde_json::json; - use super::{LIVE_PROTOCOL_VERSION, LiveRequest, ServoLiveSitePermission}; + use super::{ + LIVE_PROTOCOL_VERSION, LiveRequest, MAX_REQUEST_LINE_BYTES, ServoLiveSitePermission, + }; + + #[test] + fn request_line_limit_matches_the_host_protocol() { + assert_eq!(MAX_REQUEST_LINE_BYTES, ely_servo_host::MAX_REQUEST_LINE_BYTES); + } #[test] fn handshake_request_serializes_protocol_version() -> Result<(), serde_json::Error> { diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs index e893073..1940035 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs @@ -11,6 +11,8 @@ mod live; mod live_output; #[path = "ely_servo_sidecar/live_protocol.rs"] mod live_protocol; +#[path = "ely_servo_sidecar/live_request.rs"] +mod live_request; #[path = "ely_servo_sidecar/live_session.rs"] mod live_session; #[cfg(all(feature = "hardware-render", target_os = "macos"))] diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs index 7f8e480..ebe68db 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs @@ -1,7 +1,7 @@ use std::{ collections::{HashMap, VecDeque}, fs::{self, File, OpenOptions, TryLockError}, - io::{self, BufRead}, + io, path::Path, }; @@ -19,6 +19,7 @@ use super::{ LIVE_PROTOCOL_VERSION, LiveFrameReport, LiveOutcome, LiveRequest, LiveSidecarError, MAX_LOADED_URL_BYTES, MAX_PERMISSION_CONSUMPTIONS_PER_RESPONSE, validated_frame_byte_count, }, + live_request::{MAX_REQUEST_LINE_BYTES, RequestLineRead, read_request_line}, live_session::{ LiveInput, LiveSession, apply_input, apply_layout, apply_permissions, bind_profile, ensure_session, @@ -57,17 +58,26 @@ pub(super) fn run(args: LiveArgs) -> Result<(), LiveSidecarError> { let mut handshake_complete = false; let mut pending_permission_consumptions = VecDeque::new(); let stdin = io::stdin(); + let mut stdin = stdin.lock(); let mut stdout = io::stdout().lock(); + let mut line = Vec::new(); - for line in stdin.lock().lines() { - let line = line?; - if line.trim().is_empty() { - continue; - } - let request = serde_json::from_str::(&line); + loop { + let request = match read_request_line(&mut stdin, &mut line)? { + RequestLineRead::Eof => break, + RequestLineRead::Ready if line.iter().all(|byte| byte.is_ascii_whitespace()) => { + continue; + } + RequestLineRead::Ready => { + serde_json::from_slice::(&line).map_err(LiveSidecarError::from) + } + RequestLineRead::TooLarge => { + Err(LiveSidecarError::RequestLineTooLarge { limit: MAX_REQUEST_LINE_BYTES }) + } + }; let should_shutdown = request.as_ref().is_ok_and(|request| matches!(request, LiveRequest::Shutdown)); - let outcome = request.map_err(LiveSidecarError::from).and_then(|request| { + let outcome = request.and_then(|request| { handle_request( &mut host, &mut sessions, diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs index 124bc3e..19528f5 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs @@ -297,6 +297,9 @@ pub(super) enum LiveSidecarError { #[error("live protocol mismatch: expected {expected}, received {actual}")] ProtocolVersionMismatch { expected: u32, actual: u32 }, + #[error("live request line exceeds the {limit}-byte protocol limit")] + RequestLineTooLarge { limit: usize }, + #[error("request URL exceeds the {limit}-byte live protocol limit")] RequestUrlTooLong { limit: usize }, diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_request.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_request.rs new file mode 100644 index 0000000..7862707 --- /dev/null +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_request.rs @@ -0,0 +1,31 @@ +use std::io::{self, BufRead, Read}; + +pub(super) use ely_servo_host::MAX_REQUEST_LINE_BYTES; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum RequestLineRead { + Eof, + Ready, + TooLarge, +} + +pub(super) fn read_request_line( + reader: &mut impl BufRead, + line: &mut Vec, +) -> io::Result { + line.clear(); + let bytes = { + let mut limited = Read::by_ref(reader).take((MAX_REQUEST_LINE_BYTES + 1) as u64); + limited.read_until(b'\n', line)? + }; + if bytes == 0 { + return Ok(RequestLineRead::Eof); + } + if bytes <= MAX_REQUEST_LINE_BYTES { + return Ok(RequestLineRead::Ready); + } + if !line.ends_with(b"\n") { + reader.skip_until(b'\n')?; + } + Ok(RequestLineRead::TooLarge) +} diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_tests.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_tests.rs index 5057673..9bf0465 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_tests.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_tests.rs @@ -1,6 +1,10 @@ -use std::collections::VecDeque; +use std::{collections::VecDeque, io::Cursor}; use super::take_permission_batch; +use crate::{ + live_protocol::LiveRequest, + live_request::{MAX_REQUEST_LINE_BYTES, RequestLineRead, read_request_line}, +}; #[test] fn nine_permission_consumptions_are_sent_in_order_across_two_responses() { @@ -10,6 +14,66 @@ fn nine_permission_consumptions_are_sent_in_order_across_two_responses() { assert_eq!(take_permission_batch(&mut pending), vec![9]); } +#[test] +fn request_line_accepts_the_exact_byte_limit() -> Result<(), Box> { + let mut input = Cursor::new(padded_handshake_line(MAX_REQUEST_LINE_BYTES)); + let mut line = Vec::new(); + + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::Ready); + assert_eq!(line.len(), MAX_REQUEST_LINE_BYTES); + assert!(matches!(serde_json::from_slice::(&line)?, LiveRequest::Handshake { .. })); + Ok(()) +} + +#[test] +fn request_line_rejects_limit_plus_one_and_preserves_the_next_frame() +-> Result<(), Box> { + let mut wire = padded_handshake_line(MAX_REQUEST_LINE_BYTES + 1); + wire.extend_from_slice(br#"{"type":"shutdown"}"#); + wire.push(b'\n'); + let mut input = Cursor::new(wire); + let mut line = Vec::new(); + + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::TooLarge); + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::Ready); + assert!(matches!(serde_json::from_slice::(&line)?, LiveRequest::Shutdown)); + Ok(()) +} + +#[test] +fn request_line_drains_a_distant_delimiter_before_recovery() +-> Result<(), Box> { + let mut wire = padded_handshake_line(MAX_REQUEST_LINE_BYTES + 4_096); + wire.extend_from_slice(br#"{"type":"shutdown"}"#); + wire.push(b'\n'); + let mut input = Cursor::new(wire); + let mut line = Vec::new(); + + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::TooLarge); + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::Ready); + assert!(matches!(serde_json::from_slice::(&line)?, LiveRequest::Shutdown)); + Ok(()) +} + +#[test] +fn request_line_accepts_a_bounded_eof_terminated_frame() -> Result<(), Box> { + let mut input = Cursor::new(br#"{"type":"shutdown"}"#.to_vec()); + let mut line = Vec::new(); + + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::Ready); + assert!(matches!(serde_json::from_slice::(&line)?, LiveRequest::Shutdown)); + assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::Eof); + Ok(()) +} + +fn padded_handshake_line(bytes: usize) -> Vec { + let mut line = br#"{"type":"handshake","protocol_version":3}"#.to_vec(); + assert!(bytes > line.len()); + line.resize(bytes - 1, b' '); + line.push(b'\n'); + line +} + #[cfg(all(feature = "hardware-render", target_os = "macos"))] mod hardware { use super::super::{HardwarePollAction, hardware_poll_action}; diff --git a/crates/ely_servo_host/src/lib.rs b/crates/ely_servo_host/src/lib.rs index ba495dc..b0bf971 100644 --- a/crates/ely_servo_host/src/lib.rs +++ b/crates/ely_servo_host/src/lib.rs @@ -16,6 +16,8 @@ mod runtime_waker; #[cfg(feature = "servo-engine")] mod runtime_webview; +pub const MAX_REQUEST_LINE_BYTES: usize = 1024 * 1024; + pub use error::ServoHostError; #[cfg(feature = "hardware-render")] pub use hardware_rendering_context::HardwareOffscreenContext; diff --git a/crates/ely_servo_host/tests/sidecar.rs b/crates/ely_servo_host/tests/sidecar.rs index 88211b4..2038503 100644 --- a/crates/ely_servo_host/tests/sidecar.rs +++ b/crates/ely_servo_host/tests/sidecar.rs @@ -2,14 +2,15 @@ use std::{ error::Error, - io, + io::{self, BufRead, BufReader, Write}, process::{Command, Stdio}, thread, time::{Duration, Instant}, }; use ely_domain::{ProfileId, TabId}; -use serde_json::json; +use ely_servo_host::MAX_REQUEST_LINE_BYTES; +use serde_json::{Value, json}; #[cfg(all(feature = "hardware-render", target_os = "macos"))] #[path = "sidecar/mach_receiver.rs"] @@ -174,6 +175,59 @@ fn live_sidecar_rejects_an_incompatible_protocol() -> Result<(), Box> Ok(()) } +#[test] +fn live_sidecar_drains_an_oversized_request_line_and_recovers_the_handshake() +-> Result<(), Box> { + let root = TestDirectory::new()?; + let mut child = Command::new(env!("CARGO_BIN_EXE_ely_servo_sidecar")) + .arg("live") + .arg("--profile-data-dir") + .arg(root.path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn()?; + let mut stdin = child.stdin.take().ok_or_else(|| io::Error::other("missing stdin"))?; + let stdout = child.stdout.take().ok_or_else(|| io::Error::other("missing stdout"))?; + let mut stdout = BufReader::new(stdout); + let mut oversized = br#"{"type":"handshake","protocol_version":3}"#.to_vec(); + oversized.resize(MAX_REQUEST_LINE_BYTES + 4_096, b' '); + oversized.push(b'\n'); + + stdin.write_all(&oversized)?; + serde_json::to_writer( + &mut stdin, + &json!({"type": "handshake", "protocol_version": LIVE_PROTOCOL_VERSION}), + )?; + stdin.write_all(b"\n")?; + stdin.flush()?; + + let mut line = String::new(); + stdout.read_line(&mut line)?; + let oversized_response: Value = serde_json::from_str(&line)?; + assert_eq!( + oversized_response["error"], + format!("live request line exceeds the {MAX_REQUEST_LINE_BYTES}-byte protocol limit") + ); + + line.clear(); + stdout.read_line(&mut line)?; + let handshake_response: Value = serde_json::from_str(&line)?; + assert_eq!(handshake_response["protocol_version"], LIVE_PROTOCOL_VERSION); + assert!(handshake_response["error"].is_null()); + + serde_json::to_writer(&mut stdin, &json!({"type": "shutdown"}))?; + stdin.write_all(b"\n")?; + stdin.flush()?; + line.clear(); + stdout.read_line(&mut line)?; + let shutdown_response: Value = serde_json::from_str(&line)?; + assert!(shutdown_response["error"].is_null()); + drop(stdin); + assert!(child.wait()?.success()); + Ok(()) +} + #[test] fn live_sidecar_rejects_oversized_frame_dimensions() -> Result<(), Box> { let root = TestDirectory::new()?;