-
Notifications
You must be signed in to change notification settings - Fork 140
Using backslash (\) to escape special characters doesn't work properly in Brave adblocker #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@TheVampireInLoveWithTheCorpsesBlood how are you adding the 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 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 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, 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: 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 This is the link to test of a link that (by default) should be sanitized and it is not: 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! |
backslashes should be handled by #281 as well! |
Oh, nice to know it is more than just quotes. |
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.
The text was updated successfully, but these errors were encountered: