Using backslash (\) to escape special characters doesn't work properly in Brave adblocker
Closed this issue · 7 comments
I noticed this problem which shouldn't be crucial for most people but it would be nice if it gets fixed eventually, it applies to both, Custom filters and Internal/Default Lists.
But, I use uBo-Scriptlets and they have some nice scriptlets.
When I was testing the ones to add Local storage items in Brave, I noticed that I can't get Brave to use the quotes symbol in a string.
This issue seems to only apply to backslash, and single and double quotes, so it is like the adblocker just decided to ignore any quote or double backslash (to add one backslash) in the rules.
Since many Local Storage items are JSON, they would need the quotes to work properly.
For example if I add the scritplet for setLocalItem to Brave and then use brave.com##+js(sli, test, {\"m-tcon\":true\,\"m-other\":true\,\"m-owl\":true\,\"m-game\":true\,\"m-ffz\":true\,\"m-addon\":true}) the quotes get ignored even if technically it should just work and don't act as normal quotes.
Using the backslash with the commas works as expected though, since the backslash is needed so the commas don't act as individual arguments for the scriptlet which would crash the browser if too many are in the rule.
So, it is not a huge issue and my workaround was to modify the scriptlet and use replace() and then it replaces the | to quotes symbol, so it is easy, but it would be nice if it just worked just by adding the backslashes to quotes (or without using backslashes for these symbols like uBlock does it automatically) instead of modifying the scriptlet.
Thank you.
@TheVampireInLoveWithTheCorpsesBlood how are you adding the setLocalItem scriptlet to Brave? Is that by manually modifying the resources.json file? I'm impressed if that's the case, but I will warn you that those would get overwritten whenever there are resource updates. We do need to make it user-serviceable in the browser, at some point (brave/brave-browser#25586).
The scriptlet argument parsing is all done here. I believe I was extra cautious when writing it initially, in case somebody would try to escape the string. Should be a quick PR if you're interested in writing one; otherwise I'll get to it by the time we have official support for custom scriptlets in Brave.
@antonok-edm yes, that's what I did. I found out by reading the files to see how Brave got the resources from uBlock and the Adblock lists, how easy is to modify Brave to do cool things or disable internal lists etc etc.
I appreciate the info, when I found out how easy it was to load scriptlets in Brave I made sure to have a separated file with the scriptlets ready to be copied and pasted so it takes 2 seconds to update when/if it gets overwritten.
I even decided to drop the uBo-scriptlets set local/session item in favor of the ones pes10k did from Adguard since they are faster, I only modified them to remove the restriction about what value that can be set, so I can set it to anything I want, plus I can set multiple items with a same value in a single rule. I already modified the set-cookie to have any value and specify the domain in case the cookie has to be set in .domain instead of www. or something, so yeah, not really patient enough to wait for the official support, so I was happy to find out how easy and useful the workaround to use use scriptlets in Brave was, until Brave gets the official custom scriptlets feature.
I see, I wish I could say I would/could do it, but I am not that adventurous or confident in my knowledge to do it myself, maybe as a personal thing because coding it's mostly passion-side-projects I got thanks to Houdini, which made me want to learn python, and also reading computer languages got me interested in general, to understand and experiment with them and know what the code is doing most of the time.
Also, this would make more sense to be 'fixed' when custom scriptlet support gets officially added in Brave, since nobody needs this, but it's good to know you are now aware of it since it is something that might be useful in the future for someone besides me.
Thanks for your work and have a good day!
Since href-sanitizer in the scriptlet.js from uBlock was merged in brave/brave-core-crx-packager#582
I feel like I have to tell you that the scriplet is useless in Brave because of this same issue, since it needs an attribute as selectors and attributes need the quotation marks, then having a[href^="https://t.co/"] will not work.
Because of this issue with href-sanitizer, I realized that some scriptlet injection rules from the uBlock lists, mostly remove-attr (ra), wouldn't work in Brave either, because uBlock does the escaping internally, \" \' \\ etc, so even if chracters can be escaped with \, it means they wouldn't be compatible with rules in the uBlock lists.
Using a simple (##\+js).*["] regex, I found 67 scriptlet injections that use quotation marks that wouldn't work like in:
magnetdl.com,magnetdl.org##+js(ra, href, a[href="https://vpn-choice.com"]) or fzm.*,fzmovies.*##+js(ra, onclick, [onclick*="window.open"])
not critical but I had to mention how real rules being used in lists are affected by this limitation.
So, to make href-sanitizer work I had to do the trick of changing the scriptlets to make them work by adding .replace(), I even opted to replace emojis rather than normal characters since it works, it is easier to identify, manage and I know no rule will use them, so now my rules look like: twitter.com##+js(href-sanitizer, a[href^=🐓https://t.co/🐓]).
I even went ahead and changed the scriptlet because for whatever whatever reason gorhill decided to only allow https links, so a link like this won't be 'sanitized' but once you add https? to the scriptlet and the replace, it does the sanitizer in http or https just fine as it is meant to be done in Brave.
This is the link to test of a link that (by default) should be sanitized and it is not:
https://twitter.com/lopatonok/status/1634326102947815424
Small issue I already workarounded, and probably most of those 67 rules are invalid by now (I found some more that aren't working but most were invalid by now), but just wanted to mention about how href-sanitizer wouldn't work in Brave. And even if I don't know if uBlock will add it to a list anytime soon (since it is experimental anyway), then still is good to mention how this issue will limit its usage, maybe someone besides me is going to use it inside Brave!
Thank you and have a good day!
@Emi-TheDhamphirInLoveUnderTheFrozenStar thanks, let's get this thing fixed finally - #281
Thanks Anton, you are the best!
Cute roosters and Ghost emojis will be missed, but this is good for Brave and its compatibility with uBlock scriptlets. 👍
backslashes should be handled by #281 as well!
Oh, nice to know it is more than just quotes.
Have a nice day Anton! 👌