Browse Source

feat: add excludeUrls and modify urls in WebRequestFilter for better URL filtering (#45678)

* feat: add excludeUrls and modify urls in WebRequestFilter for better URL filtering (#44692)

* feat: add excludeUrls to web request filter

* refactor: add deprecated field

* test: update tests

* lint: newline

* docs: improve API doc

* fix: add is filter defined property to match all urls

* refactor: remove includeUrls

* refactor: remove typescript binding

* refactor: all_url

* refactor: remove isDefined methods

* refactor: remove comment

* fix: logic

* docs: add to breaking changes

* docs: remove unneeded section
Alice Zhao 1 month ago
parent
commit
5a641e02ee

+ 3 - 2
docs/api/structures/web-request-filter.md

@@ -1,4 +1,5 @@
 # WebRequestFilter Object
 
-* `urls` string[] - Array of [URL patterns](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) that will be used to filter out the requests that do not match the URL patterns.
-* `types` String[] (optional) - Array of types that will be used to filter out the requests that do not match the types. When not specified, all types will be matched. Can be `mainFrame`, `subFrame`, `stylesheet`, `script`, `image`, `font`, `object`, `xhr`, `ping`, `cspReport`, `media` or `webSocket`.
+* `urls` string[] - Array of [URL patterns](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) used to include requests that match these patterns. Use the pattern `<all_urls>` to match all URLs.
+* `excludeUrls` string[] (optional) - Array of [URL patterns](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) used to exclude requests that match these patterns.
+* `types` string[] (optional) - Array of types that will be used to filter out the requests that do not match the types. When not specified, all types will be matched. Can be `mainFrame`, `subFrame`, `stylesheet`, `script`, `image`, `font`, `object`, `xhr`, `ping`, `cspReport`, `media` or `webSocket`.

+ 1 - 0
docs/api/web-request.md

@@ -73,6 +73,7 @@ The `callback` has to be called with an `response` object.
 Some examples of valid `urls`:
 
 ```js
+'<all_urls>'
 'http://foo:1234/'
 'http://foo.com/'
 'http://foo:1234/bar'

+ 16 - 0
docs/breaking-changes.md

@@ -63,6 +63,22 @@ webContents.on('console-message', ({ level, message, lineNumber, sourceId, frame
 
 Additionally, `level` is now a string with possible values of `info`, `warning`, `error`, and `debug`.
 
+### Behavior Changed: `urls` property of `WebRequestFilter`.
+
+Previously, an empty urls array was interpreted as including all URLs. To explicitly include all URLs, developers should now use the `<all_urls>` pattern, which is a [designated URL pattern](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns#all_urls) that matches every possible URL. This change clarifies the intent and ensures more predictable behavior.
+
+```js
+// Deprecated
+const deprecatedFilter = {
+  urls: []
+}
+
+// Replace with
+const newFilter = {
+  urls: ['<all_urls>']
+}
+```
+
 ### Deprecated: `systemPreferences.isAeroGlassEnabled()`
 
 The `systemPreferences.isAeroGlassEnabled()` function has been deprecated without replacement.

+ 67 - 27
shell/browser/api/electron_api_web_request.cc

@@ -33,6 +33,7 @@
 #include "shell/common/gin_converters/std_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
+#include "shell/common/node_util.h"
 
 static constexpr auto ResourceTypes =
     base::MakeFixedFlatMap<std::string_view,
@@ -211,15 +212,23 @@ CalculateOnBeforeSendHeadersDelta(const net::HttpRequestHeaders* old_headers,
 gin::WrapperInfo WebRequest::kWrapperInfo = {gin::kEmbedderNativeGin};
 
 WebRequest::RequestFilter::RequestFilter(
-    std::set<URLPattern> url_patterns,
+    std::set<URLPattern> include_url_patterns,
+    std::set<URLPattern> exclude_url_patterns,
     std::set<extensions::WebRequestResourceType> types)
-    : url_patterns_(std::move(url_patterns)), types_(std::move(types)) {}
+    : include_url_patterns_(std::move(include_url_patterns)),
+      exclude_url_patterns_(std::move(exclude_url_patterns)),
+      types_(std::move(types)) {}
 WebRequest::RequestFilter::RequestFilter(const RequestFilter&) = default;
 WebRequest::RequestFilter::RequestFilter() = default;
 WebRequest::RequestFilter::~RequestFilter() = default;
 
-void WebRequest::RequestFilter::AddUrlPattern(URLPattern pattern) {
-  url_patterns_.emplace(std::move(pattern));
+void WebRequest::RequestFilter::AddUrlPattern(URLPattern pattern,
+                                              bool is_match_pattern) {
+  if (is_match_pattern) {
+    include_url_patterns_.emplace(std::move(pattern));
+  } else {
+    exclude_url_patterns_.emplace(std::move(pattern));
+  }
 }
 
 void WebRequest::RequestFilter::AddType(
@@ -227,11 +236,13 @@ void WebRequest::RequestFilter::AddType(
   types_.insert(type);
 }
 
-bool WebRequest::RequestFilter::MatchesURL(const GURL& url) const {
-  if (url_patterns_.empty())
-    return true;
+bool WebRequest::RequestFilter::MatchesURL(
+    const GURL& url,
+    const std::set<URLPattern>& patterns) const {
+  if (patterns.empty())
+    return false;
 
-  for (const auto& pattern : url_patterns_) {
+  for (const auto& pattern : patterns) {
     if (pattern.MatchesURL(url))
       return true;
   }
@@ -245,7 +256,29 @@ bool WebRequest::RequestFilter::MatchesType(
 
 bool WebRequest::RequestFilter::MatchesRequest(
     extensions::WebRequestInfo* info) const {
-  return MatchesURL(info->url) && MatchesType(info->web_request_type);
+  // Matches URL and type, and does not match exclude URL.
+  return MatchesURL(info->url, include_url_patterns_) &&
+         !MatchesURL(info->url, exclude_url_patterns_) &&
+         MatchesType(info->web_request_type);
+}
+
+void WebRequest::RequestFilter::AddUrlPatterns(
+    const std::set<std::string>& filter_patterns,
+    RequestFilter* filter,
+    gin::Arguments* args,
+    bool is_match_pattern) {
+  for (const std::string& filter_pattern : filter_patterns) {
+    URLPattern pattern(URLPattern::SCHEME_ALL);
+    const URLPattern::ParseResult result = pattern.Parse(filter_pattern);
+    if (result == URLPattern::ParseResult::kSuccess) {
+      filter->AddUrlPattern(std::move(pattern), is_match_pattern);
+    } else {
+      const char* error_type = URLPattern::GetParseResultString(result);
+      args->ThrowTypeError("Invalid url pattern " + filter_pattern + ": " +
+                           error_type);
+      return;
+    }
+  }
 }
 
 struct WebRequest::BlockedRequest {
@@ -315,7 +348,7 @@ gin::ObjectTemplateBuilder WebRequest::GetObjectTemplateBuilder(
 }
 
 const char* WebRequest::GetTypeName() {
-  return "WebRequest";
+  return GetClassName();
 }
 
 bool WebRequest::HasListener() const {
@@ -617,37 +650,44 @@ void WebRequest::SetListener(Event event,
                              gin::Arguments* args) {
   v8::Local<v8::Value> arg;
 
-  // { urls, types }.
-  std::set<std::string> filter_patterns, filter_types;
+  // { urls, excludeUrls, types }.
+  std::set<std::string> filter_include_patterns, filter_exclude_patterns,
+      filter_types;
+  RequestFilter filter;
+
   gin::Dictionary dict(args->isolate());
   if (args->GetNext(&arg) && !arg->IsFunction()) {
     // Note that gin treats Function as Dictionary when doing conversions, so we
     // have to explicitly check if the argument is Function before trying to
     // convert it to Dictionary.
     if (gin::ConvertFromV8(args->isolate(), arg, &dict)) {
-      if (!dict.Get("urls", &filter_patterns)) {
+      if (!dict.Get("urls", &filter_include_patterns)) {
         args->ThrowTypeError("Parameter 'filter' must have property 'urls'.");
         return;
       }
+
+      if (filter_include_patterns.empty()) {
+        util::EmitWarning(
+            "The urls array in WebRequestFilter is empty, which is deprecated. "
+            "Please use '<all_urls>' to match all URLs.",
+            "DeprecationWarning");
+        filter_include_patterns.insert("<all_urls>");
+      }
+
+      dict.Get("excludeUrls", &filter_exclude_patterns);
       dict.Get("types", &filter_types);
       args->GetNext(&arg);
     }
+  } else {
+    // If no filter is defined, create one with <all_urls> so it matches all
+    // requests
+    dict = gin::Dictionary::CreateEmpty(args->isolate());
+    filter_include_patterns.insert("<all_urls>");
+    dict.Set("urls", filter_include_patterns);
   }
 
-  RequestFilter filter;
-
-  for (const std::string& filter_pattern : filter_patterns) {
-    URLPattern pattern(URLPattern::SCHEME_ALL);
-    const URLPattern::ParseResult result = pattern.Parse(filter_pattern);
-    if (result == URLPattern::ParseResult::kSuccess) {
-      filter.AddUrlPattern(std::move(pattern));
-    } else {
-      const char* error_type = URLPattern::GetParseResultString(result);
-      args->ThrowTypeError("Invalid url pattern " + filter_pattern + ": " +
-                           error_type);
-      return;
-    }
-  }
+  filter.AddUrlPatterns(filter_include_patterns, &filter, args);
+  filter.AddUrlPatterns(filter_exclude_patterns, &filter, args, false);
 
   for (const std::string& filter_type : filter_types) {
     auto type = ParseResourceType(filter_type);

+ 12 - 3
shell/browser/api/electron_api_web_request.h

@@ -51,6 +51,8 @@ class WebRequest final : public gin::Wrappable<WebRequest>,
   static gin::Handle<WebRequest> From(v8::Isolate* isolate,
                                       content::BrowserContext* browser_context);
 
+  static const char* GetClassName() { return "WebRequest"; }
+
   // gin::Wrappable:
   static gin::WrapperInfo kWrapperInfo;
   gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
@@ -156,21 +158,28 @@ class WebRequest final : public gin::Wrappable<WebRequest>,
   class RequestFilter {
    public:
     RequestFilter(std::set<URLPattern>,
+                  std::set<URLPattern>,
                   std::set<extensions::WebRequestResourceType>);
     RequestFilter(const RequestFilter&);
     RequestFilter();
     ~RequestFilter();
 
-    void AddUrlPattern(URLPattern pattern);
+    void AddUrlPattern(URLPattern pattern, bool is_match_pattern);
+    void AddUrlPatterns(const std::set<std::string>& filter_patterns,
+                        RequestFilter* filter,
+                        gin::Arguments* args,
+                        bool is_match_pattern = true);
     void AddType(extensions::WebRequestResourceType type);
 
     bool MatchesRequest(extensions::WebRequestInfo* info) const;
 
    private:
-    bool MatchesURL(const GURL& url) const;
+    bool MatchesURL(const GURL& url,
+                    const std::set<URLPattern>& patterns) const;
     bool MatchesType(extensions::WebRequestResourceType type) const;
 
-    std::set<URLPattern> url_patterns_;
+    std::set<URLPattern> include_url_patterns_;
+    std::set<URLPattern> exclude_url_patterns_;
     std::set<extensions::WebRequestResourceType> types_;
   };
 

+ 51 - 0
spec/api-web-request-spec.ts

@@ -101,6 +101,12 @@ describe('webRequest module', () => {
       await expect(ajax(defaultURL)).to.eventually.be.rejected();
     });
 
+    it('matches all requests when no filters are defined', async () => {
+      ses.webRequest.onBeforeRequest(cancel);
+      await expect(ajax(`${defaultURL}nofilter/test`)).to.eventually.be.rejected();
+      await expect(ajax(`${defaultURL}nofilter2/test`)).to.eventually.be.rejected();
+    });
+
     it('can filter URLs', async () => {
       const filter = { urls: [defaultURL + 'filter/*'] };
       ses.webRequest.onBeforeRequest(filter, cancel);
@@ -109,6 +115,36 @@ describe('webRequest module', () => {
       await expect(ajax(`${defaultURL}filter/test`)).to.eventually.be.rejected();
     });
 
+    it('can filter all URLs with syntax <all_urls>', async () => {
+      const filter = { urls: ['<all_urls>'] };
+      ses.webRequest.onBeforeRequest(filter, cancel);
+      await expect(ajax(`${defaultURL}filter/test`)).to.eventually.be.rejected();
+      await expect(ajax(`${defaultURL}nofilter/test`)).to.eventually.be.rejected();
+    });
+
+    it('can filter URLs with overlapping patterns of urls and excludeUrls', async () => {
+      // If filter matches both urls and excludeUrls, it should be excluded.
+      const filter = { urls: [defaultURL + 'filter/*'], excludeUrls: [defaultURL + 'filter/test'] };
+      ses.webRequest.onBeforeRequest(filter, cancel);
+      const { data } = await ajax(`${defaultURL}filter/test`);
+      expect(data).to.equal('/filter/test');
+    });
+
+    it('can filter URLs with multiple excludeUrls patterns', async () => {
+      const filter = { urls: [defaultURL + 'filter/*'], excludeUrls: [defaultURL + 'filter/exclude1/*', defaultURL + 'filter/exclude2/*'] };
+      ses.webRequest.onBeforeRequest(filter, cancel);
+      expect((await ajax(`${defaultURL}filter/exclude1/test`)).data).to.equal('/filter/exclude1/test');
+      expect((await ajax(`${defaultURL}filter/exclude2/test`)).data).to.equal('/filter/exclude2/test');
+      // expect non-excluded URL to pass filter
+      await expect(ajax(`${defaultURL}filter/test`)).to.eventually.be.rejected();
+    });
+
+    it('can filter URLs with empty excludeUrls', async () => {
+      const filter = { urls: [defaultURL + 'filter/*'], excludeUrls: [] };
+      ses.webRequest.onBeforeRequest(filter, cancel);
+      await expect(ajax(`${defaultURL}filter/test`)).to.eventually.be.rejected();
+    });
+
     it('can filter URLs and types', async () => {
       const filter1: Electron.WebRequestFilter = { urls: [defaultURL + 'filter/*'], types: ['xhr'] };
       ses.webRequest.onBeforeRequest(filter1, cancel);
@@ -122,6 +158,21 @@ describe('webRequest module', () => {
       expect((await ajax(`${defaultURL}filter/test`)).data).to.equal('/filter/test');
     });
 
+    it('can filter URLs, excludeUrls and types', async () => {
+      const filter1: Electron.WebRequestFilter = { urls: [defaultURL + 'filter/*'], excludeUrls: [defaultURL + 'exclude/*'], types: ['xhr'] };
+      ses.webRequest.onBeforeRequest(filter1, cancel);
+
+      expect((await ajax(`${defaultURL}nofilter/test`)).data).to.equal('/nofilter/test');
+      expect((await ajax(`${defaultURL}exclude/test`)).data).to.equal('/exclude/test');
+      await expect(ajax(`${defaultURL}filter/test`)).to.eventually.be.rejected();
+
+      const filter2: Electron.WebRequestFilter = { urls: [defaultURL + 'filter/*'], excludeUrls: [defaultURL + 'exclude/*'], types: ['stylesheet'] };
+      ses.webRequest.onBeforeRequest(filter2, cancel);
+      expect((await ajax(`${defaultURL}nofilter/test`)).data).to.equal('/nofilter/test');
+      expect((await ajax(`${defaultURL}filter/test`)).data).to.equal('/filter/test');
+      expect((await ajax(`${defaultURL}exclude/test`)).data).to.equal('/exclude/test');
+    });
+
     it('receives details object', async () => {
       ses.webRequest.onBeforeRequest((details, callback) => {
         expect(details.id).to.be.a('number');