Repository navigation
Conversation
When Fetch.enable request-stage patterns cover the XHR class (for example a deliberately wide substring pattern without a resourceType), a synchronous XMLHttpRequest.send() recorded a terminal "Synchronous XMLHttpRequest interception is not supported" failure and threw NetworkError before any pause was published, so the client could neither observe Fetch.requestPaused nor answer it. Every synchronous XHR on the page was failed this way, including first-party URLs matching no pattern and URLs carrying a blocked marker only in their path, while Chromium pauses the request and serves it after continueRequest. A paused synchronous send cannot work in the current renderer: it blocks the single owner-loop thread that must flush the pause publication and later execute the continue page command, which self-deadlocks. Degrade to pass-through instead of fabricating a failure: only asynchronous sends are intercepted, and synchronous ones fall through to their ordinary network path — including its offline-emulation check, which the pass- through must not bypass. Deliberate blocking of synchronous XHR remains unavailable; the divergence is documented in the linked issue. Review fixes: narrow the interception gate to asynchronous sends so the offline gate and the single synchronous network path apply, bind the network-lifecycle assertions to the trap request id, and add regressions for offline emulation and for a typed XHR pattern with a matching URL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2728
Problem
A client that enables request interception with a wide pattern set — e.g. ad-blockers and crawlers using
{ "urlPattern": "*google-analytics.com*", "requestStage": "Request" }withoutresourceTypeand deciding per paused request — actively loses synchronous XHRs on moli while Chromium serves them:Fetch.requestPausedis emitted for a synchronous send, so the client can never answer it withcontinueRequest/failRequest;NetworkError;Network.loadingFailedreports"Synchronous XMLHttpRequest interception is not supported"; the server is never contacted;http://<own-host>/js/google-analytics.com.js).Empirical Chromium reference (Chrome 157 headless, raw CDP, same pattern + page): the synchronous XHR is paused,
Fetch.continueRequestcompletes it with200, and the server observes the request.Root cause
moli-renderer-v8/src/network_host/xhr/send.rs— inxhr_send_callback, thehost.should_intercept_subresource(SubresourceResourceType::Xhr)+!async_requestbranch recorded a terminal network failure ("Synchronous XMLHttpRequest interception is not supported") and threwNetworkErrorinside the synchronoussend(), before any pause was published.A paused synchronous send is not implementable on the current renderer: the send blocks the renderer's single owner-loop thread, which is also the thread that must flush the pause publication journal and later execute the
ContinuePendingSubresourceFetchpage command — pausing self-deadlocks. (Verified experimentally during the investigation: a blocking decision channel insidesend()froze the publication before the client could ever observeFetch.requestPaused.) Chromium pauses synchronous XHR in a separate network service with a nested message pump on the renderer main thread; moli's in-thread fetch has no equivalent. So "cannot pause" is a real constraint — but it was implemented as "fabricate a failure" for requests the client never asked to block.What's changed
moli-renderer-v8/src/network_host/xhr/send.rs, one gate: interception now applies to asynchronous sends only (async_request && should_intercept_subresource(Xhr)), and a synchronous send falls through to its ordinary network path — the exact path used when interception is off, including itsNetwork.emulateNetworkConditions {offline: true}check (a naive pass-through that skips the offline gate would let synchronous XHRs escape to the real network; a regression test pins this). The comment documents the deadlock mechanism and the chosen degradation.Asynchronous XHR (and
fetch()) keep the full interception contract: pause → client decision → continue/fail/fulfill, unchanged.Known remaining divergence, documented in #2728: deliberate blocking of synchronous XHR via
Fetch.failRequestis unavailable on moli (there is no pause to fail), while Chromium supports it. Full parity would require nested-message-pump work in the renderer loop, out of scope here. The worker-thread synchronous XHR path keeps the old fail-early behavior and can be aligned in a follow-up.Protocol-level regression tests in
moli-protocol/src/domains/fetch/tests/runtime_fetch/session_interception.rs:substring_pattern_interception_keeps_synchronous_xhr_on_the_network_path— with the substring pattern enabled, a synchronous XHR to the page's own host with the marker in its path completes with200from the server, produces noFetch.requestPaused, and finishes its network lifecycle bound to that request's id (Network.loadingFinished, noNetwork.loadingFailed);substring_pattern_request_pause_continues_asynchronous_xhr_onto_the_network— the asynchronous counterpart still pauses by the substring pattern (marker in path only) andFetch.continueRequestre-injects it onto the network;wide_interception_keeps_offline_emulation_for_synchronous_xhr— wide pattern +offline: truestill fails the synchronous XHR (Network emulation offline); it must not escape to the real network;typed_xhr_pattern_still_keeps_synchronous_xhr_on_the_network_path— an explicit{resourceType: "XHR"}pattern with a matching URL still yields200and no pause: the unavailable pause is the divergence, not a fabricated failure.Testing
cargo fmt --allandcargo clippy --workspace --all-targets --all-features -- -D warnings— cleancargo nextest run --no-fail-fast— 22,780 passed / 5 failed on this machine: the pre-existing unrelated set (moli-fetchproxy pair,moli-layoutpaint contract,moli-protocolemulation locale override) plus one load flake proven green in isolation on both this branch and cleanmain; post-review runs:moli-protocol4,055/4,056 (same known env failure),moli-renderer-v8XHR suite 1,435/1,435, worker synchronous-XHR tests 3/3fetch-resource-dispatch,fetch-font,xhr-sync-semantics— green on moli and on stock Chromium (--browser chromium)test_external_processfails on environment grounds on cleanmaintoo (pre-existing)Relation to #2704 / #2696
Independent defect (reproduced on stock v1.1.23 and current
main, which already includes #2696). #2704 scopes interception by resource-class set for patterns with explicitresourceType; this change covers the complementary case — patterns that cover the XHR class without a per-URL match being knowable in the renderer — and supersedes #2704's interim "ban stays for a genuinely scoped XHR class" decision for the window path: a fabricated failure for a URL the client never named is wrong in both shapes.Suggested landing (also noted in #2704): merge #2704 first, then rebase this branch — the shared test-file anchor needs a manual resolve, and #2704's two "control half" assertions that expect the ban (
heterogeneous_fetch_enable_patterns_keep_synchronous_xhr_on_the_network_path,synchronous_window_xhr_keeps_network_path_under_heterogeneous_interception_classes) should be rewritten to the new contract (200+ noFetch.requestPaused), along with the "ban stays" paragraph in its description.send.rsitself does not conflict: #2704 does not touch it.