From 6b3cc1a686bc46f17c149a85ca56eb1ae197776f Mon Sep 17 00:00:00 2001 From: SGavrl <277843151+SGavrl@users.noreply.github.com> Date: Wed, 3 Jun 2026 18:16:20 +0000 Subject: [PATCH] Fix/rescue 227 regressions (#232) * fix: restore regressions introduced by #227 #227 deleted build.rs (reverting #224's version-from-tag feature), removed DOM cycle guards in append_child/insert_before that prevent infinite loops on self-append/self-insert, dropped the CharacterData textContent fast-path, removed PI/Doctype Rust ops, and cut the env_allows_private_network() check from validate_fetch_url. Restored: - crates/obscura-cli/build.rs (version from tag/env) - OBSCURA_BUILD_VERSION in version string and banner - allow_private_network env var propagation in main.rs - append_child/insert_before self-cycle guards in tree.rs - textContent CharacterData fast-path in tree.rs - create_processing_instruction, create_doctype, pi_target, doctype_name, doctype_public_id ops in ops.rs - ProcessingInstruction branch in set_text_content op - env_allows_private_network() check in validate_fetch_url * fix(js): Europe/Berlin timezone, plugins iterator recursion Two bugs in the stealth bootstrap code from #227: - 'Europe/Germany' is not a valid IANA timezone; Intl.DateTimeFormat silently fell back to UTC. Changed to 'Europe/Berlin'. - plugins[Symbol.iterator] called this[Symbol.iterator]() recursively, blowing the stack on Array.from(navigator.plugins). Fixed to bind Array.prototype[Symbol.iterator] directly to the array. --- crates/obscura-cli/build.rs | 36 +++++++++++++++++++++++++ crates/obscura-cli/src/main.rs | 17 +++++++++--- crates/obscura-dom/src/tree.rs | 35 ++++++++++++++++++++++++ crates/obscura-js/js/bootstrap.js | 4 +-- crates/obscura-js/src/ops.rs | 44 ++++++++++++++++++++++++++++++- 5 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 crates/obscura-cli/build.rs diff --git a/crates/obscura-cli/build.rs b/crates/obscura-cli/build.rs new file mode 100644 index 0000000..89b864d --- /dev/null +++ b/crates/obscura-cli/build.rs @@ -0,0 +1,36 @@ +use std::env; + +fn env_value(name: &str) -> Option { + env::var(name) + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) +} + +fn normalize_version(version: String) -> String { + version.strip_prefix('v').unwrap_or(&version).to_string() +} + +fn github_tag_version() -> Option { + if env_value("GITHUB_REF_TYPE").as_deref() == Some("tag") { + return env_value("GITHUB_REF_NAME").map(normalize_version); + } + + env_value("GITHUB_REF") + .and_then(|value| value.strip_prefix("refs/tags/").map(str::to_owned)) + .map(normalize_version) +} + +fn main() { + println!("cargo:rerun-if-env-changed=OBSCURA_VERSION"); + println!("cargo:rerun-if-env-changed=GITHUB_REF_TYPE"); + println!("cargo:rerun-if-env-changed=GITHUB_REF_NAME"); + println!("cargo:rerun-if-env-changed=GITHUB_REF"); + + let version = env_value("OBSCURA_VERSION") + .map(normalize_version) + .or_else(github_tag_version) + .unwrap_or_else(|| env::var("CARGO_PKG_VERSION").expect("Cargo sets CARGO_PKG_VERSION")); + + println!("cargo:rustc-env=OBSCURA_BUILD_VERSION={version}"); +} diff --git a/crates/obscura-cli/src/main.rs b/crates/obscura-cli/src/main.rs index 96cd5ea..67596be 100644 --- a/crates/obscura-cli/src/main.rs +++ b/crates/obscura-cli/src/main.rs @@ -10,7 +10,7 @@ use tokio::time::{timeout, Duration}; #[derive(Parser)] #[command( name = "obscura", - version = env!("CARGO_PKG_VERSION"), + version = env!("OBSCURA_BUILD_VERSION"), about = "Obscura - A lightweight headless browser for web scraping and automation", )] struct Args { @@ -189,9 +189,9 @@ fn print_banner(port: u16) { | |__| | |_) \__ \ (__| |_| | | | (_| | \____/|_.__/|___/\___|\__,_|_| \__,_| - Headless Browser v0.1.6 + Headless Browser v{} CDP server: ws://127.0.0.1:{}/devtools/browser -"#, port); +"#, env!("OBSCURA_BUILD_VERSION"), port); } fn select_log_filter(verbose: bool, quiet: bool) -> &'static str { @@ -263,6 +263,17 @@ async fn main() -> anyhow::Result<()> { tracing::debug!("V8 flags: {}", v8_flags); obscura_js::set_v8_flags(&v8_flags); + // The js-side fetch path (op_fetch_url) reads OBSCURA_ALLOW_PRIVATE_NETWORK + // directly for its SSRF gate. Mirror the CLI flag into the env var so + // iframe loads and JS fetch() see the same policy the http_client layer + // already uses (issue #33). + if args.allow_private_network { + // SAFETY: set_var is unsafe in newer rustc; this runs before any + // spawned thread inspects the env, so it's effectively single + // threaded at this point. + unsafe { std::env::set_var("OBSCURA_ALLOW_PRIVATE_NETWORK", "1"); } + } + let global_proxy = args.proxy.clone(); match args.command { diff --git a/crates/obscura-dom/src/tree.rs b/crates/obscura-dom/src/tree.rs index 0b93abb..20d0b38 100644 --- a/crates/obscura-dom/src/tree.rs +++ b/crates/obscura-dom/src/tree.rs @@ -223,6 +223,14 @@ impl DomTree { } pub fn append_child(&self, parent_id: NodeId, child_id: NodeId) { + // Per DOM spec, appending a node to itself is a HierarchyRequestError; + // here we treat it as a no-op rather than panic. Without this the + // sibling-pointer fixup below sets the node's prev_sibling to itself + // and every later child-walk loops forever (same failure mode that + // insert_before's self-cycle guard was added to prevent). + if parent_id == child_id { + return; + } self.detach(child_id); let mut inner = self.inner.borrow_mut(); @@ -252,6 +260,15 @@ impl DomTree { } pub fn insert_before(&self, existing_id: NodeId, new_sibling_id: NodeId) { + // Per DOM spec: if the node being inserted IS the reference node, + // the operation is a no-op (the node is already in its target + // position). Without this, the linked-list fixup below sets the + // node's prev_sibling and next_sibling to itself, creating a cycle + // -- every later traversal (childNodes, querySelectorAll, etc) then + // loops forever and the test page hangs while obscura burns RAM. + if existing_id == new_sibling_id { + return; + } let (parent_id, prev_id) = { let inner = self.inner.borrow(); let node = match inner.nodes.get(existing_id.index()).and_then(|n| n.as_ref()) { @@ -463,6 +480,20 @@ impl DomTree { pub fn text_content(&self, node_id: NodeId) -> String { let inner = self.inner.borrow(); + // Per DOM spec, calling textContent ON a CharacterData node + // (Text, Comment, ProcessingInstruction) returns its .data. + // Calling textContent on an Element walks descendants and + // concatenates Text node content only (Comment + PI are + // skipped). Handle the direct-CharacterData case here so the + // descent helper can keep its element-centric behavior. + if let Some(Some(node)) = inner.nodes.get(node_id.index()) { + match &node.data { + NodeData::Text { contents } => return contents.clone(), + NodeData::Comment { contents } => return contents.clone(), + NodeData::ProcessingInstruction { data, .. } => return data.clone(), + _ => {} + } + } let mut result = String::new(); collect_text_inner(&inner, node_id, &mut result); result @@ -570,6 +601,10 @@ fn collect_text_inner(inner: &DomTreeInner, node_id: NodeId, buf: &mut String) { if let Some(Some(node)) = inner.nodes.get(node_id.index()) { match &node.data { NodeData::Text { contents } => buf.push_str(contents), + // Comment and ProcessingInstruction are intentionally NOT + // appended when traversing descendants: per spec, textContent + // on an Element only includes Text descendants. Direct + // textContent on a Comment/PI is handled by the caller. _ => { let mut child = node.first_child; while let Some(child_id) = child { diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index a1713c4..fd29331 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -1842,7 +1842,7 @@ globalThis.navigator = { p.item = (i) => p[i] || null; p.namedItem = (name) => p.find(x => x.name === name) || null; p.refresh = () => {}; - p[Symbol.iterator] = function() { return this[Symbol.iterator](); }; + p[Symbol.iterator] = Array.prototype[Symbol.iterator].bind(p); return p; }, get mimeTypes() { @@ -4312,7 +4312,7 @@ _markNative(MediaStream); _markNative(MediaStreamTrack); _markNative(RTCPeerConnection); _markNative(RTCSessionDescription); _markNative(RTCIceCandidate); const _OrigDateTimeFormat = Intl.DateTimeFormat; -const _defaultTZ = 'Europe/Germany'; +const _defaultTZ = 'Europe/Berlin'; Intl.DateTimeFormat = function(locales, options) { if (!options) options = {}; if (!options.timeZone) options.timeZone = _defaultTZ; diff --git a/crates/obscura-js/src/ops.rs b/crates/obscura-js/src/ops.rs index 259165c..86c2e37 100644 --- a/crates/obscura-js/src/ops.rs +++ b/crates/obscura-js/src/ops.rs @@ -259,6 +259,7 @@ fn op_dom(state: &OpState, #[string] cmd: String, #[string] arg1: String, #[stri match &mut n.data { NodeData::Text { contents } => { *contents = arg2.clone(); } NodeData::Comment { contents } => { *contents = arg2.clone(); } + NodeData::ProcessingInstruction { data, .. } => { *data = arg2.clone(); } _ => {} } }); @@ -279,6 +280,47 @@ fn op_dom(state: &OpState, #[string] cmd: String, #[string] arg1: String, #[stri "create_comment_node" => { dom.new_node(NodeData::Comment { contents: arg1.clone() }).index().to_string() } + "create_processing_instruction" => { + // arg1 = target, arg2 = data + dom.new_node(NodeData::ProcessingInstruction { + target: arg1.clone(), + data: arg2.clone(), + }).index().to_string() + } + "create_doctype" => { + // arg1 = name, arg2 = public_id. system_id stored only in the + // JS wrapper since neither current WPT test reads it back from + // the underlying tree. + dom.new_node(NodeData::Doctype { + name: arg1.clone(), + public_id: arg2.clone(), + system_id: String::new(), + }).index().to_string() + } + "pi_target" => { + let nid = arg1.parse::().unwrap_or(0); + let val = dom.get_node(NodeId::new(nid)).and_then(|n| match &n.data { + NodeData::ProcessingInstruction { target, .. } => Some(target.clone()), + _ => None, + }).unwrap_or_default(); + serde_json::to_string(&val).unwrap_or("\"\"".into()) + } + "doctype_name" => { + let nid = arg1.parse::().unwrap_or(0); + let val = dom.get_node(NodeId::new(nid)).and_then(|n| match &n.data { + NodeData::Doctype { name, .. } => Some(name.clone()), + _ => None, + }).unwrap_or_default(); + serde_json::to_string(&val).unwrap_or("\"\"".into()) + } + "doctype_public_id" => { + let nid = arg1.parse::().unwrap_or(0); + let val = dom.get_node(NodeId::new(nid)).and_then(|n| match &n.data { + NodeData::Doctype { public_id, .. } => Some(public_id.clone()), + _ => None, + }).unwrap_or_default(); + serde_json::to_string(&val).unwrap_or("\"\"".into()) + } "element_children" => { let nid = arg1.parse::().unwrap_or(0); let ids: Vec = dom.children(NodeId::new(nid)).iter() @@ -726,7 +768,7 @@ fn validate_fetch_url(url: &url::Url) -> Result<(), String> { )); } - if scheme == "file" { + if scheme == "file" || obscura_net::env_allows_private_network() { return Ok(()); }