- From: Mason Freed <notifications@github.com>
- Date: Fri, 20 Nov 2020 09:41:46 -0800
- To: whatwg/dom <dom@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/dom/issues/912/731307336@github.com>
> @mfreed7 how crucial is it to have the `setInnerHTML` case in v1? Do you expect the declarative shadow DOM to be commonly using this function? Can the other, already existing and unsafe sinks be used by the authors instead of `setInnerHTML` or does `setInnerHTML` have a unique behavior that you want to preserve? Hmm, I feel like we're going a bit in circles here. Recall that the original proposal was this: ```javascript document.allowDeclarativeShadowDOM = true; element.innerHTML = contentIncludingDSD; ``` That had these pros: - Avoids the sanitizer bypass, which again, is the entire goal of the "opt-in". - No additional XSS sinks added. - Easy to use. And this con: - It introduces state on the `Document`, which generally isn't great. I don't particularly think this is ergonomic or readily understandable by developers: ```javascript element.setInnerHTML(content, { includeShadowRoots: true, unsafe: true }); ``` and as @koto pointed out, it makes the XSS sanitization job more difficult. I do think we need **some** innerHTML-like functionality, as this is a common pattern. Forcing all DSD fragment parsing through `DOMParser` feels quite painful. My own WPT tests, for example, make use of `setInnerHTML()` 25 times, not including the test itself for `setInnerHTML()`. Given that this is a new API, **purely** to avoid the risk of a sanitizer bypass for sanitizers that are **unaware** of declarative Shadow DOM, **how about just this**? ```javascript element.setInnerHTML(content, {unsafe: false}); // Always throws, regardless of content element.setInnerHTML(content, {unsafe: true}); // Parses DSD, never throws ``` This clearly indicates that something about it is "unsafe". And the ability to **not** parse DSD isn't actually of value even to sanitizers, as closed shadow roots in the parsed output will be invisible to the sanitizer. @annevk @koto what do you think? @annevk as for the Permissions Policy, I'm beginning to think we should just skip adding permissions for iframes here at all. Iframes will continue to parse DSD, just as the main document does. Thoughts? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/whatwg/dom/issues/912#issuecomment-731307336
Received on Friday, 20 November 2020 17:41:59 UTC