Core:Manipulation: Add basic TrustedHTML support #4927
Conversation
|
Travis fails due to #4926 which is unrelated. |
| } | ||
| } ), | ||
| inputs = [ | ||
| [ "<div></div>", "<div class='test'></div>", [ "div" ] ], |
There was a problem hiding this comment.
Does it make sense to also add a test for TrustedHTMLs that wrap over a non-obvious HTML, e.g. just some text? From what I can tell, different branches are used then.
There was a problem hiding this comment.
Yes, good point. That will only apply to the second array items as the first ones are passed directly to jQuery which also supports selectors - and since for TrustedHTML we cannot carve out a part of an input string to later parse, I opted to restrict support for TrustedHTML-wrapped strings to ones that are start with < and end with > - i.e. what was already going through the fast path skipping the regex match.
There was a problem hiding this comment.
So, I assume that TrustedHTML wrapping something will just get stringified somewhere and be treated as a selector? That's OK, I don't think there would be expectations for jQuery to do anything else. I think it just makes sense to have an explicit test for that behavior.
There was a problem hiding this comment.
@koto It would actually just create a jQuery wrapper with a single element being this TrustedHTML instance, i.e. it would treat it like any other non-DOM object. I just didn't handle it in any special way and let it go where it goes naturally in the current flow.
There was a problem hiding this comment.
To be clear, this limitation only applies to (pseudo-code) jQuery(TrustedHTML('foo')); jQuery(document.body).append(TrustedHTML('foo')) should append the text foo just as without wrapping in TrustedHTML; I'll add a test for that.
There was a problem hiding this comment.
Oh, I forgot about https://api.jquery.com/jQuery/#jQuery-object behavior :) Yeah, that's fine, thanks for clarifying.
There was a problem hiding this comment.
I added one test case for text content. I haven't added one for the object wrapper as I'm not sure if we want this to be a part of the contract.
|
Looks great! |
|
PR updated, we're down from +38 to +30 bytes. |
|
|
This ensures HTML wrapped in TrustedHTML can be used as an input to jQuery manipulation methods in a way that doesn't violate the `require-trusted-types-for` Content Security Policy directive. This commit builds on previous work needed for trusted types support, including jquerygh-4642 and jquerygh-4724. One restriction is that while any TrustedHTML wrapper should work as input for jQuery methods like `.html()` or `.append()`, for passing directly to the `jQuery` factory the string must start with `<` and end with `>`; no trailing or leading whitespaces are allowed. This is necessary as we cannot parse out a part of the input for further construction; that would violate the CSP rule - and that's what's done to HTML input not matching these constraints. No trusted types API is used explicitly in source; the majority of the work is ensuring we don't pass the input converted to string to APIs that would eventually assign it to `innerHTML`. This extra cautiousness is caused by the API being Blink-only, at least for now. The ban on passing strings to `innerHTML` means support tests relying on such assignments are impossible. We don't currently have such tests on the `main` branch but we used to have many of them in the 3.x & older lines. If there's a need to re-add such a test, we'll need an escape hatch to skip them for apps needing CSP-enforced TrustedHTML. See https://web.dev/trusted-types/ for more information about TrustedHTML. Fixes jquerygh-4409 Ref jquerygh-4642 Ref jquerygh-4724
|
@timmywil @koto I also added one small change in the tests (in the fixup commit) that makes the tests run even in browsers with no TrustedHTML support - there, it uses a simple object wrapper with a |
Summary
This ensures HTML wrapped in TrustedHTML can be used as an input to jQuery
manipulation methods in a way that doesn't violate the
require-trusted-types-forContent Security Policy directive.This commit builds on previous work needed for trusted types support, including
gh-4642 and gh-4724.
One restriction is that while any TrustedHTML wrapper should work as input
for jQuery methods like
.html()or.append(), for passing directly to thejQueryfactory the string must start with<and end with>; no trailingor leading whitespaces are allowed. This is necessary as we cannot parse out
a part of the input for further construction; that would violate the CSP rule -
and that's what's done to HTML input not matching these constraints.
No trusted types API is used explicitly in source; the majority of the work is
ensuring we don't pass the input converted to string to APIs that would
eventually assign it to
innerHTML. This extra cautiousness is caused by theAPI being Blink-only, at least for now.
The ban on passing strings to
innerHTMLmeans support tests relying on suchassignments are impossible. We don't currently have such tests on the
mainbranch but we used to have many of them in the 3.x & older lines. If there's
a need to re-add such a test, we'll need an escape hatch to skip them for apps
needing CSP-enforced TrustedHTML.
See https://web.dev/trusted-types/ for more information about TrustedHTML.
Fixes gh-4409
Ref gh-4642
Ref gh-4724
+30 bytes
Checklist