fix: throw on invalid webRequest filters (#19580)

* 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.

* fix expect call test failure

atom/browser/api/atom_api_web_request.cc

@@ -4,6 +4,7 @@
 
 #include "atom/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 atom {
 
 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;

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)

spec/api-net-spec.js

@@ -889,6 +889,22 @@ describe('net module', () => {
       urlRequest.end()
     })
 
+    it('Should throw when invalid filters are passed to a request', () => {
+      assert.throws(() => {
+        session.defaultSession.webRequest.onBeforeRequest(
+          { urls: ['*://www.googleapis.com'] },
+          (details, callback) => { callback({ cancel: false }) }
+        )
+      }, 'Invalid url pattern *://www.googleapis.com: Empty path.')
+
+      assert.throws(() => {
+        session.defaultSession.webRequest.onBeforeRequest(
+          { urls: [ '*://www.googleapis.com/', '*://blahblah.dev' ] },
+          (details, callback) => { callback({ cancel: false }) }
+        )
+      }, 'Invalid url pattern *://blahblah.dev: Empty path.')
+    })
+
     it('should to able to create and intercept a request using a custom session object', (done) => {
       const requestUrl = '/requestUrl'
       const redirectUrl = '/redirectUrl'