brave/adblock-rust

Filters containing `$$` don't parse correctly

Emi-TheDhamphirInLoveUnderTheFrozenStar opened this issue · 3 comments

Hey @antonok-edm, I found an issue with parsing the scriptlets values when it starts with $ symbol.

if the value in a scriptlet is for example: $remove$, the result will only be parsed as a single $

But If the value is $ remove, it parses as $ remove

The problem is set-local/session-storage-item scriptlets has an important feature to remove the items by using $remove$ value, and it won't work in Brave until this is not fixed.

The issue is exclusive to only $ symbol because I tested other symbols and everything was parsed as in the custom rule.
But, it can be tested with any scriptlet, because what matters is what Devtools scriptlets source displays.

So, for example, when using $whatever as value it should display as:
image

And like I said, if there is a space after the $ , then it looks fine, since it doesn't remove anything and displays as the custom rule. Useless but it works.
image

Also, when using $$remove$$, the scriptlet completely break, and it is like if the rule didn't exist at all, because the scriptlet doesn't even show in the sources devtools.

How to reproduce it:

  1. Add brave.com##+js(set-local-storage-item, Test, 0) as a custom rule or go to Brave, or use localStorage.setItem("Test", "0"); to create Test with value 0.
  2. now replace or add the custom rule with brave.com##+js(set-local-storage-item, Test, $remove$)
  3. go to Devtools -> Application -> Local Storage
  4. The local item is still there and doesn't get removed as it should.

@Emi-TheDhamphirInLoveUnderTheFrozenStar great catch, thanks!

I've narrowed it down to the use of Regex::replace - as per the docs, $ is interpreted as a special character;

To write a literal $ use $$.

By that logic, writing $$remove$$ should fix it, although as you've noted it gets completely ignored. That's ultimately caused by this piece of code to ignore AdGuard-specific $$ syntax.

Both cases can definitely be improved.

48b6b5e should take care of the first issue. I think the second is less likely to cause problems but I'll take another look at it again later.

The $$ issue should be fixed with 96911ee.