brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.62k stars 2.29k forks source link

[trusted-]replace-node-text scriptlets operate on themselves first #39116

Closed Yuki2718 closed 3 months ago

Yuki2718 commented 3 months ago

https://github.com/gorhill/uBlock/wiki/Resources-Library#trusted-rpntjs- And thus new rules for yt's SSAP ads don't work on Brave. Probably other varargs and tokens should be checked to ensure compatibility with uBlock filters.

Edit (@ShivanKaul): we investigated and found the real reason why sedCount, 1 doesn't work: https://github.com/brave/brave-browser/issues/39116#issuecomment-2174408736.

ryanbr commented 3 months ago

From chat;

, sedCount, 1 is making the rule incompatible to Brave.

From:

m.youtube.com,music.youtube.com,www.youtube.com##+js(trusted-rpnt, script, (function() {window.ytplayer=, /*start*/(()=>{let t=document.location.href\,e=[]\,n=[]\,o=!1;const r=Array.prototype.push\,i={apply:(t\,i\,a)=>(window.yt?.config_?.EXPERIMENT_FLAGS?.html5_enable_ssap_entity_id&&a[0]&&a[0]!==window&&"number"==typeof a[0].start&&a[0].end&&"ssap"===a[0].namespace&&a[0].id&&(o||0!==a[0]?.start||n.includes(a[0].id)||(e.length=0\,n.length=0\,o=!0\,r.call(e\,a[0])\,r.call(n\,a[0].id))\,o&&0!==a[0]?.start&&!n.includes(a[0].id)&&(r.call(e\,a[0])\,r.call(n\,a[0].id)))\,Reflect.apply(t\,i\,a))};window.Array.prototype.push=new Proxy(window.Array.prototype.push\,i)\,document.addEventListener("DOMContentLoaded"\,(function(){if(!window.yt?.config_?.EXPERIMENT_FLAGS?.html5_enable_ssap_entity_id)return;const r=()=>{const t=document.querySelector("video");if(t&&e.length){const r=Math.round(t.duration)\,i=Math.round(e.at(-1).end/1e3);if(0===t.currentTime&&r&&r===i){const r=Math.ceil(e.at(-1).start/1e3);t.currentTime<r&&(t.currentTime=r\,o=!1\,e.length=0\,n.length=0)}}};r();new MutationObserver((()=>{t!==document.location.href?(t=document.location.href\,e.length=0\,n.length=0\,o=!1\,r()):r()})).observe(document\,{childList:!0\,subtree:!0})}))})();document.currentScript.textContent=document.currentScript.textContent.replace(/\/\*start\*\/(.*)\/\*end\*\//g\,"");/*end*/(function() {window.ytplayer=, sedCount, 1)

You have to add the rule without this part to Unbreak until it gets fixed by Brave devs.

ghost commented 3 months ago

I deleted my comment, but I decided to troubleshoot it further to see why in my tests it was working but not Youtube, so, like I said sedCount works, but for some reason it is not working correctly in script nodes, and that's the issue here.

The thing is, sedCount, 1 is the problem with script nodes, but it works fine on every other node. So it seems like with script nodes it is only done with a value of 0 or 2 or more, but not 1.

Just make this test, go to any website like brave.com, and use the same rule, just remove the pattern since we will match any node. You will see sedCount, 1 does nothing when using script node, but 2 works, and if you change the node from script to whatever else, it will work with sedCount, 1.

So something is happening that is causing that issue with script node, but the sedCount argument is not exactly the problem here and it is supported.

I also tested copying and pasting the same code uBlock injects as a scriptlet in Brave Adblocker, and Brave has the same issue. I had to find the replaceNodeTextFn(nodeName, pattern, replacement, ...extraArgs); and change sedCount, for it to work as uBlock, just to make things more clear.

And to make things more evident, something like cgpress.org##+js(trusted-rpnt, script, , console.log("test");, sedCount, 1) won't display any test in console, it is until sedCount is set to 2, which will only show one test and inject it in the first script node it found in head.

ShivanKaul commented 3 months ago
sedCount, n: This will cause the scriptlet to stop after n instances of substitution. Since a mutation oberver is used by the scriptlet, it's advised to stop it whenever it becomes pointless. Default to zero, which means the scriptlet never stops

So to clarify, if we just set sedCount, 2 for this particular substitution in https://github.com/brave/brave-browser/issues/39116, that would make the scriptlet work? What's the harm in doing that? @ryanbr @antonok-edm

antonok-edm commented 3 months ago

This doesn't look like an issue specific to adblock-rust if you can reproduce it by copy-pasting code into the console. Will move to brave-browser.

antonok-edm commented 3 months ago

After some investigation, it appears that the script tag in the DOM that was injected to execute the scriptlet logic is being replaced by the scriptlet logic. i.e. the scriptlet is replacing itself.

I've verified this by adding log, 1 in the scriptlet arguments. Brave doesn't yet have native support for logging so I manually modified the trusted-rpnt scriptlet from resources.json in the profile directory to call console.log in uboLog, and remove the check for logLevel for the Text before: line.

It's unclear how uBO doesn't run into this issue, since they also use the same injection method of creating a script tag, appending to the head, and immediately removing it. I assume it's because Brave can inject at an even earlier stage than uBO on Chromium. We probably don't want to change that since it does improve our reliability in many cases.

There's no immediately obvious fix for this. A check for document.currentScript in the trusted-rpnt scriptlet itself might be helpful, but can we get it accepted upstream?

     let sedCount = extraArgs.sedCount || 0;
     const handleNode = node => {
+        if (node === document.currentScript) {
+            return true;
+        }
         const before = node.textContent;
         reCondition.lastIndex = 0;

https://github.com/gorhill/uBlock/blob/master/assets/resources/scriptlets.js#L727-L730

As a workaround, having no sedCount (or setting it to 0) will avoid the issue by applying to every script on the page, or otherwise incrementing it by 1 will cause it to deal with the injected scriptlet before proceeding with the normal iteration count.

antonok-edm commented 3 months ago

Should be fixed as of Brave Ad Block Resources Library - Version: 1.0.91 in brave://components as of https://github.com/gorhill/uBlock/commit/cb0f65e035e4cd40b0127079ff1fca76ee964fa0.

Thanks again for reporting! Let me know if y'all see any other issues with this.