From 7d66e37bf680bc59e9598c471a6993a88accc58d Mon Sep 17 00:00:00 2001
From: meh <no-reply@pathscale.com>
Date: Mon, 31 Aug 2026 17:50:51 +0700
Subject: [PATCH] fix(capture): fetch the scripts a page discovers while
 running

The prefetch walks the parsed HTML and loads every script it names. A page
that builds a `<script>` from JavaScript asks for a URL nobody knew about
until the page was already running, and that request reached
`DefaultScriptFetcher`, which serves `file:` and `data:` only. The script was
dropped with `unsupported URL scheme for script: https`, which reads like a
policy decision rather than the missing capability it is.

Measured over a hundred-site corpus, this is the single most common engine
defect: **26 sites**, a quarter of the corpus. It hides well, because scripts
the parser found load perfectly, so a page fails only in the parts it
assembles itself, and jQuery going undefined on seven sites looks like its own
bug rather than a consequence of never having been fetched.

`ScriptFetcher::fetch` is synchronous, because classic scripts execute in
document order, so the network call blocks. It runs on the page's own provider
and so keeps the per-origin connection cap the rest of the loads obey, and it
is bounded by a timeout: a server that accepts and never answers would
otherwise hang the capture, and a missing script is better than a run that
never ends.

Verified by re-capturing the six worst-affected sites: dropped scripts went
12, 6, 4, 2, 2, 2 to zero everywhere. Rendering barely moved, which is the
honest result rather than a disappointing one. On one site the error count
rose from 9 to 14 with a new kind, `unescape is not defined`: the scripts now
run and reach the *next* missing global. This unblocks a layer; the missing
web APIs behind it are what turn that into pixels.

The test serves a script from a real socket and asserts a synchronous fetch
completes a round trip from inside the runtime driving the page. Its accept
loop has a deadline rather than blocking: restoring the defect makes no
connection at all, and a blocking accept hangs the run instead of failing it.
Confirmed by restoring the defect, which fails the test in ten seconds.

Only the capture path. `browser.rs` has the same fetcher and is reached from a
synchronous poll with no runtime handle to hand, so it needs a stored handle
and is left for its own change.
---
 apps/chuzz/Cargo.toml             |   4 +
 apps/chuzz/src/document_loader.rs | 202 +++++++++++++++++++++++++++++-
 2 files changed, 200 insertions(+), 6 deletions(-)

diff --git a/apps/chuzz/Cargo.toml b/apps/chuzz/Cargo.toml
index b26a7f5..e8720a0 100644
--- a/apps/chuzz/Cargo.toml
+++ b/apps/chuzz/Cargo.toml
@@ -116,6 +116,10 @@ tauri.workspace = true
 tauri-runtime-blitz.workspace = true
 
 [dev-dependencies]
+# `flavor = "multi_thread"` in the script-fetch test: the blocking fetch takes
+# the `block_in_place` path, which only the multi-threaded runtime has, and
+# that is the path the capture actually uses.
+tokio = { workspace = true, features = ["macros", "rt", "rt-multi-thread", "sync", "time"] }
 # Assembles `fixtures/panel.wat` into a module in-process, so the capture test
 # owns its guest instead of borrowing blitz-wasm's demo, and so it needs no
 # wasm32 target and no nested cargo build. See the header of that file.
diff --git a/apps/chuzz/src/document_loader.rs b/apps/chuzz/src/document_loader.rs
index b29b79d..9ac7725 100644
--- a/apps/chuzz/src/document_loader.rs
+++ b/apps/chuzz/src/document_loader.rs
@@ -312,20 +312,100 @@ pub(crate) const WEB_API_SHIM: &str = r#"
   }
 })();
 "#;
-/// Serves the sources prefetched above, falling back to the default fetcher
-/// for `file:` and `data:` URLs.
+/// Serves the sources prefetched above, and fetches the rest from the network.
+///
+/// The prefetch can only see the scripts the parsed HTML names. A page that
+/// builds a `<script>` from JavaScript, or imports a module, asks for a URL
+/// nobody knew about until the page was already running, and that request used
+/// to reach `DefaultScriptFetcher`, which serves `file:` and `data:` only. The
+/// script was dropped with `unsupported URL scheme for script: https`, which
+/// reads like a policy decision rather than the missing feature it is. It cost
+/// a quarter of a hundred-site corpus, and it hides well: scripts the parser
+/// found load perfectly, so a page fails only in the parts it assembles itself.
+///
+/// `ScriptFetcher::fetch` is synchronous because classic scripts must execute
+/// in document order, so the network call has to block. It runs on the shared
+/// provider, which keeps the per-origin connection cap that the rest of the
+/// page's loads obey.
 #[cfg(all(feature = "capture", feature = "javascript"))]
-struct PrefetchedScripts {
+struct PageScripts {
     scripts: HashMap<Url, String>,
+    net: Arc<NetProvider>,
+    runtime: tokio::runtime::Handle,
 }
 
+/// How long a runtime-discovered script may take before the page moves on.
+///
+/// Bounded because this blocks script execution: a server that accepts the
+/// connection and never answers would otherwise hang the whole capture, and a
+/// missing script is a far better outcome than a run that never finishes.
+#[cfg(all(feature = "capture", feature = "javascript"))]
+const RUNTIME_SCRIPT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
+
 #[cfg(all(feature = "capture", feature = "javascript"))]
-impl blitz_script::ScriptFetcher for PrefetchedScripts {
+impl blitz_script::ScriptFetcher for PageScripts {
     fn fetch(&self, url: &Url) -> Result<String, blitz_script::FetchError> {
         if let Some(source) = self.scripts.get(url) {
             return Ok(source.clone());
         }
-        blitz_script::DefaultScriptFetcher.fetch(url)
+        match url.scheme() {
+            "http" | "https" => self.fetch_over_network(url),
+            // `file:` and `data:` still belong to the default fetcher; it
+            // decodes data URLs correctly and needs no network.
+            _ => blitz_script::DefaultScriptFetcher.fetch(url),
+        }
+    }
+}
+
+#[cfg(all(feature = "capture", feature = "javascript"))]
+impl PageScripts {
+    fn fetch_over_network(&self, url: &Url) -> Result<String, blitz_script::FetchError> {
+        let net = Arc::clone(&self.net);
+        let target = url.clone();
+        let fetch = async move {
+            tokio::time::timeout(
+                RUNTIME_SCRIPT_TIMEOUT,
+                net.fetch_async(Request::get(target)),
+            )
+            .await
+        };
+
+        // This is called from inside the runtime that is driving the page, so
+        // blocking the thread outright would stall the executor it is waiting
+        // on. `block_in_place` hands the other tasks to a sibling worker first;
+        // it exists only on the multi-threaded runtime, hence the flavour test
+        // rather than an unwrap that would be right until someone builds a
+        // current-thread one.
+        let joined = match tokio::runtime::Handle::try_current() {
+            Ok(handle) if handle.runtime_flavor() == tokio::runtime::RuntimeFlavor::MultiThread => {
+                tokio::task::block_in_place(|| handle.block_on(fetch))
+            }
+            // Not on a runtime at all: the stored handle drives it, and
+            // blocking this thread stalls nothing that is waiting on us.
+            Err(_) => self.runtime.block_on(fetch),
+            // On a current-thread runtime there is one worker and it is this
+            // one, so blocking it and re-entering it both deadlock. Refusing
+            // the script drops it exactly as this code path did before, which
+            // is bad but survivable; a capture that never returns is not.
+            // Unreachable in the shipped configuration, where the capture
+            // runtime is multi-threaded, and spelled out rather than left to
+            // an unwrap that would be correct only until someone changed the
+            // builder.
+            Ok(_) => {
+                return Err(blitz_script::FetchError::InvalidData(
+                    "cannot fetch a script from a current-thread runtime".to_owned(),
+                ));
+            }
+        };
+
+        match joined {
+            Ok(Ok((_, bytes))) => Ok(decode_body(&bytes)),
+            Ok(Err(error)) => Err(blitz_script::FetchError::InvalidData(format!("{error:?}"))),
+            Err(_) => Err(blitz_script::FetchError::InvalidData(format!(
+                "script fetch timed out after {}s",
+                RUNTIME_SCRIPT_TIMEOUT.as_secs()
+            ))),
+        }
     }
 }
 
@@ -403,7 +483,11 @@ pub async fn load_for_capture(
                 scripts.insert(url, decode_body(&bytes));
             }
         }
-        let mut document = document.with_fetcher(PrefetchedScripts { scripts });
+        let mut document = document.with_fetcher(PageScripts {
+            scripts,
+            net: Arc::clone(&net_provider),
+            runtime: tokio::runtime::Handle::current(),
+        });
         document.eval(WEB_API_SHIM);
         document.execute_scripts();
         // Pump the script runtime until the page has built its DOM, then keep
@@ -473,3 +557,109 @@ impl CapturedDocument {
         }
     }
 }
+
+#[cfg(all(test, feature = "capture", feature = "javascript"))]
+mod tests {
+    use super::*;
+
+    /// A script the page asks for at runtime is fetched, not refused.
+    ///
+    /// The prefetch map is deliberately empty: that is the state a page reaches
+    /// when its own JavaScript builds a `<script>` for a URL the parser never
+    /// saw. Before this path existed the request fell through to a fetcher that
+    /// serves `file:` and `data:` only, and the script was dropped.
+    ///
+    /// Served from a real socket rather than a stub fetcher, because the thing
+    /// worth proving is that a synchronous `fetch` can complete a network round
+    /// trip from inside the runtime that is driving the page. A stub would pass
+    /// without ever testing that.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn a_script_discovered_at_runtime_is_fetched_over_the_network() {
+        use std::io::{Read, Write};
+
+        const SOURCE: &str = "globalThis.__loaded = 42;";
+
+        let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("loopback is available");
+        let port = listener
+            .local_addr()
+            .expect("the socket has an address")
+            .port();
+        // Non-blocking with a deadline rather than a plain `accept`. If the
+        // fetcher regresses to refusing the URL no connection is ever made, and
+        // a blocking accept would hang this test forever instead of failing it.
+        // A test that wedges on the very defect it exists to catch is worse
+        // than no test: it stops CI rather than reporting.
+        listener
+            .set_nonblocking(true)
+            .expect("the listener takes a nonblocking mode");
+        let server = std::thread::spawn(move || {
+            let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
+            let mut stream = loop {
+                match listener.accept() {
+                    Ok((stream, _)) => break stream,
+                    Err(ref error) if error.kind() == std::io::ErrorKind::WouldBlock => {
+                        if std::time::Instant::now() >= deadline {
+                            return false;
+                        }
+                        std::thread::sleep(std::time::Duration::from_millis(20));
+                    }
+                    Err(_) => return false,
+                }
+            };
+            stream
+                .set_nonblocking(false)
+                .expect("the accepted stream returns to blocking reads");
+            let mut seen = Vec::new();
+            let mut byte = [0u8; 1];
+            while !seen.ends_with(b"\r\n\r\n") {
+                match stream.read(&mut byte) {
+                    Ok(0) | Err(_) => break,
+                    Ok(_) => seen.push(byte[0]),
+                }
+            }
+            let response = format!(
+                "HTTP/1.1 200 OK\r\nContent-Type: text/javascript\r\nContent-Length: {}\r\n\r\n{}",
+                SOURCE.len(),
+                SOURCE
+            );
+            let _ = stream.write_all(response.as_bytes());
+            let _ = stream.flush();
+            true
+        });
+
+        let fetcher = PageScripts {
+            scripts: HashMap::new(),
+            net: Arc::new(NetProvider::new(None)),
+            runtime: tokio::runtime::Handle::current(),
+        };
+        let url = Url::parse(&format!("http://127.0.0.1:{port}/late.js")).expect("a valid URL");
+
+        let fetched =
+            tokio::task::spawn_blocking(move || blitz_script::ScriptFetcher::fetch(&fetcher, &url))
+                .await
+                .expect("the blocking fetch does not panic");
+
+        let connected = server.join().expect("the server thread finishes");
+        assert!(connected, "the fetcher never opened a connection");
+        assert_eq!(
+            fetched.expect("a runtime-discovered script is fetched"),
+            SOURCE
+        );
+    }
+
+    /// A `data:` script still resolves without touching the network.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn a_data_url_script_still_resolves_locally() {
+        let fetcher = PageScripts {
+            scripts: HashMap::new(),
+            net: Arc::new(NetProvider::new(None)),
+            runtime: tokio::runtime::Handle::current(),
+        };
+        // "let x = 1" as base64.
+        let url = Url::parse("data:text/javascript;base64,bGV0IHggPSAx").expect("a valid data URL");
+
+        let fetched = blitz_script::ScriptFetcher::fetch(&fetcher, &url);
+
+        assert_eq!(fetched.expect("a data URL needs no network"), "let x = 1");
+    }
+}