Timing issue in trusted-types/support/navigation-support.js
Categories
(Core :: DOM: Security, enhancement)
Tracking
()
People
(Reporter: fredw, Unassigned)
References
(Blocks 1 open bug)
Details
The original TrustedTypes "pre-navigation check" test trusted-types/trusted-types-navigation.html has historically been quite fragile. It basically loads a subframe which executes navigateToJavascriptURL which in turns:
- starts observing for security violations
- clicks a navigation element, whose target is of the form "javascript:location.href = ..."
- waits a bit that we try and execute the javascript.
- forces a connect-src violation (see csp-violations.js) to flush the violation events and stop observing security violations when that one is received.
- sends the observed security violations back to the window.opener.
- eventually, navigates the page away (if JS execution is not blocked)
The "wait a bit" at step 3 must be long enough so that we make sure "pre-navigation check" violations have been reported already but not too long so that 6 does not happen before 5. If the page is unloaded before, then the opener won't receive the reported violations and we can again get flaky failures. WebKit and Chromium seems ok with not waiting at all. For Firefox, it seems window.requestIdleCallback was long enough to make sure the violations are reported (simple or double requestAnimationFrame were still not enough) and that works in Chromium/WebKit too. So I'm doing that in https://phabricator.services.mozilla.com/D244355 basically but maybe that's still a bit fragile and we might want to consider some the of events mentioned below instead. In particular, this may not be implemented consistently in all browsers/platforms and maybe makes tests slower (especially split trusted-types-navigation.html runs multiple subtests in one file and is more subject to timeouts).
The connect-src complication makes sure we don't miss any violation. In the past (for Trusted Types tests in general) we were not doing that and we were missing some violations on some browsers, or some violations were reported in later tests which was quite confusing. For trusted-types-navigation.html the important point is to guarantee there is really no violations (otherwise we we will get false positive) or that we don't miss the reported violation (otherwise we we could get flaky test failure). Note that in the past, we were bouncing violations immediately to the opener but that made difficult to handle the case "No securitypolicyviolation reported!" and that was causing a lot of timeout.
We can consider modifying trusted_type_violations_and_exception_for so that it stops observing things and resolves immediately with the observed violations (without waiting connect-src) when we are going to navigate away from the document (see again below for possible events to consider). I still don't know if browsers that require a flush (chromium, webkit) would catch all the violations though.
For the new navigate-to-javascript-url-*.html tests added in https://phabricator.services.mozilla.com/D244732 I took a different approach. First the tests are split into separate files, so for example timeout does not affect the other ones. Next, the JavaScript URL is just setting some global boolean variable to true, so we don't need to worry that the page navigates away from the document. However requestIdleCallback is still used in setLocationToJavaScriptURL to make sure pre-navigation check violations have been reported before checking them. Again, maybe one of the events below would be better to consider to make these tests more robust.
Possible candidates of events to consider: "pagehide", "unload", "pageswap", "beforeunload", "navigate" (https://html.spec.whatwg.org/multipage/indices.html#events-2)
|
Reporter | |
Comment 1•5 months ago
|
||
|
|
Reporter | |
Comment 2•2 months ago
|
||
We have been passing trusted-types navigation tests for a while on all platforms and no flakiness has been reported, so I guess we can close this bug.
Description
•