Skip to content

Commit

Permalink
Remove setInnerHTML completely
Browse files Browse the repository at this point in the history
The conversation [1] about the recent changes to setInnerHTML have led
to the conclusion [2] that perhaps we shouldn't add a new XSS sink
method at all. That would "fix" the declarative Shadow DOM problem, but
would create a new sink that all security libraries would need to
know about and handle. Seems like not a good trade.

In the meantime, a polyfill can stand in for setInnerHTML:

  Element.prototype.setInnerHTML = function(content) {
    const fragment = (new DOMParser()).parseFromString(`<pre>${content}</pre>`,
       'text/html', {includeShadowRoots: true});
    this.replaceChildren(...fragment.body.firstChild.childNodes);
  };

[1] whatwg/dom#912
[2] whatwg/dom#912 (comment)

Bug: 1042130
Change-Id: Ibaf15a3edf86be9a720225dea2ba2741f2882b8c
  • Loading branch information
mfreed7 authored and chromium-wpt-export-bot committed Nov 24, 2020
1 parent 06e5209 commit 4ba46e1
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<script src='/resources/testharness.js'></script>
<script src='/resources/testharnessreport.js'></script>
<script src='../resources/shadow-dom-utils.js'></script>
<script src="support/helpers.js"></script>

<script>
const shadowContent = '<span>Shadow tree</span><slot></slot>';
Expand All @@ -18,7 +19,7 @@
const declarativeString = `<${elementType} id=theelement>${getDeclarativeContent(mode, delegatesFocus)}
<span class='lightdom'>${lightDomTextContent}</span></${elementType}>`;
const wrapper = document.createElement('div');
wrapper.setInnerHTML(declarativeString, {includeShadowRoots: true});
setInnerHTML(wrapper, declarativeString);
const element = wrapper.querySelector('#theelement');
return {wrapper: wrapper, element: element};
}
Expand Down
35 changes: 13 additions & 22 deletions shadow-dom/declarative/declarative-shadow-dom-basic.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<link rel="help" href="https://github.com/whatwg/dom/issues/831">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="support/helpers.js"></script>

<div id="host" style="display:none">
<template shadowroot="open">
Expand Down Expand Up @@ -32,14 +33,14 @@

test(() => {
const div = document.createElement('div');
div.setInnerHTML(`
setInnerHTML(div,`
<div id="host">
<template shadowroot="open">
<slot id="s1" name="slot1"></slot>
</template>
<div id="c1" slot="slot1"></div>
</div>
`, {includeShadowRoots: true});
`);
const host = div.querySelector('#host');
const c1 = host.querySelector('#c1');
assert_true(!!c1);
Expand All @@ -53,12 +54,12 @@

test(() => {
const div = document.createElement('div');
div.setInnerHTML(`
setInnerHTML(div,`
<div id="host">
<template shadowroot="invalid">
</template>
</div>
`, {includeShadowRoots: true});
`);
const host = div.querySelector('#host');
assert_equals(host.shadowRoot, null, "Shadow root was found");
const tmpl = host.querySelector('template');
Expand All @@ -69,25 +70,25 @@

test(() => {
const div = document.createElement('div');
div.setInnerHTML(`
setInnerHTML(div,`
<div id="host">
<template shadowroot="closed">
</template>
</div>
`, {includeShadowRoots: true});
`);
const host = div.querySelector('#host');
assert_equals(host.shadowRoot, null, "Closed shadow root");
assert_equals(host.querySelector('template'), null, "No template - converted to shadow root");
}, 'Declarative Shadow DOM: Closed shadow root attribute');

test(() => {
const div = document.createElement('div');
div.setInnerHTML(`
setInnerHTML(div,`
<div id="host">
<template shadowroot="open">
<slot id="s1" name="slot1"></slot>
</div>
`, {includeShadowRoots: true});
`);
const host = div.querySelector('#host');
assert_equals(host.querySelector('#s1'), null, "Should be inside shadow root");
assert_equals(host.querySelector('template'), null, "No leftover template node");
Expand All @@ -98,35 +99,25 @@

test(() => {
const div = document.createElement('div');
div.setInnerHTML(`
setInnerHTML(div,`
<div id="host">
<template shadowroot="open" shadowrootdelegatesfocus>
</template>
</div>
`, {includeShadowRoots: true});
`);
var host = div.querySelector('#host');
assert_true(!!host.shadowRoot,"No shadow root found");
assert_true(host.shadowRoot.delegatesFocus,"delegatesFocus should be true");
div.setInnerHTML(`
setInnerHTML(div,`
<div id="host">
<template shadowroot="open">
</template>
</div>
`, {includeShadowRoots: true});
`);
host = div.querySelector('#host');
assert_true(!!host.shadowRoot,"No shadow root found");
assert_false(host.shadowRoot.delegatesFocus,"delegatesFocus should be false without the shadowrootdelegatesfocus attribute");
}, 'Declarative Shadow DOM: delegates focus attribute');

test(() => {
const host = document.createElement('div');
// Root element of setInnerHTML is a <template shadowroot>:
host.setInnerHTML('<template shadowroot=open></template>', {allowShadowRoot: true});
assert_equals(host.shadowRoot, null, "Shadow root should not be present");
const tmpl = host.querySelector('template');
assert_true(!!tmpl,"Template should still be present");
assert_equals(tmpl.getAttribute('shadowroot'),"open","'shadowroot' attribute should still be present");
}, 'Declarative Shadow DOM: setInnerHTML root element');
</script>

<div id="multi-host" style="display:none">
Expand Down
57 changes: 20 additions & 37 deletions shadow-dom/declarative/declarative-shadow-dom-opt-in.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<link rel="help" href="https://github.com/whatwg/dom/issues/831">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src='../resources/shadow-dom-utils.js'></script>

<body>
<style>
Expand Down Expand Up @@ -58,56 +59,40 @@
assert_dsd(div,true);
}, 'Non-fragment parsing needs no opt-in');

test(() => {
const div = document.createElement('div');
div.innerHTML = content;
assert_dsd(div,false);
}, 'innerHTML on element - disallowed');

test(() => {
const div = document.createElement('div');
div.setInnerHTML(content);
assert_dsd(div,false);
div.setInnerHTML(content, {includeShadowRoots: false});
assert_dsd(div,false);
div.setInnerHTML(content, {includeShadowRoots: true});
assert_dsd(div,true);
}, 'setInnerHTML on element');

test(() => {
const templateContent = `<template id=tmpl>${content}</template>`;
const div = document.createElement('div');
div.innerHTML = templateContent;
assert_dsd(div.querySelector('#tmpl').content,false);
div.setInnerHTML(templateContent, {includeShadowRoots: true});
assert_dsd(div.querySelector('#tmpl').content,true);
}, 'setInnerHTML on element, with nested template content');
const noChildElements = ['iframe','noscript','script','select','style','textarea','title','colgroup'];
const elements = HTML5_ELEMENT_NAMES.filter(el => !noChildElements.includes(el));
for (let elementName of elements) {
var t = test(function() {
const el1 = document.createElement(elementName);
el1.innerHTML = content;
assert_dsd(el1,false);

const templateContent = `<template id=tmpl>${content}</template>`;
const el2 = document.createElement('div');
el2.innerHTML = templateContent;
assert_dsd(el2.querySelector('#tmpl').content,false);
}, `innerHTML on a <${elementName}>`);
}

test(() => {
const temp = document.createElement('template');
temp.innerHTML = content;
assert_dsd(temp.content,false, 'innerHTML should not allow declarative shadow content');
temp.setInnerHTML(content, {includeShadowRoots: true});
assert_dsd(temp.content,true, 'setInnerHTML should allow declarative shadow content if enabled');
}, 'setInnerHTML on template');
}, 'innerHTML on template');

test(() => {
const templateContent = `<template id=tmpl>${content}</template>`;
const temp = document.createElement('template');
temp.innerHTML = templateContent;
assert_dsd(temp.content.querySelector('#tmpl').content,false);
temp.setInnerHTML(templateContent, {includeShadowRoots: true});
assert_dsd(temp.content.querySelector('#tmpl').content,true);
}, 'setInnerHTML on template, with nested template content');
}, 'innerHTML on template, with nested template content');

test(() => {
const div = document.createElement('div');
const shadow = div.attachShadow({mode: 'open'});
shadow.innerHTML = content;
assert_dsd(shadow,false);
shadow.setInnerHTML(content, {includeShadowRoots: true});
assert_dsd(shadow,true);
}, 'setInnerHTML on shadowRoot');
}, 'innerHTML on shadowRoot');

test(() => {
const parser = new DOMParser();
Expand All @@ -123,9 +108,7 @@
const doc = document.implementation.createHTMLDocument('');
doc.body.innerHTML = content;
assert_dsd(doc.body,false);
doc.body.setInnerHTML(content, {includeShadowRoots: true});
assert_dsd(doc.body,true);
}, 'createHTMLDocument with setInnerHTML');
}, 'createHTMLDocument with innerHTML - not supported');

test(() => {
const doc = document.implementation.createHTMLDocument('');
Expand All @@ -145,7 +128,7 @@
client.open("GET", `data:text/html,${content}`);
client.responseType = 'document';
client.send();
}, 'XMLHttpRequest, disabled');
}, 'XMLHttpRequest - not supported');

test(() => {
const div = document.createElement('div');
Expand Down
48 changes: 0 additions & 48 deletions shadow-dom/declarative/setinnerhtml.tentative.html

This file was deleted.

4 changes: 4 additions & 0 deletions shadow-dom/declarative/support/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
function setInnerHTML(el,content) {
const fragment = (new DOMParser()).parseFromString(`<pre>${content}</pre>`, 'text/html', {includeShadowRoots: true});
el.replaceChildren(...fragment.body.firstChild.childNodes);
}

0 comments on commit 4ba46e1

Please sign in to comment.