Extend "abort-*" scriptlets -- allow to check the stack trace

[ameshkov]

"abort-*" scriptlets are the source of numerous issues for several reasons, and I suppose the main one is that they don't allow filter maintainers to narrow them down to some specific part of the code we're trying to abort.

I suggest adding one more optional argument to the "abort-*" scriptlets so that we could use the current stack trace as a condition.

For example, here's how it would look like in the case of abort-on-property-read:

example.org#%#//scriptlet('abort-on-property-read', <property>, <stack>)

• stack (optional) - string or regular expression that must match the current function call stack trace. Please find the stack trace example below.

Here's a simple code snippet that shows how we should get the stack trace:

const stack = new Error().stack // get original stack trace
    .split('\n').slice(3). // get rid of our own functions in the stack trace
    map((line) => line.trim()) // trim the lines
    .join('\n') // join

Here's an example of the stack trace:

at e (https://example.org/test.js:123:12)
at e (https://example.org/testtest.js:233:132)
at e (https://example.org/testtesttesttest.js:2:194548)
at e (https://example.org/testtesttesttesttesttesttesttest.js:2555:1212)

For instance, if you want to abort the property read that happens in test.js only, you can add a rule like this:

example.org#%#//scriptlet('abort-on-property-read', 'Object.prototype.someProperty', '/test.js')

Additionally, we should extend debug-on-property-read and debug-on-property-write:

1. Add stack argument there too
2. Calculate the stack trace as it was explained above and print it to console.

@AdguardTeam/filters-maintainers thoughts? Would you like

[dimisa-RUAdList]

@wizmak
@mapx-
Obviously, this applies to all extensions using scriptlets.

[gorhill]

I had thought about the ability to use the stack information in the past for an abort scriptlet such as abort-current-script, but to test against stack information instead of against the script text as is the case with abort-current-inline-script -- so this is something I am already inclined to support.

However having to add yet another parameter to many existing scriptlets is concerning. I now have this feeling that scriptlet syntax should have been resembling more procedural cosmetic filter syntax: a string of one or more condition operators leading to an action, example (don't mind the syntax too much, I came up with it as I wrote, just the overall idea):

+js: match-property-read(...):match-stack(...):throw()

This way to add a new condition requiring testing the stack would be merely adding a new simple and well defined condition operator (match against the stack) without having to change the syntax of existing scriptlet operators and without overloading them with ever more parameters.

[ameshkov]

@gorhill

That would require massive refactoring and I'd better do that later.

But generally, I think that the idea is really bright. Here are some thoughts on this.

Currently, there are two important scriptlets groups:

• Override a property get/set and do something.
• Override a function call and do something.

The algorithm, in general, may look like this:

1. Override a property or a function.
2. Once it's called, check the "matchers" specified in the rule: stack trace, script content, function call arguments.
3. Apply one of the possible actions.

Here are some of the existing scriptlets written this way (don't mind the naming, I am coming up with this right now):

! noeval
function('window.eval'):match-argument(1, /regex/):noop()
! set-constant
property('window.test', 'get'):return-constant(true)
! log-eval
function('window.eval):log()'
! prevent-setInterval / no-setInterval-if
function('window.setInterval'):match-argument(1, /regex/):match-argument(2, 500):noop()
! json-prune
function('JSON.parse'):match-argument-json-props(1, 'someProp'):remove-json-props-and-return(1, 'someProp')

I am not sure about the JSON.parse example, though. Having the capability to apply this kind of modification to an arbitrary function call can be dangerous in the wrong hands.

This approach makes adding new matching criteria really easy. Just a couple of examples that can be useful: match-url, match-selector, etc.

Some scriptlets that cannot be categorized as "function", "property" or "matcher" will be categorized as "actions", and it should still be possible to apply some "matchers" to them. For instance:

! run remove-attr only on a specific page
:match-url('some-page-url'):remove-attr('onclick', 'selector>here')

[gorhill]

Yes, there is great potential with this approach, for both sides, code maintainers (because we don't have to keep complicating scriplet units) and filter crafters (because of freedom to compose complex scriptlets).

This approach makes adding new matching criteria really easy.

Just to add to this, I was made aware of a case where match-readystate() would also be useful.

That would require massive refactoring and I'd better do that later.

I think the best approach is to just slowly introduce a new syntax without refactoring the old one. Eventually the old one will be deprecated in some distant future.

[ameshkov]

Regarding match-readystate, I guess we also might need to introduce another category alongside "function" and "property".

Could be like this:

• readystate('complete'):remove-attr('blahblah') - run machers and the action on window.load
• observe-dom('selector'):remove-attr('blahblah') - run machers and the action when we are notified about dom changes

[ameshkov]

@gorhill

I moved the initial spec description to #83

I agree that this new version may be introduced alongside the old scriptlets, and I'd like us (AG) to start working on it after we finish with v1.3.

Also, I think it'd be convenient for everyone if we use the similar syntax for this type of rules in both uBO and AdGuard.

Let's work on the spec together. If you're okay with this, I'll write up a more detailed spec in the coming days, and we can discuss the details.

[gorhill]

If you're okay with this

Yes, having filter list maintenance efforts easily shared across blockers with normalized syntax benefits everybody.