fix: throw on invalid webRequest filters (#19337)

Closes #11371.

Previously, we didn't consider the return value of the webRequest URLPattern mate converter, which meant that when the pattern wasn't correctly parsed owing to invalid filter specification users would not be made aware of that fact and would just think that the filtering itself had failed. This corrects that error by moving the business logic of url pattern parsing out of the converter and into the function itself so that granular and specific errors can be thrown.

There's also no real reason that i'm aware of not to allow wider breadth of filters by letting users use a wildcard for effective TLD, so I also overrode that (default for the 1-arg Parse is not to allow that).

Finally, I added some examples of url filter types for users to reference.

docs/api/web-request.md

@@ -68,6 +68,21 @@ The `uploadData` is an array of `UploadData` objects.
 
 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'
+'*://*/*'
+'*://example.com/*'
+'*://example.com/foo/*'
+'http://*.foo:1234/'
+'file://foo:1234/bar'
+'http://foo:*/'
+'*://www.foo.com/'
+```
+
 #### `webRequest.onBeforeSendHeaders([filter, ]listener)`
 
 * `filter` Object (optional)

shell/browser/api/atom_api_web_request.cc

@@ -4,6 +4,7 @@
 
 #include "shell/browser/api/atom_api_web_request.h"
 
+#include <set>
 #include <string>
 #include <utility>
 
@@ -20,23 +21,6 @@
 
 using content::BrowserThread;
 
-namespace mate {
-
-template <>
-struct Converter<URLPattern> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> val,
-                     URLPattern* out) {
-    std::string pattern;
-    if (!ConvertFromV8(isolate, val, &pattern))
-      return false;
-    *out = URLPattern(URLPattern::SCHEME_ALL);
-    return out->Parse(pattern) == URLPattern::ParseResult::kSuccess;
-  }
-};
-
-}  // namespace mate
-
 namespace electron {
 
 namespace api {
@@ -84,7 +68,25 @@ void WebRequest::SetListener(Method method, Event type, mate::Arguments* args) {
   // { urls }.
   URLPatterns patterns;
   mate::Dictionary dict;
-  args->GetNext(&dict) && dict.Get("urls", &patterns);
+  std::set<std::string> filter_patterns;
+
+  if (args->GetNext(&dict) && !dict.Get("urls", &filter_patterns)) {
+    args->ThrowError(
+        "onBeforeRequest parameter 'filter' must have property 'urls'.");
+    return;
+  }
+
+  URLPattern pattern(URLPattern::SCHEME_ALL);
+  for (const std::string& filter_pattern : filter_patterns) {
+    const URLPattern::ParseResult result = pattern.Parse(filter_pattern);
+    if (result == URLPattern::ParseResult::kSuccess) {
+      patterns.insert(pattern);
+    } else {
+      const char* error_type = URLPattern::GetParseResultString(result);
+      args->ThrowError("Invalid url pattern " + filter_pattern + ": " +
+                       error_type);
+    }
+  }
 
   // Function or null.
   v8::Local<v8::Value> value;

spec-main/api-net-spec.ts

@@ -633,6 +633,31 @@ describe('net module', () => {
         session.defaultSession.webRequest.onBeforeRequest(null)
       })
 
+      it('Should throw when invalid filters are passed', () => {
+        expect(() => {
+          session.defaultSession.webRequest.onBeforeRequest(
+            { urls: ['*://www.googleapis.com'] },
+            (details, callback) => { callback({ cancel: false }) }
+          )
+        }).to.throw('Invalid url pattern *://www.googleapis.com: Empty path.')
+
+        expect(() => {
+          session.defaultSession.webRequest.onBeforeRequest(
+            { urls: [ '*://www.googleapis.com/', '*://blahblah.dev' ] },
+            (details, callback) => { callback({ cancel: false }) }
+          )
+        }).to.throw('Invalid url pattern *://blahblah.dev: Empty path.')
+      })
+
+      it('Should not throw when valid filters are passed', () => {
+        expect(() => {
+          session.defaultSession.webRequest.onBeforeRequest(
+            { urls: ['*://www.googleapis.com/'] },
+            (details, callback) => { callback({ cancel: false }) }
+          )
+        }).to.not.throw()
+      })
+
       it('Requests should be intercepted by webRequest module', (done) => {
         const requestUrl = '/requestUrl'
         const redirectUrl = '/redirectUrl'