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.
This commit is contained in:
parent
65648756b5
commit
49096c2359
3 changed files with 60 additions and 18 deletions
|
@ -68,6 +68,21 @@ The `uploadData` is an array of `UploadData` objects.
|
||||||
|
|
||||||
The `callback` has to be called with an `response` object.
|
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)`
|
#### `webRequest.onBeforeSendHeaders([filter, ]listener)`
|
||||||
|
|
||||||
* `filter` Object (optional)
|
* `filter` Object (optional)
|
||||||
|
|
|
@ -4,6 +4,7 @@
|
||||||
|
|
||||||
#include "shell/browser/api/atom_api_web_request.h"
|
#include "shell/browser/api/atom_api_web_request.h"
|
||||||
|
|
||||||
|
#include <set>
|
||||||
#include <string>
|
#include <string>
|
||||||
#include <utility>
|
#include <utility>
|
||||||
|
|
||||||
|
@ -20,23 +21,6 @@
|
||||||
|
|
||||||
using content::BrowserThread;
|
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 electron {
|
||||||
|
|
||||||
namespace api {
|
namespace api {
|
||||||
|
@ -84,7 +68,25 @@ void WebRequest::SetListener(Method method, Event type, mate::Arguments* args) {
|
||||||
// { urls }.
|
// { urls }.
|
||||||
URLPatterns patterns;
|
URLPatterns patterns;
|
||||||
mate::Dictionary dict;
|
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.
|
// Function or null.
|
||||||
v8::Local<v8::Value> value;
|
v8::Local<v8::Value> value;
|
||||||
|
|
|
@ -633,6 +633,31 @@ describe('net module', () => {
|
||||||
session.defaultSession.webRequest.onBeforeRequest(null)
|
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) => {
|
it('Requests should be intercepted by webRequest module', (done) => {
|
||||||
const requestUrl = '/requestUrl'
|
const requestUrl = '/requestUrl'
|
||||||
const redirectUrl = '/redirectUrl'
|
const redirectUrl = '/redirectUrl'
|
||||||
|
|
Loading…
Reference in a new issue