wpt
wpt copied to clipboard
web-platform-tests
Simple test with `require-trusted-types-for 'script';` CSP throws
STR:
Create a file <x.html> containing:
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta http-equiv="Content-Security-Policy" content="require-trusted-types-for 'script';">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<body>
<script>
test(t => { assert_equals(1, 1);}, "t");
</script>
</body>
</html>
run it with ./wpt run chrome path/to/x.html (in my case <trusted-types/block-string-assignment-to-Element-setAttributeNode.html>).
It throws a TypeError because setAttribute is called with "onclick" at https://searchfox.org/mozilla-central/rev/76cb3efe3b19e649bf675bb6ec5d4af8109b9771/testing/web-platform/tests/resources/testharness.js#4507:
Uncaught TypeError: Failed to execute 'setAttribute' on 'Element': This document requires 'TrustedScript' assignment.
at make_dom_single (testharness.js:4509:29)
at make_dom (testharness.js:4529:20)
at make_dom_single (testharness.js:4514:39)
at make_dom (testharness.js:4529:20)
at make_dom_single (testharness.js:4514:39)
at make_dom (testharness.js:4529:20)
at render (testharness.js:4539:16)
at Output.show_results (testharness.js:4231:25)
at testharness.js:189:37
at testharness.js:3816:22
CC @lukewarlow
I wonder how intrusive it would be to make the WPT helpers trusted type compliant? The alternative is a custom harness for trusted type tests that does what it needs to in a trusted types compliant way?
I wonder how intrusive it would be to make the WPT helpers trusted type compliant?
Don't know. Presumably the only way to find out is to do it. @otherdaniel has this been attempted before?
The alternative is a custom harness for trusted type tests that does what it needs to in a trusted types compliant way?
It seems that would need more effort than making the WPT helpers TT compliant.
Yeah that's the trade off, benefit it's less intrusive to the rest of the suite, it wouldn't need to be the whole harness like just the helpers we use but just cloning rather than changing the ones used everywhere else.
Yeah that's the trade off, benefit it's less intrusive to the rest of the suite, it wouldn't need to be the whole harness like just the helpers we use but just cloning rather than changing the ones used everywhere else.
@smaug---- suggested potentially running the tests in a new window, reporting their result via postMessage. Don't know if that would allow calling assert_*s (e.g. assert_equals, https://searchfox.org/mozilla-central/source/testing/web-platform/tests/resources/testharness.js#1531) though.
You should be able to use fetch_tests_from_window, compare for example https://github.com/web-platform-tests/wpt/tree/master/FileAPI/url/resources/fetch-tests.js
I wonder how intrusive it would be to make the WPT helpers trusted type compliant?
Don't know. Presumably the only way to find out is to do it. @otherdaniel has this been attempted before?
I don't recall having done anything in particular to make Trusted Types work with WPT. wpt.fyi seems to run the tests just fine.
Yeah, as explained in https://github.com/web-platform-tests/wpt/issues/44348, the issue comes up when running a test manually, because it goes wrong when rendering the results table and "rerun" button
You should be able to use
fetch_tests_from_window, compare for example https://github.com/web-platform-tests/wpt/tree/master/FileAPI/url/resources/fetch-tests.js
Okay, that's https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#3886-3897 and should work.
You should be able to use
fetch_tests_from_window, compare for example https://github.com/web-platform-tests/wpt/tree/master/FileAPI/url/resources/fetch-tests.jsOkay, that's https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#3886-3897 and should work.
Since existing tests suffer from the same problem (e.g. https://github.com/web-platform-tests/wpt/issues/44348), the proper fix would be to either rewrite all of those tests or fix the testharness. The existing tests are quite numerous (https://searchfox.org/mozilla-central/source/testing/web-platform/tests/trusted-types).
When commenting out https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4367,4507 the harness throws no exceptions.
For https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4367 there's a performance-relevant comment at https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4257-4261.
CC @smaug----
For https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4367 there's a performance-relevant comment at https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4257-4261.
CC @smaug----
According to @smaug---- the performance-comment is presumably still relevant, because using innerHTML saves JS wrappers for the <td> elements.
I wonder how intrusive it would be to make the WPT helpers trusted type compliant? The alternative is a custom harness for trusted type tests that does what it needs to in a trusted types compliant way?
Another alternative is to set a default policy which propagates scripts and HTML at the end of the last test. I.e.:
t.add_cleanup(() => {
// The testharness passes strings to injection sinks, see <TODO-file-bug>.
// The following default policy allows that.
const p = trustedTypes.createPolicy("default", { createHTML: s => s, createScript: s => s});
})
Works for synchronous tests which aren't already using a default policy.
I wonder how intrusive it would be to make the WPT helpers trusted type compliant? The alternative is a custom harness for trusted type tests that does what it needs to in a trusted types compliant way?
Another alternative is to set a default policy which propagates scripts and HTML at the end of the last test. I.e.:
t.add_cleanup(() => { // The testharness passes strings to injection sinks, see <TODO-file-bug>. // The following default policy allows that. const p = trustedTypes.createPolicy("default", { createHTML: s => s, createScript: s => s}); })Works for synchronous tests which aren't already using a default policy.
This might be less intrusive than the suggested alternatives.
Once trusted types are supported by all browser engines, the testharness could be refactored to use non-default policies at the relevant sinks, allowing to remove the default policies in the tests' cleanup functions.
Since existing tests suffer from the same problem (e.g. #44348), the proper fix would be to either rewrite all of those tests or fix the testharness. The existing tests are quite numerous (https://searchfox.org/mozilla-central/source/testing/web-platform/tests/trusted-types).
Maybe adapting the testharness wouldn't be that much work. I tried doing this in a draft-PR, here: https://github.com/web-platform-tests/wpt/pull/44561 This basically de-string-ifies building of the result table.
For https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4367 there's a performance-relevant comment at https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4257-4261. CC @smaug----
According to @smaug---- the performance-comment is presumably still relevant, because using
innerHTMLsaves JS wrappers for the<td>elements.
Ah... that's a very good point I hadn't thought of. Provided the PR above looks generally good to people, what exactly should I measure to figure out whether the performance cost is still here?
Since existing tests suffer from the same problem (e.g. #44348), the proper fix would be to either rewrite all of those tests or fix the testharness. The existing tests are quite numerous (https://searchfox.org/mozilla-central/source/testing/web-platform/tests/trusted-types).
Maybe adapting the testharness wouldn't be that much work. I tried doing this in a draft-PR, here: #44561 This basically de-string-ifies building of the result table.
For https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4367 there's a performance-relevant comment at https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4257-4261. CC @smaug----
According to @smaug---- the performance-comment is presumably still relevant, because using
innerHTMLsaves JS wrappers for the<td>elements.Ah... that's a very good point I hadn't thought of. Provided the PR above looks generally good to people, what exactly should I measure to figure out whether the performance cost is still here?
Trying to figure that out. It might be, that such measurements nowadays are unnecessary. The performance comment at https://searchfox.org/mozilla-central/rev/9bb5d5f55da6cc7163dd93825c25609b769822f9/testing/web-platform/tests/resources/testharness.js#4257-4261 was added 2014, so things might've changed. One argument for that is that the runs at <wpt.fyi> potentially don't need that information. See e.g. https://wpt.fyi/results/trusted-types/block-string-assignment-to-attribute-via-attribute-node.html?label=experimental&label=master&aligned.
Moreover, when running a folder of tests locally, e.g.
./wpt run --no-headless chrome trusted-types/
the output table is never shown.
It's only shown when executing a single test file, e.g.:
./wpt run --no-headless chrome trusted-types/
Single tests have a timeout of at most 60s, see https://web-platform-tests.org/writing-tests/testharness-api.html#harness-timeout.
The variant feature (https://web-platform-tests.org/writing-tests/testharness.html#variants) allows exceeding that timeout, but no output table is shown in that case.
Is anyone aware of present-day scenarios when the output table is generated for multiple, e.g. hundreds or thousands, of tests?
http://wpt.live/html/dom/reflection-text.html has over 10000
It executes in a few seconds though.
The time to build the results table (with this patch) for html/dom/reflection-text.text (with indeed >10k entries) took ~800ms (my machine; current version of Chrome). That was the total build time; not the cost difference between parsing and Unless anyone has additional concerns, I'll then clean up the patch; handle Mirko's feedback; and submit..textContent. I'd say the warning of "Using textContent on each individual adds tens of seconds of execution time for large test suites (tens of thousands of tests)." is no longer accurate.
The time to build the results table (with this patch) for html/dom/reflection-text.text (with indeed >10k entries) took ~800ms (my machine; current version of Chrome). That was the total build time; not the cost difference between parsing and
.textContent. I'd say the warning of "Using textContent on each individual adds tens of seconds of execution time for large test suites (tens of thousands of tests)." is no longer accurate.Unless anyone has additional concerns, I'll then clean up the patch; handle Mirko's feedback; and submit.
Agree. Moreover the 60s limit mentioned at https://github.com/web-platform-tests/wpt/issues/44352#issuecomment-1944048762 applies to all tests.
@jgraham PTAL at the last question mentioned in https://github.com/web-platform-tests/wpt/issues/44352#issuecomment-1944048762.
@jgraham could you also PTAL at the corresponding testharness patch. Especially at https://chromium-review.googlesource.com/c/chromium/src/+/5290877/comment/b3ec4e27_c7bdddba/.
Could we move this patch into github, please? Although we generally support syncing with vendor repos, for infra changes it's quite hard to be sure that they work correctly if they are only run through vendor CI. In this specific case I'd like to fetch the branch and test the perf difference locally.
Could we move this patch into github, please? Although we generally support syncing with vendor repos, for infra changes it's quite hard to be sure that they work correctly if they are only run through vendor CI. In this specific case I'd like to fetch the branch and test the perf difference locally.
CC @otherdaniel
Could we move this patch into github, please? Although we generally support syncing with vendor repos, for infra changes it's quite hard to be sure that they work correctly if they are only run through vendor CI. In this specific case I'd like to fetch the branch and test the perf difference locally.
https://github.com/web-platform-tests/wpt/pull/44561
I meant a real PR on wpt, not one that's owned by the Chromium CI bot and where the status is controlled by what happens in Chromium's code review.
I meant a real PR on wpt, not one that's owned by the Chromium CI bot and where the status is controlled by what happens in Chromium's code review.
Ah, I didn't realize this would block perf testing. Please look at it here: https://github.com/web-platform-tests/wpt/pull/45002
Please also look at https://github.com/web-platform-tests/wpt/pull/45003 That seems to do extra work after each test run, which might be relevant if we're performance limited. It's code that was originally added (see reference in PR description) to support an experiment. A cursory search doesn't turn up any usage of it. Even if it's used, there might be better ways to do this, e.g. adding the string to something that isn't a <script>.
@jgraham could you PTAL at above post. Getting this fixed would help to implement trusted-types.
