fix(servo): bound live request frames

This commit is contained in:
2026-07-09 23:44:27 -04:00
parent 83badaea81
commit 58e9bd7d38
11 changed files with 244 additions and 17 deletions
+3 -1
View File
@@ -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);
}
};
+47 -4
View File
@@ -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<ChildStdout>,
request: &LiveRequest,
) -> Result<IpcReply, ServoLiveError> {
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<Vec<u8>, 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<IpcReply, ServoLiveError> {
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();
@@ -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,
}
}
@@ -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> {
@@ -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"))]
@@ -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::<LiveRequest>(&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::<LiveRequest>(&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,
@@ -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 },
@@ -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<u8>,
) -> io::Result<RequestLineRead> {
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)
}
@@ -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<dyn std::error::Error>> {
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::<LiveRequest>(&line)?, LiveRequest::Handshake { .. }));
Ok(())
}
#[test]
fn request_line_rejects_limit_plus_one_and_preserves_the_next_frame()
-> Result<(), Box<dyn std::error::Error>> {
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::<LiveRequest>(&line)?, LiveRequest::Shutdown));
Ok(())
}
#[test]
fn request_line_drains_a_distant_delimiter_before_recovery()
-> Result<(), Box<dyn std::error::Error>> {
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::<LiveRequest>(&line)?, LiveRequest::Shutdown));
Ok(())
}
#[test]
fn request_line_accepts_a_bounded_eof_terminated_frame() -> Result<(), Box<dyn std::error::Error>> {
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::<LiveRequest>(&line)?, LiveRequest::Shutdown));
assert_eq!(read_request_line(&mut input, &mut line)?, RequestLineRead::Eof);
Ok(())
}
fn padded_handshake_line(bytes: usize) -> Vec<u8> {
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};
+2
View File
@@ -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;
+56 -2
View File
@@ -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<dyn Error>>
Ok(())
}
#[test]
fn live_sidecar_drains_an_oversized_request_line_and_recovers_the_handshake()
-> Result<(), Box<dyn Error>> {
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<dyn Error>> {
let root = TestDirectory::new()?;