From 452cc8554fdeb3a059baeaf81692d6a5dc285e7e Mon Sep 17 00:00:00 2001
From: Andrey Mnatsakanov <andrey@3ig.dev>
Date: Tue, 4 Aug 2026 17:40:19 +0200
Subject: [PATCH] fix(js): run string timer handlers as global-scope scripts at
 fire time

A string setTimeout/setInterval handler was compiled with new Function(fn) at
scheduling time. That diverged from the HTML spec in two ways: top-level
var/function declarations in the body stayed local to the compiled function
instead of becoming globals, and a syntax error was caught eagerly at
scheduling and turned into a silent no-op (that still returned a timer id)
rather than surfacing when the timer fires.

Defer string handlers to fire-time indirect eval ((0, eval)(src)), which runs
in the true global scope like a classic script, so declarations become global
and a SyntaxError is thrown when the timer elapses. Matches the dynamic-script
path, which already uses indirect eval for the same reason. Function handlers
are unchanged.

Fixes #558
---
 crates/obscura-js/js/bootstrap.js | 10 +++++++++-
 crates/obscura-js/src/runtime.rs  | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js
index 4ffed6c..4cd6739 100644
--- a/crates/obscura-js/js/bootstrap.js
+++ b/crates/obscura-js/js/bootstrap.js
@@ -481,7 +481,15 @@ const _scheduleAfter = (delay, fn) => {
 // call silently no-ops and JS-triggered navigations (cookie → reload) never fire.
 const _coerceTimerFn = (fn) => {
   if (typeof fn === "string") {
-    try { return new Function(fn); } catch (_) { return null; }
+    // Per HTML, a string handler is compiled and run as a classic script in
+    // global scope *at fire time*. Indirect eval ((0, eval)) runs in the true
+    // global scope, so top-level var/function declarations become globals (a
+    // `new Function(fn)` wrapper kept them local); deferring to fire time also
+    // surfaces a SyntaxError when the timer elapses, matching a real browser,
+    // instead of swallowing it eagerly at scheduling. The dynamic-script path
+    // uses the same indirect eval for the same reason.
+    const src = fn;
+    return () => { (0, eval)(src); };
   }
   return typeof fn === "function" ? fn : null;
 };
diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs
index 03a12fc..92a900d 100644
--- a/crates/obscura-js/src/runtime.rs
+++ b/crates/obscura-js/src/runtime.rs
@@ -1330,6 +1330,26 @@ mod tests {
         );
     }
 
+    #[tokio::test(flavor = "current_thread")]
+    async fn string_timeout_declarations_reach_global_scope() {
+        // A string timer handler runs as a classic script in global scope, so a
+        // top-level var/function declaration in it becomes a global. new Function()
+        // kept those declarations local to the compiled function, so they never
+        // reached globalThis.
+        let mut rt = setup_runtime("<html><body></body></html>");
+        rt.evaluate(
+            "setTimeout('var __leaked = 42; function __leakedFn(){ return 7; }', 0)",
+        )
+        .unwrap();
+        rt.run_event_loop_bounded(100).await.unwrap();
+        let v = rt
+            .evaluate(
+                "String(globalThis.__leaked) + '|' + (typeof globalThis.__leakedFn === 'function' ? globalThis.__leakedFn() : 'missing')",
+            )
+            .unwrap();
+        assert_eq!(v, serde_json::json!("42|7"));
+    }
+
     #[tokio::test(flavor = "current_thread")]
     async fn string_interval_handler_repeats_and_can_clear_itself() {
         let mut rt = setup_runtime("<html><body></body></html>");