From 5a641e02ee42fc322f33728ca1f8892e8173e860 Mon Sep 17 00:00:00 2001 From: Alice Zhao Date: Thu, 20 Feb 2025 12:54:58 -0800 Subject: [PATCH] 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 --- docs/api/structures/web-request-filter.md | 5 +- docs/api/web-request.md | 1 + docs/breaking-changes.md | 16 ++++ shell/browser/api/electron_api_web_request.cc | 94 +++++++++++++------ shell/browser/api/electron_api_web_request.h | 15 ++- spec/api-web-request-spec.ts | 51 ++++++++++ 6 files changed, 150 insertions(+), 32 deletions(-) diff --git a/docs/api/structures/web-request-filter.md b/docs/api/structures/web-request-filter.md index 13a70663dd4b..480a94f7b948 100644 --- a/docs/api/structures/web-request-filter.md +++ b/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 `` 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`. diff --git a/docs/api/web-request.md b/docs/api/web-request.md index 062084831d67..c8ee3cd1e4bf 100644 --- a/docs/api/web-request.md +++ b/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 +'' 'http://foo:1234/' 'http://foo.com/' 'http://foo:1234/bar' diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index b2ba4ef3a137..4d2f01980141 100644 --- a/docs/breaking-changes.md +++ b/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 `` 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: [''] +} +``` + ### Deprecated: `systemPreferences.isAeroGlassEnabled()` The `systemPreferences.isAeroGlassEnabled()` function has been deprecated without replacement. diff --git a/shell/browser/api/electron_api_web_request.cc b/shell/browser/api/electron_api_web_request.cc index e71f68f96a5b..0410c9683dce 100644 --- a/shell/browser/api/electron_api_web_request.cc +++ b/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 url_patterns, + std::set include_url_patterns, + std::set exclude_url_patterns, std::set 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& 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& 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 arg; - // { urls, types }. - std::set filter_patterns, filter_types; + // { urls, excludeUrls, types }. + std::set 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 '' to match all URLs.", + "DeprecationWarning"); + filter_include_patterns.insert(""); + } + + dict.Get("excludeUrls", &filter_exclude_patterns); dict.Get("types", &filter_types); args->GetNext(&arg); } + } else { + // If no filter is defined, create one with so it matches all + // requests + dict = gin::Dictionary::CreateEmpty(args->isolate()); + filter_include_patterns.insert(""); + 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); diff --git a/shell/browser/api/electron_api_web_request.h b/shell/browser/api/electron_api_web_request.h index d7a78c3e591e..2114444aae7d 100644 --- a/shell/browser/api/electron_api_web_request.h +++ b/shell/browser/api/electron_api_web_request.h @@ -51,6 +51,8 @@ class WebRequest final : public gin::Wrappable, static gin::Handle 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, class RequestFilter { public: RequestFilter(std::set, + std::set, std::set); RequestFilter(const RequestFilter&); RequestFilter(); ~RequestFilter(); - void AddUrlPattern(URLPattern pattern); + void AddUrlPattern(URLPattern pattern, bool is_match_pattern); + void AddUrlPatterns(const std::set& 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& patterns) const; bool MatchesType(extensions::WebRequestResourceType type) const; - std::set url_patterns_; + std::set include_url_patterns_; + std::set exclude_url_patterns_; std::set types_; }; diff --git a/spec/api-web-request-spec.ts b/spec/api-web-request-spec.ts index 2a276afaff2e..b2792cf9a66d 100644 --- a/spec/api-web-request-spec.ts +++ b/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 ', async () => { + const filter = { 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');