From 8334fcbd6e0661fc226d09a26304bc4fe2453caa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 8 May 2026 22:10:10 -0400 Subject: [PATCH] Wire sidecar profile data isolation --- crates/ely_app/src/services/mod.rs | 2 + .../src/services/servo_profile_data.rs | 17 ++ crates/ely_app/src/services/servo_sidecar.rs | 162 +++++++----------- .../src/services/servo_sidecar_command.rs | 97 +++++++++++ .../src/services/servo_sidecar_tests.rs | 42 ++++- crates/ely_app/src/shell/web_surface.rs | 11 +- crates/ely_domain/src/error.rs | 3 + crates/ely_domain/src/identifiers.rs | 57 ++++++ .../src/bin/ely_servo_sidecar.rs | 11 +- .../src/bin/ely_servo_sidecar/args.rs | 100 ++++++++++- .../src/bin/ely_servo_sidecar/report.rs | 2 + crates/ely_servo_host/src/lib.rs | 2 + crates/ely_servo_host/src/runtime.rs | 42 ++--- .../ely_servo_host/src/runtime_permissions.rs | 20 +++ crates/ely_servo_host/tests/sidecar.rs | 67 +++++++- .../ely_servo_host/tests/sidecar/support.rs | 29 +++- 16 files changed, 523 insertions(+), 141 deletions(-) create mode 100644 crates/ely_app/src/services/servo_profile_data.rs create mode 100644 crates/ely_app/src/services/servo_sidecar_command.rs create mode 100644 crates/ely_servo_host/src/runtime_permissions.rs diff --git a/crates/ely_app/src/services/mod.rs b/crates/ely_app/src/services/mod.rs index 31d5a97..ac69174 100644 --- a/crates/ely_app/src/services/mod.rs +++ b/crates/ely_app/src/services/mod.rs @@ -3,7 +3,9 @@ pub mod download_files; pub mod plugin_package_store; pub mod plugin_packages; pub mod plugin_signatures; +mod servo_profile_data; pub mod servo_sidecar; +mod servo_sidecar_command; #[cfg(all(test, feature = "live-site-smoke"))] pub(crate) mod prd_live_sites; diff --git a/crates/ely_app/src/services/servo_profile_data.rs b/crates/ely_app/src/services/servo_profile_data.rs new file mode 100644 index 0000000..beed14e --- /dev/null +++ b/crates/ely_app/src/services/servo_profile_data.rs @@ -0,0 +1,17 @@ +use std::path::{Path, PathBuf}; + +use directories::ProjectDirs; +use ely_domain::ProfileId; + +const ELY_QUALIFIER: &str = "com"; +const ELY_ORGANIZATION: &str = "elydora"; +const ELY_APPLICATION: &str = "ELY Browser"; + +pub(super) fn default_profile_data_root() -> Option { + ProjectDirs::from(ELY_QUALIFIER, ELY_ORGANIZATION, ELY_APPLICATION) + .map(|project_dirs| project_dirs.data_dir().join("profiles")) +} + +pub(super) fn profile_data_dir(profile_data_root: &Path, profile_id: &ProfileId) -> PathBuf { + profile_data_root.join(profile_id.as_str()).join("servo") +} diff --git a/crates/ely_app/src/services/servo_sidecar.rs b/crates/ely_app/src/services/servo_sidecar.rs index f747b06..5604b0d 100644 --- a/crates/ely_app/src/services/servo_sidecar.rs +++ b/crates/ely_app/src/services/servo_sidecar.rs @@ -6,26 +6,33 @@ use std::{ time::{Duration, Instant, SystemTime, SystemTimeError, UNIX_EPOCH}, }; -use ely_domain::UrlText; +use ely_domain::{ProfileId, UrlText}; use serde::Deserialize; use thiserror::Error; -const SIDECAR_BINARY_TIMEOUT: Duration = Duration::from_secs(45); -const SIDECAR_CARGO_TIMEOUT: Duration = Duration::from_secs(180); +use super::{ + servo_profile_data::{default_profile_data_root, profile_data_dir}, + servo_sidecar_command::{SidecarCommandTarget, default_sidecar_command}, +}; + const SIDECAR_POLL_INTERVAL: Duration = Duration::from_millis(20); const SIDECAR_RETRY_INTERVAL: Duration = Duration::from_millis(250); const SIDECAR_NAVIGATION_ATTEMPTS: usize = 3; const SIDECAR_INTERACTION_ATTEMPTS: usize = 1; -const SIDECAR_PATH_ENV: &str = "ELY_SERVO_SIDECAR"; #[derive(Clone, Debug)] pub struct ServoSidecarClient { command_target: SidecarCommandTarget, + profile_data_root: PathBuf, } impl ServoSidecarClient { pub fn new() -> Result { - Ok(Self { command_target: default_sidecar_command()? }) + Ok(Self { + command_target: default_sidecar_command()?, + profile_data_root: default_profile_data_root() + .ok_or(ServoSidecarError::ProfileDataRootUnavailable)?, + }) } pub fn snapshot( @@ -56,7 +63,8 @@ impl ServoSidecarClient { request: &SidecarSnapshotRequest, ) -> Result { let rgba_path = temporary_rgba_path()?; - let output = match self.run_snapshot_command(request, &rgba_path) { + let profile_data_dir = profile_data_dir(&self.profile_data_root, &request.profile_id); + let output = match self.run_snapshot_command(request, &rgba_path, &profile_data_dir) { Ok(output) => output, Err(error) => { remove_temporary_file(&rgba_path)?; @@ -72,7 +80,7 @@ impl ServoSidecarClient { }); } - let snapshot = read_sidecar_snapshot(&output.stdout, &rgba_path); + let snapshot = read_sidecar_snapshot(&output.stdout, &rgba_path, &request.profile_id); let cleanup = remove_temporary_file(&rgba_path); match (snapshot, cleanup) { (Ok(snapshot), Ok(())) => Ok(snapshot), @@ -85,9 +93,10 @@ impl ServoSidecarClient { &self, request: &SidecarSnapshotRequest, rgba_path: &Path, + profile_data_dir: &Path, ) -> Result { let mut command = self.command_target.command(); - append_snapshot_args(&mut command, request, rgba_path); + append_snapshot_args(&mut command, request, rgba_path, profile_data_dir); if let Some(click_point) = request.click_point { command .arg("--click-x") @@ -125,55 +134,20 @@ impl ServoSidecarClient { } } -#[derive(Clone, Debug)] -enum SidecarCommandTarget { - Binary(PathBuf), - Cargo { manifest_path: PathBuf }, -} - -impl SidecarCommandTarget { - fn command(&self) -> Command { - match self { - Self::Binary(path) => Command::new(path), - Self::Cargo { manifest_path } => { - let mut command = Command::new("cargo"); - command - .arg("run") - .arg("--quiet") - .arg("--manifest-path") - .arg(manifest_path) - .arg("-p") - .arg("ely_servo_host") - .arg("--features") - .arg("servo-engine") - .arg("--bin") - .arg("ely_servo_sidecar") - .arg("--"); - command - } - } - } - - fn timeout(&self) -> Duration { - match self { - Self::Binary(_) => SIDECAR_BINARY_TIMEOUT, - Self::Cargo { .. } => SIDECAR_CARGO_TIMEOUT, - } - } - - fn missing_binary_path(&self) -> Option<&Path> { - match self { - Self::Binary(path) if !path.is_file() => Some(path.as_path()), - Self::Binary(_) | Self::Cargo { .. } => None, - } - } -} - -fn append_snapshot_args(command: &mut Command, request: &SidecarSnapshotRequest, rgba_path: &Path) { +fn append_snapshot_args( + command: &mut Command, + request: &SidecarSnapshotRequest, + rgba_path: &Path, + profile_data_dir: &Path, +) { command .arg("snapshot") .arg("--url") .arg(request.url.as_str()) + .arg("--profile-id") + .arg(request.profile_id.as_str()) + .arg("--profile-data-dir") + .arg(profile_data_dir) .arg("--rgba-out") .arg(rgba_path) .arg("--width") @@ -189,6 +163,7 @@ fn append_snapshot_args(command: &mut Command, request: &SidecarSnapshotRequest, #[derive(Clone, Debug)] pub struct SidecarSnapshotRequest { url: UrlText, + profile_id: ProfileId, width: u32, height: u32, scroll_x: i32, @@ -199,8 +174,17 @@ pub struct SidecarSnapshotRequest { impl SidecarSnapshotRequest { #[must_use] - pub fn new(url: UrlText, width: u32, height: u32) -> Self { - Self { url, width, height, scroll_x: 0, scroll_y: 0, click_point: None, typed_text: None } + pub fn new(url: UrlText, profile_id: ProfileId, width: u32, height: u32) -> Self { + Self { + url, + profile_id, + width, + height, + scroll_x: 0, + scroll_y: 0, + click_point: None, + typed_text: None, + } } #[must_use] @@ -227,6 +211,11 @@ impl SidecarSnapshotRequest { self.typed_text.as_deref() } + #[cfg(test)] + pub(crate) fn profile_id_for_test(&self) -> &ProfileId { + &self.profile_id + } + fn max_attempts(&self) -> usize { if self.click_point.is_some() || self.typed_text.is_some() { return SIDECAR_INTERACTION_ATTEMPTS; @@ -258,8 +247,18 @@ pub struct SidecarSnapshot { } impl SidecarSnapshot { - fn from_report(report: SidecarReport, rgba_bytes: Vec) -> Result { + fn from_report( + report: SidecarReport, + expected_profile_id: &ProfileId, + rgba_bytes: Vec, + ) -> Result { let expected_byte_count = expected_rgba_byte_count(report.width, report.height)?; + if report.profile_id != expected_profile_id.as_str() { + return Err(ServoSidecarError::ProfileMismatch { + expected: expected_profile_id.as_str().to_string(), + actual: report.profile_id, + }); + } if !is_renderable_state(&report.state) { return Err(ServoSidecarError::IncompleteRender { state: report.state }); } @@ -337,6 +336,9 @@ pub enum ServoSidecarError { #[error("temporary frame directory is unavailable: {0}")] TempDirectory(#[source] io::Error), + #[error("profile data root is unavailable")] + ProfileDataRootUnavailable, + #[error("system clock is before UNIX epoch")] SystemClock(#[from] SystemTimeError), @@ -361,6 +363,9 @@ pub enum ServoSidecarError { #[error("servo sidecar returned incomplete render state: {state}")] IncompleteRender { state: String }, + #[error("servo sidecar returned profile {actual}, expected {expected}")] + ProfileMismatch { expected: String, actual: String }, + #[error( "servo frame byte count mismatch: expected {expected}, reported {reported}, actual {actual}" )] @@ -383,6 +388,7 @@ fn is_renderable_state(state: &str) -> bool { #[derive(Deserialize)] struct SidecarReport { requested_url: String, + profile_id: String, loaded_url: Option, title: Option, state: String, @@ -395,47 +401,6 @@ struct SidecarReport { sample_hash: u64, } -fn default_sidecar_command() -> Result { - if let Some(path) = env::var_os(SIDECAR_PATH_ENV) { - return Ok(SidecarCommandTarget::Binary(PathBuf::from(path))); - } - - let current_exe = env::current_exe().map_err(ServoSidecarError::CurrentExecutable)?; - let exe_dir = current_exe.parent().ok_or_else(|| { - ServoSidecarError::CurrentExecutableDirectoryUnavailable { path: current_exe.clone() } - })?; - let adjacent_sidecar = exe_dir.join(sidecar_binary_name()); - if adjacent_sidecar.is_file() { - return Ok(SidecarCommandTarget::Binary(adjacent_sidecar)); - } - - if let Some(manifest_path) = workspace_manifest_path() { - if let Some(target_sidecar) = workspace_target_sidecar_path(&manifest_path) - && target_sidecar.is_file() - { - return Ok(SidecarCommandTarget::Binary(target_sidecar)); - } - if manifest_path.is_file() { - return Ok(SidecarCommandTarget::Cargo { manifest_path }); - } - } - - Ok(SidecarCommandTarget::Binary(adjacent_sidecar)) -} - -fn workspace_manifest_path() -> Option { - option_env!("ELY_WORKSPACE_MANIFEST").map(PathBuf::from) -} - -fn workspace_target_sidecar_path(manifest_path: &Path) -> Option { - let profile = if cfg!(debug_assertions) { "debug" } else { "release" }; - Some(manifest_path.parent()?.join("target").join(profile).join(sidecar_binary_name())) -} - -fn sidecar_binary_name() -> String { - format!("ely_servo_sidecar{}", env::consts::EXE_SUFFIX) -} - fn temporary_rgba_path() -> Result { let directory = env::temp_dir().join("ely-browser-servo"); fs::create_dir_all(&directory).map_err(ServoSidecarError::TempDirectory)?; @@ -465,10 +430,11 @@ fn terminate_child(mut child: std::process::Child) -> Result<(), ServoSidecarErr fn read_sidecar_snapshot( stdout: &[u8], rgba_path: &Path, + expected_profile_id: &ProfileId, ) -> Result { let report: SidecarReport = serde_json::from_slice(stdout)?; let rgba_bytes = fs::read(rgba_path).map_err(ServoSidecarError::FrameRead)?; - SidecarSnapshot::from_report(report, rgba_bytes) + SidecarSnapshot::from_report(report, expected_profile_id, rgba_bytes) } fn expected_rgba_byte_count(width: u32, height: u32) -> Result { diff --git a/crates/ely_app/src/services/servo_sidecar_command.rs b/crates/ely_app/src/services/servo_sidecar_command.rs new file mode 100644 index 0000000..1bef907 --- /dev/null +++ b/crates/ely_app/src/services/servo_sidecar_command.rs @@ -0,0 +1,97 @@ +use std::{ + env, + path::{Path, PathBuf}, + process::Command, + time::Duration, +}; + +use super::servo_sidecar::ServoSidecarError; + +const SIDECAR_BINARY_TIMEOUT: Duration = Duration::from_secs(45); +const SIDECAR_CARGO_TIMEOUT: Duration = Duration::from_secs(180); +const SIDECAR_PATH_ENV: &str = "ELY_SERVO_SIDECAR"; + +#[derive(Clone, Debug)] +pub(super) enum SidecarCommandTarget { + Binary(PathBuf), + Cargo { manifest_path: PathBuf }, +} + +impl SidecarCommandTarget { + pub(super) fn command(&self) -> Command { + match self { + Self::Binary(path) => Command::new(path), + Self::Cargo { manifest_path } => { + let mut command = Command::new("cargo"); + command + .arg("run") + .arg("--quiet") + .arg("--manifest-path") + .arg(manifest_path) + .arg("-p") + .arg("ely_servo_host") + .arg("--features") + .arg("servo-engine") + .arg("--bin") + .arg("ely_servo_sidecar") + .arg("--"); + command + } + } + } + + pub(super) fn timeout(&self) -> Duration { + match self { + Self::Binary(_) => SIDECAR_BINARY_TIMEOUT, + Self::Cargo { .. } => SIDECAR_CARGO_TIMEOUT, + } + } + + pub(super) fn missing_binary_path(&self) -> Option<&Path> { + match self { + Self::Binary(path) if !path.is_file() => Some(path.as_path()), + Self::Binary(_) | Self::Cargo { .. } => None, + } + } +} + +pub(super) fn default_sidecar_command() -> Result { + if let Some(path) = env::var_os(SIDECAR_PATH_ENV) { + return Ok(SidecarCommandTarget::Binary(PathBuf::from(path))); + } + + let current_exe = env::current_exe().map_err(ServoSidecarError::CurrentExecutable)?; + let exe_dir = current_exe.parent().ok_or_else(|| { + ServoSidecarError::CurrentExecutableDirectoryUnavailable { path: current_exe.clone() } + })?; + let adjacent_sidecar = exe_dir.join(sidecar_binary_name()); + if adjacent_sidecar.is_file() { + return Ok(SidecarCommandTarget::Binary(adjacent_sidecar)); + } + + if let Some(manifest_path) = workspace_manifest_path() { + if let Some(target_sidecar) = workspace_target_sidecar_path(&manifest_path) + && target_sidecar.is_file() + { + return Ok(SidecarCommandTarget::Binary(target_sidecar)); + } + if manifest_path.is_file() { + return Ok(SidecarCommandTarget::Cargo { manifest_path }); + } + } + + Ok(SidecarCommandTarget::Binary(adjacent_sidecar)) +} + +fn workspace_manifest_path() -> Option { + option_env!("ELY_WORKSPACE_MANIFEST").map(PathBuf::from) +} + +fn workspace_target_sidecar_path(manifest_path: &Path) -> Option { + let profile = if cfg!(debug_assertions) { "debug" } else { "release" }; + Some(manifest_path.parent()?.join("target").join(profile).join(sidecar_binary_name())) +} + +fn sidecar_binary_name() -> String { + format!("ely_servo_sidecar{}", env::consts::EXE_SUFFIX) +} diff --git a/crates/ely_app/src/services/servo_sidecar_tests.rs b/crates/ely_app/src/services/servo_sidecar_tests.rs index b8ac8e1..64ac083 100644 --- a/crates/ely_app/src/services/servo_sidecar_tests.rs +++ b/crates/ely_app/src/services/servo_sidecar_tests.rs @@ -17,7 +17,12 @@ const MINIMUM_CONTENT_PIXELS: u64 = 1_000; #[test] fn accepts_loading_report_with_visible_content() -> Result<(), ServoSidecarError> { - let snapshot = SidecarSnapshot::from_report(report_with_state("loading"), visible_frame())?; + let profile_id = ProfileId::new(); + let snapshot = SidecarSnapshot::from_report( + report_with_state("loading", &profile_id), + &profile_id, + visible_frame(), + )?; assert_eq!(snapshot.loaded_url(), Some("https://example.com/")); assert_eq!(snapshot.title(), Some("Example Domain")); @@ -31,16 +36,36 @@ fn accepts_loading_report_with_visible_content() -> Result<(), ServoSidecarError #[test] fn rejects_created_report_with_visible_content() { - let result = SidecarSnapshot::from_report(report_with_state("created"), visible_frame()); + let profile_id = ProfileId::new(); + let result = SidecarSnapshot::from_report( + report_with_state("created", &profile_id), + &profile_id, + visible_frame(), + ); assert!( matches!(result, Err(ServoSidecarError::IncompleteRender { state }) if state == "created") ); } +#[test] +fn rejects_report_from_different_profile() { + let expected_profile_id = ProfileId::new(); + let actual_profile_id = ProfileId::new(); + let result = SidecarSnapshot::from_report( + report_with_state("loading", &actual_profile_id), + &expected_profile_id, + visible_frame(), + ); + + assert!(matches!(result, Err(ServoSidecarError::ProfileMismatch { expected, actual }) + if expected == expected_profile_id.as_str() && actual == actual_profile_id.as_str())); +} + #[test] fn retries_navigation_snapshots() -> Result<(), Box> { - let request = SidecarSnapshotRequest::new(UrlText::parse("https://example.com")?, 2, 1); + let request = + SidecarSnapshotRequest::new(UrlText::parse("https://example.com")?, ProfileId::new(), 2, 1); assert_eq!(request.max_attempts(), SIDECAR_NAVIGATION_ATTEMPTS); Ok(()) @@ -48,9 +73,10 @@ fn retries_navigation_snapshots() -> Result<(), Box> { #[test] fn keeps_page_interactions_single_attempt() -> Result<(), Box> { - let request = SidecarSnapshotRequest::new(UrlText::parse("https://example.com")?, 2, 1) - .with_click_point(1, 1) - .with_typed_text("ely".to_string()); + let request = + SidecarSnapshotRequest::new(UrlText::parse("https://example.com")?, ProfileId::new(), 2, 1) + .with_click_point(1, 1) + .with_typed_text("ely".to_string()); assert_eq!(request.max_attempts(), SIDECAR_INTERACTION_ATTEMPTS); Ok(()) @@ -80,6 +106,7 @@ fn assert_live_sites_render(cases: &[LiveSiteCase]) -> Result<(), Box for case in cases { let request = SidecarSnapshotRequest::new( UrlText::parse(case.url)?, + ProfileId::new(), LIVE_SITE_WIDTH, LIVE_SITE_HEIGHT, ); @@ -122,9 +149,10 @@ fn assert_title_contains(snapshot: &SidecarSnapshot, fragment: &str) -> Result<( Ok(()) } -fn report_with_state(state: &str) -> SidecarReport { +fn report_with_state(state: &str, profile_id: &ProfileId) -> SidecarReport { SidecarReport { requested_url: "https://example.com".to_string(), + profile_id: profile_id.as_str().to_string(), loaded_url: Some("https://example.com/".to_string()), title: Some("Example Domain".to_string()), state: state.to_string(), diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 117b4a0..3398768 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -102,9 +102,13 @@ impl WebSurfaceStore { }, ); - let mut snapshot_request = - SidecarSnapshotRequest::new(tab.url().clone(), size.width, size.height) - .with_scroll_offset(scroll_offset.x(), scroll_offset.y()); + let mut snapshot_request = SidecarSnapshotRequest::new( + tab.url().clone(), + tab.profile_id().clone(), + size.width, + size.height, + ) + .with_scroll_offset(scroll_offset.x(), scroll_offset.y()); if let Some(click_point) = click_point { snapshot_request = snapshot_request.with_click_point(click_point.x(), click_point.y()); } @@ -427,6 +431,7 @@ mod tests { assert_eq!(request.typed_text.as_deref(), Some("el")); assert_eq!(request.snapshot_request.typed_text_for_test(), Some("el")); + assert_eq!(request.snapshot_request.profile_id_for_test(), tab.profile_id()); Ok(()) } diff --git a/crates/ely_domain/src/error.rs b/crates/ely_domain/src/error.rs index 1860838..238a36f 100644 --- a/crates/ely_domain/src/error.rs +++ b/crates/ely_domain/src/error.rs @@ -14,6 +14,9 @@ pub enum DomainError { #[error("invalid file name: {value}")] InvalidFileName { value: String }, + #[error("invalid {kind} id: {value}")] + InvalidEntityId { kind: &'static str, value: String }, + #[error("invalid {algorithm} download checksum: {value}")] InvalidDownloadChecksum { algorithm: &'static str, value: String }, diff --git a/crates/ely_domain/src/identifiers.rs b/crates/ely_domain/src/identifiers.rs index f88d2dd..b6c3b2f 100644 --- a/crates/ely_domain/src/identifiers.rs +++ b/crates/ely_domain/src/identifiers.rs @@ -1,5 +1,6 @@ use std::fmt; +use crate::DomainError; use uuid::Uuid; macro_rules! entity_id { @@ -13,6 +14,22 @@ macro_rules! entity_id { Self(format!("{}_{}", $prefix, Uuid::now_v7().simple())) } + pub fn parse(value: impl Into) -> Result { + let value = value.into(); + let trimmed = value.trim(); + if trimmed.is_empty() { + return Err(DomainError::EmptyField { field: concat!($prefix, "_id") }); + } + if !is_valid_entity_id(trimmed, $prefix) { + return Err(DomainError::InvalidEntityId { + kind: $prefix, + value: trimmed.to_string(), + }); + } + + Ok(Self(trimmed.to_string())) + } + #[must_use] pub fn as_str(&self) -> &str { &self.0 @@ -33,6 +50,14 @@ macro_rules! entity_id { }; } +fn is_valid_entity_id(value: &str, prefix: &str) -> bool { + let Some(suffix) = value.strip_prefix(prefix).and_then(|value| value.strip_prefix('_')) else { + return false; + }; + + suffix.len() == 32 && suffix.as_bytes().iter().all(|byte| byte.is_ascii_hexdigit()) +} + entity_id!(TabId, "tab"); entity_id!(SpaceId, "space"); entity_id!(ProfileId, "profile"); @@ -43,3 +68,35 @@ entity_id!(DownloadId, "download"); entity_id!(BookmarkId, "bookmark"); entity_id!(ReadingListId, "reading"); entity_id!(NoteId, "note"); + +#[cfg(test)] +mod tests { + use super::{ProfileId, TabId}; + use crate::DomainError; + + #[test] + fn parses_existing_entity_id() -> Result<(), DomainError> { + let profile_id = ProfileId::new(); + let parsed = ProfileId::parse(profile_id.as_str())?; + + assert_eq!(parsed, profile_id); + Ok(()) + } + + #[test] + fn rejects_wrong_entity_prefix() { + let tab_id = TabId::new(); + let result = ProfileId::parse(tab_id.as_str()); + + assert!( + matches!(result, Err(DomainError::InvalidEntityId { kind, .. }) if kind == "profile") + ); + } + + #[test] + fn rejects_empty_entity_id() { + let result = ProfileId::parse(" "); + + assert_eq!(result, Err(DomainError::EmptyField { field: "profile_id" })); + } +} 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 27ebfec..c09ff36 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar.rs @@ -4,7 +4,7 @@ use std::{ time::{Duration, Instant}, }; -use ely_domain::{ProfileId, TabId}; +use ely_domain::TabId; use ely_servo_host::{ KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest, ScrollRequest, ServoHost, ServoHostError, ServoSurfaceSize, SoftwareServoHost, TouchTapRequest, @@ -51,10 +51,13 @@ enum SidecarError { } fn run_snapshot(args: SnapshotArgs) -> Result<(), SidecarError> { - let mut host = SoftwareServoHost::new(ServoSurfaceSize::new(args.width, args.height))?; + std::fs::create_dir_all(&args.profile_data_dir)?; + let mut host = SoftwareServoHost::new_with_config_dir( + ServoSurfaceSize::new(args.width, args.height), + Some(args.profile_data_dir.clone()), + )?; let tab_id = TabId::new(); - let profile_id = ProfileId::new(); - let webview_id = host.create_webview(tab_id.clone(), profile_id)?; + let webview_id = host.create_webview(tab_id.clone(), args.profile_id.clone())?; host.navigate(NavigationRequest { webview_id: webview_id.clone(), diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs index b1d0a42..bb7fa7e 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs @@ -1,6 +1,6 @@ use std::{env, num::ParseIntError, path::PathBuf}; -use ely_domain::UrlText; +use ely_domain::{ProfileId, UrlText}; use thiserror::Error; pub(super) enum SidecarCommand { @@ -9,6 +9,8 @@ pub(super) enum SidecarCommand { pub(super) struct SnapshotArgs { pub(super) url: UrlText, + pub(super) profile_id: ProfileId, + pub(super) profile_data_dir: PathBuf, pub(super) rgba_out: PathBuf, pub(super) width: u32, pub(super) height: u32, @@ -69,8 +71,8 @@ pub(super) enum SidecarArgsError { #[error("--touch-x and --touch-y must be provided together")] IncompleteTouchPoint, - #[error("rgba output path is empty")] - EmptyRgbaOutputPath, + #[error("{name} path is empty")] + EmptyPath { name: &'static str }, #[error(transparent)] Domain(#[from] ely_domain::DomainError), @@ -98,6 +100,8 @@ fn parse_snapshot_args( ) -> Result { let mut args = args.into_iter(); let mut url = None; + let mut profile_id = None; + let mut profile_data_dir = None; let mut rgba_out = None; let mut width = None; let mut height = None; @@ -116,8 +120,17 @@ fn parse_snapshot_args( while let Some(name) = args.next() { match name.as_str() { "--url" => url = Some(UrlText::parse(next_argument(&mut args, "--url")?)?), + "--profile-id" => { + profile_id = Some(ProfileId::parse(next_argument(&mut args, "--profile-id")?)?) + } + "--profile-data-dir" => { + profile_data_dir = Some(parse_path( + "--profile-data-dir", + next_argument(&mut args, "--profile-data-dir")?, + )?) + } "--rgba-out" => { - rgba_out = Some(parse_output_path(next_argument(&mut args, "--rgba-out")?)?) + rgba_out = Some(parse_path("--rgba-out", next_argument(&mut args, "--rgba-out")?)?) } "--width" => { width = Some(parse_dimension("--width", next_argument(&mut args, "--width")?)?) @@ -207,6 +220,10 @@ fn parse_snapshot_args( Ok(SnapshotArgs { url: url.ok_or(SidecarArgsError::MissingRequiredArgument { name: "--url" })?, + profile_id: profile_id + .ok_or(SidecarArgsError::MissingRequiredArgument { name: "--profile-id" })?, + profile_data_dir: profile_data_dir + .ok_or(SidecarArgsError::MissingRequiredArgument { name: "--profile-data-dir" })?, rgba_out: rgba_out .ok_or(SidecarArgsError::MissingRequiredArgument { name: "--rgba-out" })?, width: width.ok_or(SidecarArgsError::MissingRequiredArgument { name: "--width" })?, @@ -248,10 +265,81 @@ fn parse_click_coordinate(name: &'static str, value: String) -> Result().map_err(|source| SidecarArgsError::InvalidInteger { name, value, source }) } -fn parse_output_path(value: String) -> Result { +fn parse_path(name: &'static str, value: String) -> Result { if value.trim().is_empty() { - return Err(SidecarArgsError::EmptyRgbaOutputPath); + return Err(SidecarArgsError::EmptyPath { name }); } Ok(PathBuf::from(value)) } + +#[cfg(test)] +mod tests { + use std::{env, path::PathBuf}; + + use super::{SidecarArgsError, SidecarCommand, parse_command}; + use ely_domain::{DomainError, ProfileId}; + + #[test] + fn parses_snapshot_profile_identity() -> Result<(), SidecarArgsError> { + let profile_id = ProfileId::new(); + let profile_data_dir = env::temp_dir().join(profile_id.as_str()); + let command = parse_snapshot_command(&profile_id, profile_data_dir.clone())?; + + let SidecarCommand::Snapshot(args) = command; + assert_eq!(args.profile_id, profile_id); + assert_eq!(args.profile_data_dir, profile_data_dir); + Ok(()) + } + + #[test] + fn rejects_invalid_snapshot_profile_id() { + let command = parse_command( + [ + "ely_servo_sidecar", + "snapshot", + "--url", + "https://example.com", + "--profile-id", + "profile_invalid", + "--profile-data-dir", + "/tmp/profile", + "--rgba-out", + "/tmp/frame.rgba", + "--width", + "64", + "--height", + "64", + ] + .into_iter() + .map(str::to_string), + ); + + assert!(matches!( + command, + Err(SidecarArgsError::Domain(DomainError::InvalidEntityId { .. })) + )); + } + + fn parse_snapshot_command( + profile_id: &ProfileId, + profile_data_dir: PathBuf, + ) -> Result { + parse_command([ + "ely_servo_sidecar".to_string(), + "snapshot".to_string(), + "--url".to_string(), + "https://example.com".to_string(), + "--profile-id".to_string(), + profile_id.as_str().to_string(), + "--profile-data-dir".to_string(), + profile_data_dir.display().to_string(), + "--rgba-out".to_string(), + "/tmp/frame.rgba".to_string(), + "--width".to_string(), + "64".to_string(), + "--height".to_string(), + "64".to_string(), + ]) + } +} diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/report.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/report.rs index 1a61cc5..ca7adaf 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/report.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/report.rs @@ -14,6 +14,7 @@ pub(super) struct SnapshotInputChanges { #[derive(Serialize)] pub(super) struct SnapshotReport { requested_url: String, + profile_id: String, loaded_url: Option, title: Option, rgba_path: String, @@ -52,6 +53,7 @@ impl SnapshotReport { ) -> Self { Self { requested_url: args.url.as_str().to_string(), + profile_id: snapshot.profile_id().as_str().to_string(), loaded_url: snapshot.url().map(str::to_string), title: snapshot.title().map(str::to_string), rgba_path: args.rgba_out.display().to_string(), diff --git a/crates/ely_servo_host/src/lib.rs b/crates/ely_servo_host/src/lib.rs index a4c3eb7..c0455f3 100644 --- a/crates/ely_servo_host/src/lib.rs +++ b/crates/ely_servo_host/src/lib.rs @@ -7,6 +7,8 @@ mod runtime; #[cfg(feature = "servo-engine")] mod runtime_input; #[cfg(feature = "servo-engine")] +mod runtime_permissions; +#[cfg(feature = "servo-engine")] mod runtime_waker; pub use error::ServoHostError; diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index fb18009..adf9d5a 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -1,6 +1,7 @@ use std::{ cell::{Cell, RefCell}, collections::HashMap, + path::PathBuf, rc::Rc, sync::{ Arc, @@ -13,7 +14,7 @@ use std::{ use dpi::PhysicalSize; use ely_domain::{ProfileId, TabId, WebViewId}; use servo::{ - DeviceIntPoint, DeviceIntRect, DeviceIntSize, DevicePoint, DeviceVector2D, LoadStatus, + DeviceIntPoint, DeviceIntRect, DeviceIntSize, DevicePoint, DeviceVector2D, LoadStatus, Opts, RenderingContext, Scroll, Servo, ServoBuilder, WebView, WebViewBuilder, WebViewDelegate, WebViewPoint, WebViewVector, }; @@ -24,6 +25,7 @@ use crate::{ PermissionDecision, PermissionRequest, RenderedFrame, ResizeRequest, ScreenshotRequest, ScrollRequest, ServoHost, ServoHostError, TouchTapRequest, WebViewSnapshot, WebViewState, runtime_input::{send_keyboard_text, send_mouse_click, send_mouse_drag, send_touch_tap}, + runtime_permissions::{PermissionKey, PermissionStore}, runtime_waker::ServoWakeFlag, }; @@ -59,6 +61,13 @@ pub struct SoftwareServoHost { impl SoftwareServoHost { pub fn new(size: ServoSurfaceSize) -> Result { + Self::new_with_config_dir(size, None) + } + + pub fn new_with_config_dir( + size: ServoSurfaceSize, + config_dir: Option, + ) -> Result { if SERVO_RUNTIME_STARTED .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) .is_err() @@ -66,14 +75,17 @@ impl SoftwareServoHost { return Err(ServoHostError::RuntimeAlreadyStarted); } - let host = Self::new_started(size); + let host = Self::new_started(size, config_dir); if host.is_err() { SERVO_RUNTIME_STARTED.store(false, Ordering::Release); } host } - fn new_started(size: ServoSurfaceSize) -> Result { + fn new_started( + size: ServoSurfaceSize, + config_dir: Option, + ) -> Result { let rendering_context = Rc::new( servo::SoftwareRenderingContext::new(size.physical()) .map_err(|_| ServoHostError::RenderingContextUnavailable)?, @@ -81,9 +93,12 @@ impl SoftwareServoHost { rendering_context.make_current().map_err(|_| ServoHostError::RenderingContextNotCurrent)?; let wake_requested = Arc::new(AtomicBool::new(false)); - let servo = ServoBuilder::default() - .event_loop_waker(Box::new(ServoWakeFlag::new(wake_requested.clone()))) - .build(); + let mut builder = ServoBuilder::default() + .event_loop_waker(Box::new(ServoWakeFlag::new(wake_requested.clone()))); + if let Some(config_dir) = config_dir { + builder = builder.opts(Opts { config_dir: Some(config_dir), ..Opts::default() }); + } + let servo = builder.build(); Ok(Self { servo, @@ -481,18 +496,3 @@ impl WebViewDelegate for HostWebViewDelegate { } } } - -type PermissionStore = Rc>>; - -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -struct PermissionKey { - profile_id: ProfileId, - tab_id: TabId, - feature: String, -} - -impl PermissionKey { - fn new(profile_id: ProfileId, tab_id: TabId, feature: String) -> Self { - Self { profile_id, tab_id, feature } - } -} diff --git a/crates/ely_servo_host/src/runtime_permissions.rs b/crates/ely_servo_host/src/runtime_permissions.rs new file mode 100644 index 0000000..967b95c --- /dev/null +++ b/crates/ely_servo_host/src/runtime_permissions.rs @@ -0,0 +1,20 @@ +use std::{cell::RefCell, collections::HashMap, rc::Rc}; + +use ely_domain::{ProfileId, TabId}; + +use crate::PermissionDecision; + +pub(super) type PermissionStore = Rc>>; + +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub(super) struct PermissionKey { + profile_id: ProfileId, + tab_id: TabId, + feature: String, +} + +impl PermissionKey { + pub(super) fn new(profile_id: ProfileId, tab_id: TabId, feature: String) -> Self { + Self { profile_id, tab_id, feature } + } +} diff --git a/crates/ely_servo_host/tests/sidecar.rs b/crates/ely_servo_host/tests/sidecar.rs index 30f8661..ed7edf2 100644 --- a/crates/ely_servo_host/tests/sidecar.rs +++ b/crates/ely_servo_host/tests/sidecar.rs @@ -1,6 +1,8 @@ #![cfg(feature = "servo-engine")] -use std::{collections::BTreeSet, error::Error, fs, path::PathBuf}; +use std::{collections::BTreeSet, error::Error, fs, path::PathBuf, process::Command}; + +use ely_domain::ProfileId; #[path = "sidecar/support.rs"] mod support; @@ -37,10 +39,73 @@ fn sidecar_opens_and_renders_prd_sites_to_rgba_files() -> Result<(), Box Result<(), Box> { + let profile_id = ProfileId::new(); + let profile_data_dir = std::env::temp_dir().join(format!( + "ely-servo-sidecar-profile-test-{}-{}", + std::process::id(), + profile_id.as_str() + )); + let rgba_path = std::env::temp_dir().join(format!( + "ely-servo-sidecar-profile-test-{}-{}.rgba", + std::process::id(), + profile_id.as_str() + )); + + let output = Command::new(env!("CARGO_BIN_EXE_ely_servo_sidecar")) + .arg("snapshot") + .arg("--url") + .arg("data:text/html,%3Ctitle%3EProfile%20Probe%3C%2Ftitle%3EProfile%20Probe") + .arg("--profile-id") + .arg(profile_id.as_str()) + .arg("--profile-data-dir") + .arg(&profile_data_dir) + .arg("--rgba-out") + .arg(&rgba_path) + .arg("--width") + .arg("64") + .arg("--height") + .arg("64") + .output()?; + + assert!( + output.status.success(), + "stdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let report: serde_json::Value = serde_json::from_slice(&output.stdout)?; + assert_eq!( + report.get("profile_id").and_then(serde_json::Value::as_str), + Some(profile_id.as_str()) + ); + + remove_file_if_present(rgba_path)?; + remove_dir_if_present(profile_data_dir)?; + Ok(()) +} + fn prd_path() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("..").join("..").join("PRD.md") } +fn remove_file_if_present(path: PathBuf) -> Result<(), Box> { + match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error.into()), + } +} + +fn remove_dir_if_present(path: PathBuf) -> Result<(), Box> { + match fs::remove_dir_all(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error.into()), + } +} + fn prd_reference_urls(prd: &str) -> Vec { prd.lines() .filter(|line| line.starts_with("[R")) diff --git a/crates/ely_servo_host/tests/sidecar/support.rs b/crates/ely_servo_host/tests/sidecar/support.rs index 18e48ee..33eeaee 100644 --- a/crates/ely_servo_host/tests/sidecar/support.rs +++ b/crates/ely_servo_host/tests/sidecar/support.rs @@ -1,12 +1,15 @@ use std::{ error::Error, io, + path::{Path, PathBuf}, process::{Child, Command, Output, Stdio}, sync::Mutex, thread, time::{Duration, Instant}, }; +use ely_domain::ProfileId; + pub(super) const MINIMUM_CONTENT_PIXELS: u64 = 1_000; const SIDECAR_TIMEOUT: Duration = Duration::from_secs(45); const SIDECAR_POLL_INTERVAL: Duration = Duration::from_millis(20); @@ -313,7 +316,7 @@ fn snapshot_probe( fn run_sidecar_snapshot( site_url: &str, - output_path: &std::path::Path, + output_path: &Path, size: FrameSize, scroll_offset: ScrollOffset, input: SnapshotInput<'_>, @@ -321,11 +324,17 @@ fn run_sidecar_snapshot( let _guard = SIDECAR_COMMAND_LOCK .lock() .map_err(|_| io::Error::other("sidecar command lock poisoned"))?; + let profile_id = ProfileId::new(); + let profile_data_dir = temporary_profile_data_dir(&profile_id); let mut command = Command::new(env!("CARGO_BIN_EXE_ely_servo_sidecar")); command .arg("snapshot") .arg("--url") .arg(site_url) + .arg("--profile-id") + .arg(profile_id.as_str()) + .arg("--profile-data-dir") + .arg(&profile_data_dir) .arg("--rgba-out") .arg(output_path) .arg("--width") @@ -361,12 +370,14 @@ fn run_sidecar_snapshot( loop { if child.try_wait()?.is_some() { let output = child.wait_with_output()?; + remove_temporary_dir(&profile_data_dir)?; thread::sleep(SIDECAR_COMMAND_COOLDOWN); return Ok(output); } if started_at.elapsed() >= SIDECAR_TIMEOUT { terminate_child(child)?; + remove_temporary_dir(&profile_data_dir)?; thread::sleep(SIDECAR_COMMAND_COOLDOWN); return Err(format!( "timed out rendering {site_url} at {}x{}", @@ -379,6 +390,22 @@ fn run_sidecar_snapshot( } } +fn temporary_profile_data_dir(profile_id: &ProfileId) -> PathBuf { + std::env::temp_dir().join(format!( + "ely-servo-sidecar-profile-{}-{}", + std::process::id(), + profile_id.as_str() + )) +} + +fn remove_temporary_dir(path: &Path) -> Result<(), Box> { + match std::fs::remove_dir_all(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error.into()), + } +} + fn run_sidecar_snapshot_with_retry( site_url: &str, output_path: &std::path::Path,