Conversation
|
Two things for the top comment, I don't think you do fold set and filter html anymore? Also this also doesn't touch execCommand with the insertHTML command. (This is probably the right move because that isnt specced correctly to start with. But worth calling out probably). |
Thanks, OP updated. |
|
Did we decide it was okay to not enforce the sanitizing if the document was XML? |
That's what I understood but @mozfreddyb, @evilpie or @otherdaniel would know more. Sanitization is not specified for the XML parser. |
What I remember is that we specified Sanitizer API only for methods that would inherently only support HTML syntax. This is probably best articulated in the hopelessly outdated explainer Nearly all interesting bits are specified in terms of DOM & DOM operations, so I'd expect this to be easy to adapt to XML. But IMHO, application to XML-parser parse trees requires a second look, since it invalidates one of the assumptions we had when specifying any of this. A silly example, but the only one I can think of: CDataSection in https://wicg.github.io/sanitizer-api/#sanitize-core step 1.1. That shouldn't be difficult to fix; but at least for now Sanitizer would assert-fail on (some) XML parse trees. In our implementation, there's a runtime assert there. |
|
I guess my main concern is people defining a trusted types policy with this new function thinking it protects them and then it doesn't because they're in XHTML or something? Assuming I'm reading this right you'd end up with a default policy explicitly setup to remove unsafe and then it actually no-ops when it's called by a legacy sync in XML. |
You mean sink? Yea it's limited in that way. But |
I think that the specific guidance to developers to be to check the type of document when they create the default policy, use createHTML with the appropriate userland sanitizer if either this is an XML document or TrustedParserOptions is not supported, and createParserOptions otherwise |
|
Non-authoratative LGTM. I'm still slightly unsure about the XML case mentioned above but if the consensus is that it's fine then I buy that. |
|
I asked chatgpt pro to help review the stack of 4 PRs. Sharing the response below; I haven't yet read through the PRs or this response in detail myself. Prompt:
ResponseReview verdictI would request changes on all four PRs in their current state. The stack itself is arranged correctly—#12583 targets The most serious stack-wide interaction is between #12756 and #12758:
Taken together, the safe streaming path can preserve a #12583 — Trusted Types parser options1. Blocking:
|
It seems reasonable to throw in this case. |
…owIfMissing polarity
…x TT throwIfMissing polarity" This reverts commit 7898aca.
Thanks! See below if you want to run it again. One of the issues is false, some are known limitations/existing issues, most are now fixed. I'd be happy to learn if there are pending design issues while we work through the logical/editorial bits.
Fixed
Fixed (together with w3c/trusted-types#606)
Changed to throw when
Fixed
Fixed
Fixed
Fixed
Fixed
Fixed
I believe this is false. It goes through "insert node the adjusted insertion location" which sanitizes.
Fixed
Fixed
Added an XXX note for now
There are tests for this. I will point it out in the OP for streaming.
Fixed
Fixed
Fixed
Fixed
Fixed
Fixed
Exisiting issue
Ack |
In almost* all of the places where where TrustedTypes'
createHTMLis called,we now also check for
createParserOptionsand call that if it's available, by calling "get trusted types compliant input" instead of "get trusted types compliant string".The exceptions to this are
srcdoc,document.write(), and theinsertHTMLexecCommand. We can resolve separately as to whether these should also be sanitized withcreateParserOptions.This now allows us to use the sanitizer in legacy markup insertion methods like the
innerHTMLsetter, and todisallow running scripts in
createContextualFragment.Since all markup insertion methods can now include a sanitizer, most of the steps from "set and filter HTML" are folded into the "fragment parsing algorithm steps", and it's assumed that there is always a sanitizer (though sometimes with an empty configuration).
Together with w3c/trusted-types#606
(See WHATWG Working Mode: Changes for more details.)
/dom.html ( diff )
/dynamic-markup-insertion.html ( diff )
/infrastructure.html ( diff )