Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ff9f22b
Add browser interoperability test infrastructure
ai-and-i Feb 22, 2026
a07d600
Add comprehensive browser interoperability test suite
ai-and-i Feb 23, 2026
dfea632
Update browser tests to use W3C-compliant session close
ai-and-i Feb 26, 2026
281467b
Validate close code and reason propagation in browser tests
ai-and-i Feb 26, 2026
08fb257
Add extended browser test coverage for streams, datagrams, and error …
ai-and-i Feb 27, 2026
2f2446e
Remove unsafe impl Sync for SharedBrowser
ai-and-i Feb 28, 2026
ea3f4f8
Surface accept-loop panics in TestServer::shutdown
ai-and-i Feb 28, 2026
288bdcf
Remove sort() in bidi_stream_server_priority to assert arrival order
ai-and-i Feb 28, 2026
db71530
Fix shell injection in cleanup watchdog by using positional arguments
ai-and-i Feb 28, 2026
7649691
Fix watchdog kill targeting entire process group when Chrome PID unav…
ai-and-i Feb 28, 2026
a87e244
Avoid panic on shutdown send when accept loop already exited
ai-and-i Feb 28, 2026
de3f466
Clean connection close in stream_error tests
ai-and-i Feb 28, 2026
2e135a4
Replace wt.close() synchronization with stream-based handshakes in st…
ai-and-i Feb 28, 2026
148b9c7
Add tests reproducing concurrent accept_bi/accept_uni lost waker bug
ai-and-i Feb 28, 2026
27696fa
Update send_datagram_after_close test to expect ConnectionError
ai-and-i Mar 3, 2026
c18b87f
Replace sleep-based synchronization with stream handshakes in browser…
ai-and-i Mar 3, 2026
de3a83a
Applied cargo fmt
ai-and-i Mar 3, 2026
76ac16a
Fixed one more flaky test
ai-and-i Mar 3, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[workspace]
members = [
"web-transport",
"web-transport-browser-tests",
"web-transport-proto",
"web-transport-quiche",
"web-transport-quinn",
Expand Down
16 changes: 10 additions & 6 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ setup-tools:

# Run the CI checks
check:
cargo check --workspace --all-targets --all-features
cargo clippy --workspace --all-targets --all-features -- -D warnings
cargo check --workspace --all-targets --all-features --exclude web-transport-browser-tests
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main thing that worries me. It makes sense not to run everything locally, but not even type checking the implementation seems sus.

cargo clippy --workspace --all-targets --all-features --exclude web-transport-browser-tests -- -D warnings

# Do the same but explicitly use the WASM target.
cargo check --target wasm32-unknown-unknown -p web-transport --all-targets --all-features
Expand All @@ -34,7 +34,7 @@ check:
cargo fmt --all --check

# requires: cargo install cargo-hack
cargo hack check --feature-powerset --workspace --keep-going
cargo hack check --feature-powerset --workspace --keep-going --exclude web-transport-browser-tests
cargo hack check --feature-powerset --target wasm32-unknown-unknown -p web-transport --keep-going
cargo hack check --feature-powerset --target wasm32-unknown-unknown -p web-transport-wasm --keep-going

Expand All @@ -50,14 +50,18 @@ check:

# Run any CI tests
test:
cargo test --workspace --all-targets --all-features
cargo test --workspace --all-targets --all-features --exclude web-transport-browser-tests
cargo test --target wasm32-unknown-unknown -p web-transport --all-targets --all-features
cargo test --target wasm32-unknown-unknown -p web-transport-wasm --all-targets --all-features

# Run browser interoperability tests (requires Chromium or auto-downloads it)
browser-test:
cargo test -p web-transport-browser-tests

# Automatically fix some issues.
fix:
cargo fix --allow-staged --allow-dirty --workspace --all-targets --all-features
cargo clippy --fix --allow-staged --allow-dirty --workspace --all-targets --all-features
cargo fix --allow-staged --allow-dirty --workspace --all-targets --all-features --exclude web-transport-browser-tests
cargo clippy --fix --allow-staged --allow-dirty --workspace --all-targets --all-features --exclude web-transport-browser-tests

# Do the same but explicitly use the WASM target.
cargo fix --allow-staged --allow-dirty --target wasm32-unknown-unknown -p web-transport --all-targets --all-features
Expand Down
34 changes: 34 additions & 0 deletions web-transport-browser-tests/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
[package]
name = "web-transport-browser-tests"
description = "Browser interoperability test infrastructure for web-transport"
version = "0.1.0"
edition = "2021"
publish = false

[dependencies]
anyhow = "1"
chromiumoxide = { version = "0.9", default-features = false, features = [
"bytes",
"fetcher",
"rustls",
"zip8",
] }
futures = "0.3"
rcgen = { version = "0.14", default-features = false, features = ["aws_lc_rs"] }
rustls = { version = "0.23", default-features = false, features = [
"aws-lc-rs",
"std",
] }
serde = { version = "1", features = ["derive"] }
serde_json = "1"
time = "0.3"
tokio = { version = "1", features = ["full"] }
tracing = "0.1"

[dependencies.web-transport-quinn]
path = "../web-transport-quinn"

[dev-dependencies]
bytes = "1"
http = "1"
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
292 changes: 292 additions & 0 deletions web-transport-browser-tests/src/browser.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,292 @@
use std::net::SocketAddr;
use std::path::PathBuf;
use std::sync::OnceLock;
use std::time::Duration;

use anyhow::{Context, Result};
use chromiumoxide::browser::{Browser, BrowserConfig};
use chromiumoxide::cdp::browser_protocol::browser::BrowserContextId;
use chromiumoxide::cdp::browser_protocol::target::{
CreateBrowserContextParams, CreateTargetParams,
};
use chromiumoxide::fetcher::{BrowserFetcher, BrowserFetcherOptions};
use chromiumoxide::Page;
use futures::StreamExt;
use serde::Deserialize;

use crate::js;

struct SharedBrowser {
browser: tokio::sync::Mutex<Browser>,
/// URL of the blank page server (http://localhost:{port}).
page_url: String,
// A dedicated Tokio runtime that owns the browser event handler and HTTP
// server tasks. This runtime lives as long as the `SharedBrowser` (i.e.
// for the entire process) so that the tasks are not cancelled when
// individual `#[tokio::test]` runtimes shut down.
_runtime: tokio::runtime::Runtime,
// A page that stays open for the lifetime of the browser to prevent
// Chrome from exiting when all test contexts are disposed.
_keepalive_page: Page,
// Holds the stdin pipe to the cleanup watchdog process. When our process
// exits (for any reason, including SIGKILL), the OS closes this FD,
// unblocking the watchdog which then kills Chrome and removes the temp dir.
_watchdog_pipe: std::sync::Mutex<Option<std::process::ChildStdin>>,
}

static BROWSER: OnceLock<SharedBrowser> = OnceLock::new();

/// Try to build a BrowserConfig, auto-downloading Chromium via the fetcher if
/// no local executable is detected.
async fn build_browser_config() -> BrowserConfig {
// Use a per-process data dir to avoid stale SingletonLock conflicts.
let data_dir = std::env::temp_dir().join(format!("chromiumoxide-{}", std::process::id()));

// Note: .arg() auto-prefixes with "--", so pass bare names.
let builder = BrowserConfig::builder()
.new_headless_mode()
.no_sandbox()
.user_data_dir(&data_dir)
.arg("allow-insecure-localhost");

match builder.clone().build() {
Ok(config) => return config,
Err(_) => {
tracing::info!("no local Chrome found, downloading via fetcher...");
}
}

// Auto-detection failed — download Chromium.
let exe_path = fetch_chromium().await;

builder
.chrome_executable(exe_path)
.build()
.expect("failed to build browser config with fetched executable")
}

/// Download Chromium using chromiumoxide_fetcher and return the executable path.
async fn fetch_chromium() -> PathBuf {
// Use the same cache directory as the fetcher's default
// ($XDG_CACHE_HOME or $HOME/.cache)/chromiumoxide. We must create it
// ourselves because the fetcher doesn't create parent directories.
let base = std::env::var("XDG_CACHE_HOME")
.map(PathBuf::from)
.unwrap_or_else(|_| {
PathBuf::from(std::env::var("HOME").expect("HOME not set")).join(".cache")
});
let cache_dir = base.join("chromiumoxide");
std::fs::create_dir_all(&cache_dir).expect("failed to create browser cache directory");

let options = BrowserFetcherOptions::builder()
.with_path(&cache_dir)
.build()
.expect("failed to create fetcher options");

let fetcher = BrowserFetcher::new(options);
let installation = fetcher.fetch().await.expect("failed to download Chromium");

tracing::info!(path = %installation.executable_path.display(), "downloaded Chromium");
installation.executable_path
}

/// Spawn a background shell that blocks on stdin. When our process exits (for
/// any reason), the OS closes the pipe, `read` gets EOF, and the shell kills
/// Chrome and removes its data directory.
fn spawn_cleanup_watchdog(
chrome_pid: u32,
data_dir: &std::path::Path,
) -> Option<std::process::ChildStdin> {
use std::process::{Command, Stdio};

// Pass chrome_pid and data_dir as positional arguments ($0, $1) to avoid
// shell injection from interpolating paths into the command string.
let mut child = Command::new("sh")
.arg("-c")
.arg(r#"read _; kill -9 "$0" 2>/dev/null; rm -rf "$1""#)
.arg(chrome_pid.to_string())
.arg(data_dir.as_os_str())
.stdin(Stdio::piped())
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.ok()?;
child.stdin.take()
}

fn init_browser() -> SharedBrowser {
// Create a dedicated runtime on a separate thread. We cannot call
// `block_on` from within an existing Tokio runtime (which `#[tokio::test]`
// creates), so we spawn a plain OS thread to build and initialise
// everything, then send the results back.
std::thread::spawn(|| {
let rt = tokio::runtime::Builder::new_multi_thread()
.worker_threads(2)
.enable_all()
.build()
.expect("failed to build dedicated browser runtime");

let (browser, page_url, keepalive_page, chrome_pid) = rt.block_on(async {
let config = build_browser_config().await;

// Start the blank page server on this runtime.
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
.await
.expect("failed to bind blank page server");
let addr: SocketAddr = listener.local_addr().unwrap();
let page_url = format!("http://localhost:{}", addr.port());

tokio::spawn(async move {
loop {
let Ok((mut stream, _)) = listener.accept().await else {
break;
};
tokio::spawn(async move {
use tokio::io::{AsyncReadExt, AsyncWriteExt};
let mut buf = [0u8; 1024];
let _ = stream.read(&mut buf).await;
let response = "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nContent-Length: 13\r\n\r\n<html></html>";
let _ = stream.write_all(response.as_bytes()).await;
});
}
});

let (mut browser, mut handler) = Browser::launch(config)
.await
.expect("failed to launch browser");

// Extract Chrome's PID before we lose mutability, so the watchdog
// can kill it on abnormal exit.
let chrome_pid = browser.get_mut_child().and_then(|c| c.inner.id());

// Spawn the CDP event handler on this runtime.
tokio::spawn(async move {
while let Some(event) = handler.next().await {
let _ = event;
}
});

// Open a keepalive page in the default context so Chrome doesn't
// exit when all test browser contexts are disposed.
let keepalive_page = browser
.new_page(&page_url)
.await
.expect("failed to create keepalive page");

(browser, page_url, keepalive_page, chrome_pid)
});

// Spawn a watchdog that kills Chrome and cleans up the temp dir when
// our process exits. The data dir path matches build_browser_config().
let data_dir =
std::env::temp_dir().join(format!("chromiumoxide-{}", std::process::id()));
let watchdog_pipe = chrome_pid.and_then(|pid| spawn_cleanup_watchdog(pid, &data_dir));

SharedBrowser {
browser: tokio::sync::Mutex::new(browser),
page_url,
_runtime: rt,
_keepalive_page: keepalive_page,
_watchdog_pipe: std::sync::Mutex::new(watchdog_pipe),
}
})
.join()
.expect("browser init thread panicked")
}

fn get_browser() -> &'static SharedBrowser {
BROWSER.get_or_init(init_browser)
}

/// An isolated browser context for a single test.
///
/// Each context has its own cookies, cache, and page — preventing
/// cross-test interference.
pub struct TestContext {
page: Page,
context_id: Option<BrowserContextId>,
}

impl TestContext {
/// Create a new isolated browser context.
pub async fn new() -> Result<Self> {
let shared = get_browser();

let context_id = shared
.browser
.lock()
.await
.create_browser_context(CreateBrowserContextParams::default())
.await
.context("failed to create browser context")?;

// Navigate to a localhost HTTP page so the JS context has a secure
// origin with the WebTransport API available.
let page = shared
.browser
.lock()
.await
.new_page(
CreateTargetParams::builder()
.url(&shared.page_url)
.browser_context_id(context_id.clone())
.build()
.unwrap(),
)
.await
.context("failed to create page")?;
Comment on lines +215 to +237
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Dispose the created browser context when page creation fails.

TestContext::new can leak an isolated context if new_page(...) errors after create_browser_context(...) succeeds. This can accumulate stale contexts and make later tests noisier.

🔧 Proposed fix
-        let page = shared
-            .browser
-            .lock()
-            .await
-            .new_page(
-                CreateTargetParams::builder()
-                    .url(&shared.page_url)
-                    .browser_context_id(context_id.clone())
-                    .build()
-                    .unwrap(),
-            )
-            .await
-            .context("failed to create page")?;
+        let page = match shared
+            .browser
+            .lock()
+            .await
+            .new_page(
+                CreateTargetParams::builder()
+                    .url(&shared.page_url)
+                    .browser_context_id(context_id.clone())
+                    .build()
+                    .unwrap(),
+            )
+            .await
+        {
+            Ok(page) => page,
+            Err(err) => {
+                let _ = shared
+                    .browser
+                    .lock()
+                    .await
+                    .dispose_browser_context(context_id.clone())
+                    .await;
+                return Err(err).context("failed to create page");
+            }
+        };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web-transport-browser-tests/src/browser.rs` around lines 215 - 237, When
create_browser_context in TestContext::new succeeds but new_page(...) fails,
ensure you dispose the created context to avoid leaks: after obtaining
context_id, wrap the new_page call so that on any error you call the browser
context teardown API (e.g., delete_browser_context / close_browser_context or
the appropriate method on shared.browser) using the same context_id (via
shared.browser.lock().await) before returning the error; update the error path
in TestContext::new around create_browser_context/new_page to perform this
cleanup.


Ok(Self {
page,
context_id: Some(context_id),
})
}

/// Evaluate a JavaScript test snippet in the browser.
///
/// The `js_code` is wrapped with `connectWebTransport()` and error handling
/// via [`js::wrap_test_js`].
pub async fn run_js_test(
&self,
server_url: &str,
certificate_hash: &[u8],
js_code: &str,
timeout: Duration,
) -> Result<JsTestResult> {
let wrapped = js::wrap_test_js(server_url, certificate_hash, js_code);

let result: String = tokio::time::timeout(timeout, self.page.evaluate(wrapped))
.await
.context("JS test timed out")?
.context("JS evaluation failed")?
.into_value()
.context("failed to extract JS result value")?;

let parsed: JsTestResult =
serde_json::from_str(&result).context("failed to parse JS test result")?;

Ok(parsed)
}

/// Dispose of the browser context.
pub async fn dispose(mut self) {
if let Some(id) = self.context_id.take() {
let shared = get_browser();
let _ = shared
.browser
.lock()
.await
.dispose_browser_context(id)
.await;
}
}
}

/// The result of a JavaScript test evaluation.
#[derive(Debug, Deserialize)]
pub struct JsTestResult {
pub success: bool,
pub message: String,
#[serde(default)]
pub details: serde_json::Value,
}
Loading