11.0.41

Bug reports and enhancement requests
skriptimaahinen
Master Bug Buster
Posts: 244
Joined: Wed Jan 10, 2018 7:37 am

11.0.41

Post by skriptimaahinen »

Interesting update again. Some remarks.

The event attribute removal would appear to be redundant now that there is a method to "suppress" them.

However, the list of the eventTypes is not complete. For example beforescriptexecute is a valid event, that could be put e.g. in head and would currently slip by if the head is parsed prematurely. Unfortunately I have no good idea how to obtain list of all on-events.

And slightly more profound problem. On some instances, onload event handlers fail to run.

This is caused by the body getting parsed before the permissions are fetched, which can happen because the mutationobserver based suspending does not work reliably anymore.

Suspending on beforescriptexecute does seem to still block body parsing, ASSUMING that there is a script tag in the head. (Dynamically added scripts appear not to block DOM parsing when suspended.) So even that would not cover all cases. Unfortunately can't give any solutions here either.

---

Considering how much effort has already been put into trying to get this work properly, I'm going to suggest an alternative approach. That does come with a downside.

Register the permissions as a contentscript. That way the permissions are definitely there at document-start without relying on any hacks.

Downside being that there would be no tabId info available, which means that "Disable restrictions for this tab" could not work anymore. Also it would further deviate Firefox and Chromium code bases.

Question is: would that be acceptable trade off?

There could be alternatives like "Disable restrictions for this site", in which the restrictions are removed based on originUrl/documentUrl. However, I do not use this feature and have no idea what kind of use cases people have for it and what would be good replacement. Discuss, please.
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0
User avatar
Giorgio Maone
Site Admin
Posts: 9454
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: 11.0.41

Post by Giorgio Maone »

skriptimaahinen wrote: Wed Aug 26, 2020 10:14 amThe event attribute removal would appear to be redundant now that there is a method to "suppress" them.
It's not, actually. We remove the event suppression as soon as we install the CSP (in order to avoid unespected side effects in the "normal" user interaction with a scriptless page), and this could be abused by including a very slow loading script / stylesheet / whatever in the head causing the onload event fire after the event suppression handler has been removed.
skriptimaahinen wrote: Wed Aug 26, 2020 10:14 am However, the list of the eventTypes is not complete. For example beforescriptexecute is a valid event, that could be put e.g. in head and would currently slip by if the head is parsed prematurely. Unfortunately I have no good idea how to obtain list of all on-events.
I'll look into that. It's less elegant, but as a last resort I can use the same list I retrieve from Firefox's code base on each build to keep the XSS filter up to date.
skriptimaahinen wrote: Wed Aug 26, 2020 10:14 am And slightly more profound problem. On some instances, onload event handlers fail to run.

This is caused by the body getting parsed before the permissions are fetched, which can happen because the mutationobserver based suspending does not work reliably anymore.
Have you got any idea of why this changed, and most importantly a test case to bang my head on? Thanks.
skriptimaahinen wrote: Wed Aug 26, 2020 10:14 am Register the permissions as a contentscript. That way the permissions are definitely there at document-start without relying on any hacks.
Been there, done that months (years?) ago. I had to back off from it for the tab permissions problem, and I'd really prefer not to go back (unless both Chrome & Firefox agree to implement it with a tabId filter, as I've suggested here).
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0
skriptimaahinen
Master Bug Buster
Posts: 244
Joined: Wed Jan 10, 2018 7:37 am

Re: 11.0.41

Post by skriptimaahinen »

Giorgio Maone wrote: Wed Aug 26, 2020 11:33 am It's not, actually. We remove the event suppression as soon as we install the CSP (in order to avoid unespected side effects in the "normal" user interaction with a scriptless page), and this could be abused by including a very slow loading script / stylesheet / whatever in the head causing the onload event fire after the event suppression handler has been removed.
I meant the removal from documentElement alone. The document.querySelectorAll("*") method should cover that too.
Giorgio Maone wrote: Wed Aug 26, 2020 11:33 am
Have you got any idea of why this changed, and most importantly a test case to bang my head on? Thanks.
No idea. This is pretty much undocumented behaviour so I guess there is no guarantee that it remains the same. Or could be that it has always been like this and I failed to make refined enough test cases, drawing wrong conclusions. And I still might be making some. Currently I can't explain why sometimes the body gets parsed first and sometimes our scripts run before.

One example that seems to have the body parsed quickly is this one viewtopic.php?f=7&t=26061#p102698

Other is the html taken from https://webdriver-herald.herokuapp.com/. Just put it in local file. The external js and css are not needed.

Also things that help bring out the behaviour are:
  • Navigate to about:newtab and back with the navigation arrow.
  • Close tab and restore it.
  • Close Firefox and let session restore bring back the page.
Giorgio Maone wrote: Wed Aug 26, 2020 11:33 am
Been there, done that months (years?) ago. I had to back off from it for the tab permissions problem, and I'd really prefer not to go back (unless both Chrome & Firefox agree to implement it with a tabId filter, as I've suggested here).
Yes, something of this sort was tried back in Noscript 10. But like I said, it would be a trade off. Question is, which one is less pain. Unless your proposal goes through. That would be ideal.
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0
barbaz
Senior Member
Posts: 10841
Joined: Sat Aug 03, 2013 5:45 pm

(11.0.40rc2 and up) Local files external JS files sometimes cannot listen on DOMContentLoaded

Post by barbaz »

I have file:// set to Trusted. With NoScript 11.0.40rc1 and earlier, in this configuration my local HTML files can run scripts fine. In 11.0.40rc2 and later, including in 11.0.41rc2, some of my local files are broken.

I can't reproduce this 100% consistently. Seems to be more reproducible when opening the file with Firefox, rather than opening Firefox then dragging the file into it. Reloading the page mostly doesn't help, although occasionally gets it working.

I'm using Firefox 80 Dev Edition.

Possible test case:

Code: Select all

<!DOCTYPE html>
<html>
<head>
  <meta charset="UTF-8"/>
  <script src="Foo.js"></script>
</head>
<body>Foo foo foo foo</body></html>
Foo.js

Code: Select all

window.addEventListener('DOMContentLoaded', function() {
  let a = document.createElement('code');
  a.textContent = 'Script worked';
  document.body.appendChild(a);
}, false);
*Always* check the changelogs BEFORE updating that important software!
-
skriptimaahinen
Master Bug Buster
Posts: 244
Joined: Wed Jan 10, 2018 7:37 am

Re: (11.0.40rc2 and up) Local files external JS files sometimes cannot listen on DOMContentLoaded

Post by skriptimaahinen »

Pointed this problem out in my 11.0.41 thread already.

Giorgio is working on it, but I'm afraid it's not a easy one.
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0
barbaz
Senior Member
Posts: 10841
Joined: Sat Aug 03, 2013 5:45 pm

Re: (11.0.40rc2 and up) Local files external JS files sometimes cannot listen on DOMContentLoaded

Post by barbaz »

skriptimaahinen wrote: Fri Aug 28, 2020 4:17 am Pointed this problem out in my 11.0.41 thread already.
Threads merged.
*Always* check the changelogs BEFORE updating that important software!
-
User avatar
Giorgio Maone
Site Admin
Posts: 9454
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: 11.0.41

Post by Giorgio Maone »

Please check latest development build, thanks.
v 11.0.42rc1
============================================================
x Refactored and improved syncFetchPolicy fallback for file:
and ftp: special cases
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0
barbaz
Senior Member
Posts: 10841
Joined: Sat Aug 03, 2013 5:45 pm

Re: 11.0.41

Post by barbaz »

11.0.42rc1 is slightly better, but the problem I reported still persists.
*Always* check the changelogs BEFORE updating that important software!
-
User avatar
Giorgio Maone
Site Admin
Posts: 9454
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: 11.0.41

Post by Giorgio Maone »

barbaz wrote: Fri Aug 28, 2020 7:37 pm 11.0.42rc1 is slightly better, but the problem I reported still persists.
Please check 11.0.42rc2, with NoScript Options>Advanced>Debug enabled, and annotate the content console (Ctrl+Shift+K) output.
Thank you!
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0
skriptimaahinen
Master Bug Buster
Posts: 244
Joined: Wed Jan 10, 2018 7:37 am

Re: 11.0.41

Post by skriptimaahinen »

Event attribute suppression looking good.

And to test the suspending on DOMContentLoaded:

Document used for testing:

Code: Select all

<!DOCTYPE html>
<html>
<head>      
    <meta charset='UTF-8'>
</head>
<body>
    <script src="script.js"></script>
</body>
</html>
script.js:

Code: Select all

console.log("External script hello!");
window.onload = function(event) {
    console.log("onload"); 
};
document.addEventListener('DOMContentLoaded', function(event) {  
    console.log("DOMContentLoaded"); 
});
Could not get any ouput from the "bodySuspender" unless I used document.addEventListener in SyncMessage.js. With that I got the outputs below:

Code: Select all

sendSyncMessage suspend #0/1                           SyncMessage.js:231:19
sendSyncMessage suspending on Array(6) [ MutationRecord, MutationRecord, MutationRecord, MutationRecord, MutationRecord, MutationRecord ] SyncMessage.js:244:19
sendSyncMessage suspend #1/2                           SyncMessage.js:231:19
sendSyncMessage suspending on Array [ MutationRecord ] SyncMessage.js:244:19
sendSyncMessage suspend #2/3                           SyncMessage.js:231:19
sendSyncMessage finalizing                             SyncMessage.js:256:19
sendSyncMessage resume #2/2 - 55ms                     SyncMessage.js:240:19
sendSyncMessage resume #1/1 - 55ms                     SyncMessage.js:240:19
sendSyncMessage resume #0/0 - 56ms                     SyncMessage.js:240:19
sendSyncMessage finalizing                             SyncMessage.js:256:19
Suspending on DOMContentLoaded                         SyncMessage.js:250:19
sendSyncMessage suspend #3/1                           SyncMessage.js:231:19
External script hello!                                 script.js:1:9
sendSyncMessage resume #3/0 - 71ms                     SyncMessage.js:240:19
onload                                                 script.js:3:13

Code: Select all

sendSyncMessage suspend #0/1                           SyncMessage.js:231:19
sendSyncMessage suspending on Array(6) [ MutationRecord, MutationRecord, MutationRecord, MutationRecord, MutationRecord, MutationRecord ] SyncMessage.js:244:19
sendSyncMessage suspend #1/2                           SyncMessage.js:231:19
sendSyncMessage finalizing                             SyncMessage.js:256:19
sendSyncMessage resume #1/1 - 92ms                     SyncMessage.js:240:19
sendSyncMessage resume #0/0 - 95ms                     SyncMessage.js:240:19
sendSyncMessage finalizing                             SyncMessage.js:256:19
External script hello!                                 script.js:1:9
Suspending on DOMContentLoaded                         SyncMessage.js:250:19
sendSyncMessage suspend #2/1                           SyncMessage.js:231:19
sendSyncMessage resume #2/0 - 139ms                    SyncMessage.js:240:19
DOMContentLoaded                                       script.js:6:13
onload                                                 script.js:3:13

Code: Select all

sendSyncMessage suspend #0/1                           SyncMessage.js:231:19
sendSyncMessage suspending on Array(6) [ MutationRecord, MutationRecord, MutationRecord, MutationRecord, MutationRecord, MutationRecord ] SyncMessage.js:244:19
sendSyncMessage suspend #1/2                           SyncMessage.js:231:19
sendSyncMessage finalizing                             SyncMessage.js:256:19
sendSyncMessage resume #1/1 - 20ms                     SyncMessage.js:240:19
sendSyncMessage resume #0/0 - 23ms                     SyncMessage.js:240:19
sendSyncMessage finalizing                             SyncMessage.js:256:19
Suspending on DOMContentLoaded                         SyncMessage.js:250:19
sendSyncMessage suspend #2/1                           SyncMessage.js:231:19
External script hello!                                 script.js:1:9
sendSyncMessage resume #2/0 - 59ms                     SyncMessage.js:240:19
onload                                                 script.js:3:13
Onload fires pretty reliably but DOMContentLoaded somewhat rarely.
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0
barbaz
Senior Member
Posts: 10841
Joined: Sat Aug 03, 2013 5:45 pm

Re: 11.0.41

Post by barbaz »

Giorgio Maone wrote: Sat Aug 29, 2020 6:24 am Please check 11.0.42rc2, with NoScript Options>Advanced>Debug enabled, and annotate the content console (Ctrl+Shift+K) output.
Thank you!
Better still, but I still see the issue. So here's the console output when the issue occurs, on my local file that seems to hit this issue the most.

Code: Select all

sendSyncMessage suspend #0/1 SyncMessage.js:231:19
sendSyncMessage suspending on  
(2) […]

0: MutationRecord

addedNodes: NodeList [ script ]

attributeName: null

attributeNamespace: null

nextSibling: null

oldValue: null

previousSibling: <head>
removedNodes: NodeList []

target: <html>

type: "childList"

<prototype>: MutationRecordPrototype { type: Getter, target: Getter, addedNodes: Getter, … }

1: MutationRecord

addedNodes: NodeList []

attributeName: null

attributeNamespace: null

nextSibling: null

oldValue: null

previousSibling: <head>
removedNodes: NodeList [ script ]

target: <html>

type: "childList"

<prototype>: MutationRecordPrototype { type: Getter, target: Getter, addedNodes: Getter, … }

length: 2

<prototype>: Array []
SyncMessage.js:244:19
sendSyncMessage suspend #1/2 SyncMessage.js:231:19
sendSyncMessage finalizing SyncMessage.js:256:19
sendSyncMessage resume #1/1 - 245ms SyncMessage.js:240:19
sendSyncMessage resume #0/0 - 245ms SyncMessage.js:240:19
sendSyncMessage finalizing SyncMessage.js:256:19
For comparison, when the issue does *not* occur, I tried a few times and got either this...

Code: Select all

 [NoScript] Policy was undefined, retrying in 300ms... 2 log.js:9:13
... or this -

Code: Select all

sendSyncMessage suspend #0/1 SyncMessage.js:231:19
sendSyncMessage finalizing SyncMessage.js:256:19
sendSyncMessage resume #0/0 - 73ms SyncMessage.js:240:19
sendSyncMessage finalizing SyncMessage.js:256:19
*Always* check the changelogs BEFORE updating that important software!
-
User avatar
Giorgio Maone
Site Admin
Posts: 9454
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: 11.0.41

Post by Giorgio Maone »

skriptimaahinen wrote: Sat Aug 29, 2020 11:11 am Could not get any ouput from the "bodySuspender" unless I used document.addEventListener in SyncMessage.js.
I suspect that's because you forgot to change to document also the removeEventListener() call in finalize(), therefore bodySuspender gets called after finalization.
Pretty pointless, I'd say. I'd rather try a different approach: dispatching a sythethic DOMContentLoaded for the scripts which missed it.
Stay tuned.
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0
skriptimaahinen
Master Bug Buster
Posts: 244
Joined: Wed Jan 10, 2018 7:37 am

Re: 11.0.41

Post by skriptimaahinen »

Giorgio Maone wrote: Sat Aug 29, 2020 4:59 pm I suspect that's because you forgot to change to document also the removeEventListener() call in finalize(), therefore bodySuspender gets called after finalization.
:oops:

Interestingly, if I use the original plain addEventListener() and remove the removeEventListener() from finalize(), I get pretty good results. With quick initial tests the DOMContentLoaded fires reliably. Need to make more tests and figure out explanation.
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0
User avatar
Giorgio Maone
Site Admin
Posts: 9454
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: 11.0.41

Post by Giorgio Maone »

Unfortunately this approach works quite well on Firefox 81, but it's pretty screwed up on Firefox 80.
And anyway page scripts mixing document.write with more complex event handler registration can became really messy when you reorder stuff on the fly.
Therefore I decided to completely change my approach in latest development build:
v 11.0.42rc3
============================================================
x "Soft reload" approach to fix file: and ftp: issues
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:80.0) Gecko/20100101 Firefox/80.0
barbaz
Senior Member
Posts: 10841
Joined: Sat Aug 03, 2013 5:45 pm

Re: 11.0.41

Post by barbaz »

So far looks like 11.0.42rc3 completely fixed the issue I reported. Thanks Giorgio! Image
*Always* check the changelogs BEFORE updating that important software!
-
Post Reply