diff --git a/crates/obscura-browser/src/page.rs b/crates/obscura-browser/src/page.rs index f0e4f0a..92a4e19 100644 --- a/crates/obscura-browser/src/page.rs +++ b/crates/obscura-browser/src/page.rs @@ -240,6 +240,11 @@ pub struct Page { /// It is reset once author styles are installed, so stylesheet download /// latency does not incorrectly advance newly-created animations. document_timeline_origin: std::time::Instant, + /// Optional page-scoped ceiling for an end-to-end navigation. Automation + /// frontends set this from their request timeout so a caller asking for a + /// 50-second navigation is not silently cut off by the process default. + /// Pages without an override retain the environment-configurable default. + navigation_timeout: Option, /// Navigation history for Page.getNavigationHistory / navigateToHistoryEntry. /// Entries are URLs in visit order; `history_index` is the current position. /// Pushed on every successful navigation; truncated on goBack -> new nav. @@ -272,6 +277,22 @@ pub struct Page { const MAX_STYLESHEET_IMPORT_DEPTH: u8 = 4; const MAX_STYLESHEET_RESOURCES: usize = 128; +const DEFAULT_NAVIGATION_TIMEOUT_MS: u64 = 30_000; + +fn default_navigation_timeout() -> std::time::Duration { + navigation_timeout_from_env_value(std::env::var("OBSCURA_NAV_TIMEOUT_MS").ok().as_deref()) +} + +fn navigation_timeout_from_env_value(value: Option<&str>) -> std::time::Duration { + let milliseconds = value + .and_then(|value| value.parse().ok()) + .unwrap_or(DEFAULT_NAVIGATION_TIMEOUT_MS); + std::time::Duration::from_millis(milliseconds) +} + +fn duration_millis_u64(duration: std::time::Duration) -> u64 { + duration.as_millis().min(u128::from(u64::MAX)) as u64 +} #[derive(Clone)] struct LoadedStylesheet { @@ -863,6 +884,7 @@ impl Page { default_background_color_override: None, encoding: "UTF-8".to_string(), document_timeline_origin: std::time::Instant::now(), + navigation_timeout: None, history: Vec::new(), history_index: 0, network_events: Vec::new(), @@ -881,6 +903,19 @@ impl Page { } } + /// Set the end-to-end navigation deadline for this page. This page-scoped + /// value takes precedence over `OBSCURA_NAV_TIMEOUT_MS`; callers that do + /// not set it retain the existing environment-configurable 30s default. + pub fn set_navigation_timeout(&mut self, timeout: std::time::Duration) { + self.navigation_timeout = Some(timeout); + } + + /// Return the effective end-to-end navigation deadline for this page. + pub fn navigation_timeout(&self) -> std::time::Duration { + self.navigation_timeout + .unwrap_or_else(default_navigation_timeout) + } + fn should_block_url(&self, url: &str) -> bool { for pattern in &self.blocked_url_patterns { if url_matches_cdp_pattern(pattern, url) { @@ -2125,12 +2160,10 @@ impl Page { // dispatcher holds the lock across the entire handler. 30 seconds // matches reqwest's default per-request timeout — the worst case is // one slow primary GET plus one slow JS-redirect chain step. Override - // with `OBSCURA_NAV_TIMEOUT_MS=NN`. - let nav_timeout_ms: u64 = std::env::var("OBSCURA_NAV_TIMEOUT_MS") - .ok() - .and_then(|s| s.parse().ok()) - .unwrap_or(30_000); - let nav_timeout = tokio::time::Duration::from_millis(nav_timeout_ms); + // with `OBSCURA_NAV_TIMEOUT_MS=NN`, or set a page-scoped deadline when + // the automation request already has an explicit timeout. + let nav_timeout = self.navigation_timeout(); + let nav_timeout_ms = duration_millis_u64(nav_timeout); let result = match tokio::time::timeout( nav_timeout, @@ -3482,12 +3515,10 @@ impl Page { .map(|target| navigation_referrer(source, &target)) }) .unwrap_or_default(); - let nav_timeout_ms: u64 = std::env::var("OBSCURA_NAV_TIMEOUT_MS") - .ok() - .and_then(|s| s.parse().ok()) - .unwrap_or(30_000); + let nav_timeout = self.navigation_timeout(); + let nav_timeout_ms = duration_millis_u64(nav_timeout); let result = tokio::time::timeout( - tokio::time::Duration::from_millis(nav_timeout_ms), + nav_timeout, self.navigate_with_wait_post_inner( &url, crate::lifecycle::WaitUntil::Load, @@ -3562,15 +3593,35 @@ fn url_matches_cdp_pattern(pattern: &str, url: &str) -> bool { mod tests { use super::{ css_resource_urls, linked_stylesheet_requests, materialize_linked_stylesheet_script, - materialize_stylesheet_graph, navigation_referrer, parse_import_url, rebase_css_urls, - script_response_is_executable, split_css_imports, truncate_on_char_boundary, - url_matches_cdp_pattern, LoadedStylesheet, StylesheetImport, + materialize_stylesheet_graph, navigation_referrer, navigation_timeout_from_env_value, + parse_import_url, rebase_css_urls, script_response_is_executable, split_css_imports, + truncate_on_char_boundary, url_matches_cdp_pattern, LoadedStylesheet, StylesheetImport, }; #[cfg(feature = "render")] use super::remaining_settle_resource_warmup_ms; use base64::Engine as _; use obscura_dom::parse_html; + #[test] + fn navigation_timeout_environment_default_remains_thirty_seconds() { + assert_eq!( + navigation_timeout_from_env_value(None), + std::time::Duration::from_secs(30) + ); + assert_eq!( + navigation_timeout_from_env_value(Some("not-a-timeout")), + std::time::Duration::from_secs(30) + ); + } + + #[test] + fn navigation_timeout_environment_override_remains_available() { + assert_eq!( + navigation_timeout_from_env_value(Some("42000")), + std::time::Duration::from_secs(42) + ); + } + #[test] fn css_resource_discovery_ignores_strings_comments_data_and_fragments() { let base = url::Url::parse("https://example.test/css/app/main.css").unwrap(); diff --git a/crates/obscura-cli/src/main.rs b/crates/obscura-cli/src/main.rs index 1106f04..e7ff681 100644 --- a/crates/obscura-cli/src/main.rs +++ b/crates/obscura-cli/src/main.rs @@ -667,6 +667,10 @@ async fn settle_page(page: &mut Page, wait_secs: u64, fixed: bool) { } } +fn configure_fetch_navigation_timeout(page: &mut Page, timeout_secs: u64) { + page.set_navigation_timeout(Duration::from_secs(timeout_secs)); +} + async fn run_fetch( url_str: &str, dump: Option, @@ -710,6 +714,10 @@ async fn run_fetch( allow_private_network, )); let mut page = Page::new("fetch-page".to_string(), context.clone()); + // Keep the browser's end-to-end navigation ceiling aligned with the CLI + // request deadline. Previously Page retained its independent 30s default, + // so `fetch --timeout 50` could still fail after 30 seconds. + configure_fetch_navigation_timeout(&mut page, timeout_secs); // A screenshot viewport is also the navigation viewport: responsive // frameworks must build the DOM for the same dimensions we later paint. // Previously page JS saw a randomized screen-sized innerWidth while the @@ -1868,10 +1876,10 @@ fn dump_assets(page: &Page) -> String { #[cfg(test)] mod tests { use super::{ - effective_v8_flags, extract_assets, extract_readable_text, fetch_original_bytes, - is_quiet_command, link_kind_from_rel, merge_proxy, normalize_v8_flags, read_urls_from_file, - resolve_asset_url, select_log_filter, write_or_print, write_or_print_bytes, Args, Command, - DumpFormat, DEFAULT_V8_FLAGS, + configure_fetch_navigation_timeout, effective_v8_flags, extract_assets, + extract_readable_text, fetch_original_bytes, is_quiet_command, link_kind_from_rel, + merge_proxy, normalize_v8_flags, read_urls_from_file, resolve_asset_url, select_log_filter, + write_or_print, write_or_print_bytes, Args, Command, DumpFormat, DEFAULT_V8_FLAGS, }; use clap::Parser; use obscura_dom::parse_html; @@ -2226,6 +2234,51 @@ mod tests { } } + fn configured_fetch_timeout(args: Args) -> std::time::Duration { + let timeout = match args.command { + Some(Command::Fetch { timeout, .. }) => timeout, + _ => panic!("expected Fetch command"), + }; + let context = std::sync::Arc::new( + obscura_browser::BrowserContext::with_storage_and_network( + "cli-timeout-test".to_string(), + None, + false, + None, + None, + true, + ), + ); + let mut page = obscura_browser::Page::new("cli-timeout-test".to_string(), context); + configure_fetch_navigation_timeout(&mut page, timeout); + page.navigation_timeout() + } + + #[test] + fn fetch_timeout_sets_the_page_navigation_budget() { + let args = Args::try_parse_from([ + "obscura", + "fetch", + "https://example.com", + "--timeout", + "50", + ]) + .unwrap(); + assert_eq!( + configured_fetch_timeout(args), + std::time::Duration::from_secs(50) + ); + } + + #[test] + fn fetch_default_navigation_budget_remains_thirty_seconds() { + let args = Args::try_parse_from(["obscura", "fetch", "https://example.com"]).unwrap(); + assert_eq!( + configured_fetch_timeout(args), + std::time::Duration::from_secs(30) + ); + } + #[test] fn matcher_still_uses_fetch_variant() { let cmd = Some(Command::Fetch {