fix: run webRequest handlers for URLs handled by ElectronURLLoaderFactory (#45915)

* fix: continue to run ProxyingURLLoaderFactory for intercepted protocols

* test: webRequest handlers when loading browser windows

* fix: wrap special URL loaders factories with ProxyingURLLoaderFactory

* test: webRequest handlers when using net.fetch

* refactor: remove redundant intercepted protocol handling

AsarURLLoaderFactory is now intercepted by ProxyingURLLoaderFactory, which already handles when the file:// scheme is intercepted.

* fix: check before using saved headers in OnReceiveResponse

* fix: run webRequest handlers when loading file service workers

* test: handlers when loading file service workers

* refactor: add shared CreateURLLoaderFactoryBuilder method

---------

Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>

shell/browser/electron_browser_context.cc

@@ -43,9 +43,7 @@
 #include "media/audio/audio_device_description.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/originating_process_id.h"
-#include "services/network/public/cpp/url_loader_factory_builder.h"
 #include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
-#include "services/network/public/mojom/network_context.mojom.h"
 #include "shell/browser/cookie_change_notifier.h"
 #include "shell/browser/electron_browser_client.h"
 #include "shell/browser/electron_browser_main_parts.h"
@@ -583,11 +581,9 @@ void ElectronBrowserContext::OnNetworkServiceProcessGone(bool /* crashed */) {
   url_loader_factory_.reset();
 }
 
-scoped_refptr<network::SharedURLLoaderFactory>
-ElectronBrowserContext::GetURLLoaderFactory() {
-  if (url_loader_factory_)
-    return url_loader_factory_;
-
+std::pair<network::URLLoaderFactoryBuilder,
+          mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>>
+ElectronBrowserContext::CreateURLLoaderFactoryBuilder() {
   network::URLLoaderFactoryBuilder factory_builder;
 
   // Consult the embedder.
@@ -601,6 +597,16 @@ ElectronBrowserContext::GetURLLoaderFactory() {
           ukm::kInvalidSourceIdObj, factory_builder, &header_client, nullptr,
           nullptr, nullptr, nullptr);
 
+  return std::make_pair(std::move(factory_builder), std::move(header_client));
+}
+
+scoped_refptr<network::SharedURLLoaderFactory>
+ElectronBrowserContext::GetURLLoaderFactory() {
+  if (url_loader_factory_)
+    return url_loader_factory_;
+
+  auto [factory_builder, header_client] = CreateURLLoaderFactoryBuilder();
+
   network::mojom::URLLoaderFactoryParamsPtr params =
       network::mojom::URLLoaderFactoryParams::New();
   params->header_client = std::move(header_client);
@@ -618,6 +624,12 @@ ElectronBrowserContext::GetURLLoaderFactory() {
   return url_loader_factory_;
 }
 
+scoped_refptr<network::SharedURLLoaderFactory>
+ElectronBrowserContext::InterceptURLLoaderFactory(
+    scoped_refptr<network::SharedURLLoaderFactory> factory) {
+  return CreateURLLoaderFactoryBuilder().first.Finish(factory);
+}
+
 content::PushMessagingService*
 ElectronBrowserContext::GetPushMessagingService() {
   return nullptr;

shell/browser/electron_browser_context.h

@@ -19,6 +19,8 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/media_stream_request.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "services/network/public/cpp/url_loader_factory_builder.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/ssl_config.mojom.h"
 #include "third_party/blink/public/common/permissions/permission_utils.h"
 
@@ -93,6 +95,8 @@ class ElectronBrowserContext : public content::BrowserContext {
   ResolveProxyHelper* GetResolveProxyHelper();
   content::PreconnectManager* GetPreconnectManager();
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory();
+  scoped_refptr<network::SharedURLLoaderFactory> InterceptURLLoaderFactory(
+      scoped_refptr<network::SharedURLLoaderFactory> factory);
 
   std::string GetMediaDeviceIDSalt();
 
@@ -183,6 +187,10 @@ class ElectronBrowserContext : public content::BrowserContext {
       content::MediaResponseCallback callback,
       gin::Arguments* args);
 
+  std::pair<network::URLLoaderFactoryBuilder,
+            mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>>
+  CreateURLLoaderFactoryBuilder();
+
   // Initialize pref registry.
   void InitPrefs();
 

shell/browser/net/proxying_url_loader_factory.cc

@@ -22,7 +22,7 @@
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/early_hints.mojom.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
-#include "shell/browser/net/asar/asar_url_loader.h"
+#include "shell/browser/net/asar/asar_url_loader_factory.h"
 #include "shell/common/options_switches.h"
 #include "third_party/abseil-cpp/absl/strings/str_format.h"
 #include "url/origin.h"
@@ -36,6 +36,7 @@ ProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams::
 
 ProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
     ProxyingURLLoaderFactory* factory,
+    mojo::Remote<network::mojom::URLLoaderFactory> override_target_factory,
     uint64_t web_request_id,
     int32_t frame_routing_id,
     int32_t network_service_request_id,
@@ -45,6 +46,7 @@ ProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
     mojo::PendingReceiver<network::mojom::URLLoader> loader_receiver,
     mojo::PendingRemote<network::mojom::URLLoaderClient> client)
     : factory_(factory),
+      override_target_factory_(std::move(override_target_factory)),
       request_(request),
       original_initiator_(request.request_initiator),
       request_id_(web_request_id),
@@ -238,7 +240,11 @@ void ProxyingURLLoaderFactory::InProgressRequest::OnReceiveResponse(
     // Set-Cookie if it existed.
     auto saved_headers = current_response_->headers;
     current_response_ = std::move(head);
-    current_response_->headers = saved_headers;
+    // If this response is from a file or handler, OnHeadersReceived will not
+    // be called before OnReceiveResponse, so make sure the saved headers exist
+    // before setting them.
+    if (saved_headers)
+      current_response_->headers = saved_headers;
     ContinueToResponseStarted(net::OK);
   } else {
     current_response_ = std::move(head);
@@ -493,7 +499,12 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToStartRequest(
     return;
   }
 
-  if (!target_loader_.is_bound() && factory_->target_factory_.is_bound()) {
+  if (!target_loader_.is_bound()) {
+    auto& target_factory = override_target_factory_.is_bound()
+                               ? override_target_factory_
+                               : factory_->target_factory_;
+    if (!target_factory.is_bound())
+      return;
     // No extensions have cancelled us up to this point, so it's now OK to
     // initiate the real network request.
     uint32_t options = options_;
@@ -501,7 +512,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToStartRequest(
     // might, so we need to set the option on the loader.
     if (has_any_extra_headers_listeners_)
       options |= network::mojom::kURLLoadOptionUseHeaderClient;
-    factory_->target_factory_->CreateLoaderAndStart(
+    target_factory->CreateLoaderAndStart(
         target_loader_.BindNewPipeAndPassReceiver(),
         network_service_request_id_, options, request_,
         proxied_client_receiver_.BindNewPipeAndPassRemote(),
@@ -794,23 +805,16 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart(
     request.load_flags |= net::LOAD_IGNORE_LIMITS;
   }
 
+  mojo::Remote<network::mojom::URLLoaderFactory> override_target_factory;
+
   // Check if user has intercepted this scheme.
   bool bypass_custom_protocol_handlers =
       options & kBypassCustomProtocolHandlers;
   if (!bypass_custom_protocol_handlers) {
     auto it = intercepted_handlers_->find(request.url.scheme());
     if (it != intercepted_handlers_->end()) {
-      mojo::PendingRemote<network::mojom::URLLoaderFactory> loader_remote;
-      this->Clone(loader_remote.InitWithNewPipeAndPassReceiver());
-
-      // <scheme, <type, handler>>
-      it->second.second.Run(
-          request,
-          base::BindOnce(&ElectronURLLoaderFactory::StartLoading,
-                         std::move(loader), request_id, options, request,
-                         std::move(client), traffic_annotation,
-                         std::move(loader_remote), it->second.first));
-      return;
+      override_target_factory.Bind(ElectronURLLoaderFactory::Create(
+          it->second.first, it->second.second));
     }
   }
 
@@ -818,18 +822,19 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart(
   // Chromium does not provide a way to override this behavior. So in order to
   // make ServiceWorker work with file:// URLs, we have to intercept its
   // requests here.
-  if (IsForServiceWorkerScript() && request.url.SchemeIsFile()) {
-    asar::CreateAsarURLLoader(
-        request, std::move(loader), std::move(client),
-        base::MakeRefCounted<net::HttpResponseHeaders>(""));
-    return;
+  if (IsForServiceWorkerScript() && request.url.SchemeIsFile() &&
+      !override_target_factory.is_bound()) {
+    override_target_factory.Bind(AsarURLLoaderFactory::Create());
   }
 
   if (!web_request_->HasListener()) {
     // Pass-through to the original factory.
-    target_factory_->CreateLoaderAndStart(std::move(loader), request_id,
-                                          options, request, std::move(client),
-                                          traffic_annotation);
+    auto& target_factory = override_target_factory.is_bound()
+                               ? override_target_factory
+                               : target_factory_;
+    target_factory->CreateLoaderAndStart(std::move(loader), request_id, options,
+                                         request, std::move(client),
+                                         traffic_annotation);
     return;
   }
 
@@ -849,8 +854,9 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart(
   auto result = requests_.emplace(
       web_request_id,
       std::make_unique<InProgressRequest>(
-          this, web_request_id, frame_routing_id_, request_id, options, request,
-          traffic_annotation, std::move(loader), std::move(client)));
+          this, std::move(override_target_factory), web_request_id,
+          frame_routing_id_, request_id, options, request, traffic_annotation,
+          std::move(loader), std::move(client)));
   result.first->second->Restart();
 }
 

shell/browser/net/proxying_url_loader_factory.h

@@ -64,6 +64,8 @@ class ProxyingURLLoaderFactory
     // For usual requests
     InProgressRequest(
         ProxyingURLLoaderFactory* factory,
+        const mojo::Remote<network::mojom::URLLoaderFactory>
+            override_target_factory,
         uint64_t web_request_id,
         int32_t frame_routing_id,
         int32_t network_service_request_id,
@@ -140,6 +142,8 @@ class ProxyingURLLoaderFactory
     void HandleBeforeRequestRedirect();
 
     raw_ptr<ProxyingURLLoaderFactory> const factory_;
+    const mojo::Remote<network::mojom::URLLoaderFactory>
+        override_target_factory_;
     network::ResourceRequest request_;
     const std::optional<url::Origin> original_initiator_;
     const uint64_t request_id_ = 0;

shell/common/api/electron_api_url_loader.cc

@@ -496,36 +496,27 @@ SimpleURLLoaderWrapper::GetURLLoaderFactoryForURL(const GURL& url) {
     return URLLoaderBundle::GetInstance()->GetSharedURLLoaderFactory();
 
   CHECK(browser_context_);
-  // Explicitly handle intercepted protocols here, even though
-  // ProxyingURLLoaderFactory would handle them later on, so that we can
-  // correctly intercept file:// scheme URLs.
   if (const bool bypass = request_options_ & kBypassCustomProtocolHandlers;
       !bypass) {
     const std::string_view scheme = url.scheme();
     const auto* const protocol_registry =
         ProtocolRegistry::FromBrowserContext(browser_context_);
 
-    if (const auto* const protocol_handler =
-            protocol_registry->FindIntercepted(scheme)) {
-      return network::SharedURLLoaderFactory::Create(
-          std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-              ElectronURLLoaderFactory::Create(protocol_handler->first,
-                                               protocol_handler->second)));
-    }
-
     if (const auto* const protocol_handler =
             protocol_registry->FindRegistered(scheme)) {
-      return network::SharedURLLoaderFactory::Create(
-          std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-              ElectronURLLoaderFactory::Create(protocol_handler->first,
-                                               protocol_handler->second)));
+      return browser_context_->InterceptURLLoaderFactory(
+          network::SharedURLLoaderFactory::Create(
+              std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
+                  ElectronURLLoaderFactory::Create(protocol_handler->first,
+                                                   protocol_handler->second))));
     }
   }
 
   if (url.SchemeIsFile()) {
-    return network::SharedURLLoaderFactory::Create(
-        std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-            AsarURLLoaderFactory::Create()));
+    return browser_context_->InterceptURLLoaderFactory(
+        network::SharedURLLoaderFactory::Create(
+            std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
+                AsarURLLoaderFactory::Create())));
   }
 
   return browser_context_->GetURLLoaderFactory();

spec/api-browser-window-spec.ts

@@ -569,6 +569,67 @@ describe('BrowserWindow module', () => {
         .catch((e) => console.log(e));
       expect(await w.webContents.executeJavaScript('window.ping')).to.equal('pong');
     });
+
+    describe('webRequest', () => {
+      afterEach(() => {
+        session.defaultSession.webRequest.onBeforeRequest(null);
+      });
+
+      it('triggers webRequest handlers for https', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        await expect(w.loadURL('https://foo')).to.eventually.be.rejectedWith(/^ERR_BLOCKED_BY_CLIENT/);
+      });
+
+      it('triggers webRequest handlers for intercepted https', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        session.defaultSession.protocol.handle('https', () => new Response());
+        defer(() => {
+          session.defaultSession.protocol.unhandle('https');
+        });
+
+        await expect(w.loadURL('https://foo')).to.eventually.be.rejectedWith(/^ERR_BLOCKED_BY_CLIENT/);
+      });
+
+      it('triggers webRequest handlers for file urls', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        await expect(w.loadURL('file://foo')).to.eventually.be.rejectedWith(/^ERR_BLOCKED_BY_CLIENT/);
+      });
+
+      it('triggers webRequest handlers for intercepted file urls', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        session.defaultSession.protocol.handle('file', () => new Response());
+        defer(() => {
+          session.defaultSession.protocol.unhandle('file');
+        });
+
+        await expect(w.loadURL('file://foo')).to.eventually.be.rejectedWith(/^ERR_BLOCKED_BY_CLIENT/);
+      });
+
+      it('triggers webRequest handlers for registered protocols', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        session.defaultSession.protocol.handle('custom-protocol', () => new Response());
+        defer(() => {
+          session.defaultSession.protocol.unhandle('custom-protocol');
+        });
+
+        await expect(w.loadURL('custom-protocol://foo')).to.eventually.be.rejectedWith(/^ERR_BLOCKED_BY_CLIENT/);
+      });
+    });
   });
 
   for (const sandbox of [false, true]) {

spec/api-net-session-spec.ts

@@ -5,6 +5,7 @@ import { expect } from 'chai';
 import * as dns from 'node:dns';
 
 import { collectStreamBody, getResponse, respondNTimes, respondOnce } from './lib/net-helpers';
+import { defer } from './lib/spec-helpers';
 
 // See https://github.com/nodejs/node/issues/40702.
 dns.setDefaultResultOrder('ipv4first');
@@ -660,5 +661,66 @@ describe('net module (session)', () => {
       });
       expect(response.headers.get('x-cookie')).to.equal(`wild_cookie=${cookieVal}`);
     });
+
+    describe('webRequest', () => {
+      afterEach(() => {
+        session.defaultSession.webRequest.onBeforeRequest(null);
+      });
+
+      it('triggers webRequest handlers for https', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        await expect(net.fetch('https://foo')).to.eventually.be.rejectedWith('net::ERR_BLOCKED_BY_CLIENT');
+      });
+
+      it('triggers webRequest handlers for intercepted https', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        session.defaultSession.protocol.handle('https', () => new Response());
+        defer(() => {
+          session.defaultSession.protocol.unhandle('https');
+        });
+
+        await expect(net.fetch('https://foo')).to.eventually.be.rejectedWith('net::ERR_BLOCKED_BY_CLIENT');
+      });
+
+      it('triggers webRequest handlers for file urls', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        await expect(net.fetch('file://foo')).to.eventually.be.rejectedWith('net::ERR_BLOCKED_BY_CLIENT');
+      });
+
+      it('triggers webRequest handlers for intercepted file urls', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        session.defaultSession.protocol.handle('file', () => new Response());
+        defer(() => {
+          session.defaultSession.protocol.unhandle('file');
+        });
+
+        await expect(net.fetch('file://foo')).to.eventually.be.rejectedWith('net::ERR_BLOCKED_BY_CLIENT');
+      });
+
+      it('triggers webRequest handlers for registered protocols', async () => {
+        session.defaultSession.webRequest.onBeforeRequest((_, cb) => {
+          cb({ cancel: true });
+        });
+
+        session.defaultSession.protocol.handle('custom-protocol', () => new Response());
+        defer(() => {
+          session.defaultSession.protocol.unhandle('custom-protocol');
+        });
+
+        await expect(net.fetch('custom-protocol://foo')).to.eventually.be.rejectedWith('net::ERR_BLOCKED_BY_CLIENT');
+      });
+    });
   });
 });

spec/chromium-spec.ts

@@ -759,6 +759,8 @@ describe('chromium features', () => {
         let file = new URL(request.url).pathname!;
         if (file[0] === '/' && process.platform === 'win32') file = file.slice(1);
 
+        file = file.replace('service-worker.js', 'service-worker-intercepted.js');
+
         const content = fs.readFileSync(path.normalize(file));
         const ext = path.extname(file);
         let type = 'text/html';
@@ -781,7 +783,7 @@ describe('chromium features', () => {
         } else if (channel === 'error') {
           done(`unexpected error : ${message}`);
         } else if (channel === 'response') {
-          expect(message).to.equal('Hello from serviceWorker!');
+          expect(message).to.equal('Hello from serviceWorker intercepted!');
           customSession.clearStorageData({
             storages: ['serviceworkers']
           }).then(() => {
@@ -794,6 +796,27 @@ describe('chromium features', () => {
       w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'index.html'));
     });
 
+    it('should trigger webRequest handlers when loaded as a file', (done) => {
+      const customSession = session.fromPartition('sw-file-scheme-webRequest');
+      customSession.webRequest.onBeforeRequest((details, cb) => {
+        if (details.url.endsWith('service-worker.js')) {
+          done(); // Service worker triggered webRequest handler.
+        }
+        cb({});
+      });
+
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true,
+          session: customSession,
+          contextIsolation: false
+        }
+      });
+      w.webContents.on('render-process-gone', () => done(new Error('WebContents crashed.')));
+      w.loadFile(path.join(fixturesPath, 'pages', 'service-worker', 'index.html'));
+    });
+
     it('should register for custom scheme', (done) => {
       const customSession = session.fromPartition('custom-scheme');
       customSession.protocol.registerFileProtocol(serviceWorkerScheme, (request, callback) => {

spec/fixtures/pages/service-worker/service-worker-intercepted.js

@@ -0,0 +1,9 @@
+self.addEventListener('fetch', function (event) {
+  const requestUrl = new URL(event.request.url);
+
+  if (requestUrl.pathname === '/echo' &&
+    event.request.headers.has('X-Mock-Response')) {
+    const mockResponse = new Response('Hello from serviceWorker intercepted!');
+    event.respondWith(mockResponse);
+  }
+});