Skip to content

Commit eb41b26

Browse files
nolanlawsonwjhsfekashida
authored
fix(engine): disallow innerHTML outside lwc:inner-html (#4578)
Co-authored-by: Will Harney <wharney@salesforce.com> Co-authored-by: Eugene Kashida <ekashida@gmail.com>
1 parent e8098e1 commit eb41b26

File tree

29 files changed

+396
-12
lines changed

29 files changed

+396
-12
lines changed

‎eslint.config.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export default tseslint.config(
5555
{
5656
argsIgnorePattern: '^_',
5757
caughtErrorsIgnorePattern: '^_',
58+
destructuredArrayIgnorePattern: '^_',
5859
},
5960
],
6061

‎packages/@lwc/engine-core/src/framework/api.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import {
5959
VText,
6060
} from './vnodes';
6161
import { getComponentRegisteredName } from './component';
62+
import { createSanitizedHtmlContent, SanitizedHtmlContent } from './sanitized-html-content';
6263

6364
const SymbolIterator: typeof Symbol.iterator = Symbol.iterator;
6465

@@ -727,8 +728,9 @@ export function setSanitizeHtmlContentHook(newHookImpl: SanitizeHtmlContentHook)
727728
}
728729

729730
// [s]anitize [h]tml [c]ontent
730-
function shc(content: unknown): string {
731-
return sanitizeHtmlContentHook(content);
731+
function shc(content: unknown): SanitizedHtmlContent {
732+
const sanitizedString = sanitizeHtmlContentHook(content);
733+
return createSanitizedHtmlContent(sanitizedString);
732734
}
733735

734736
/**

‎packages/@lwc/engine-core/src/framework/hydration.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import { hydrateStaticParts, traverseAndSetElements } from './modules/static-par
6161
import { getScopeTokenClass, getStylesheetTokenHost } from './stylesheet';
6262
import { renderComponent } from './component';
6363
import { applyRefs } from './modules/refs';
64+
import { isSanitizedHtmlContentEqual } from './sanitized-html-content';
6465

6566
// These values are the ones from Node.nodeType (https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType)
6667
const enum EnvNodeTypes {
@@ -240,7 +241,9 @@ function hydrateComment(node: Node, vnode: VComment, renderer: RendererAPI): Nod
240241
}
241242

242243
const { setProperty } = renderer;
243-
setProperty(node, NODE_VALUE_PROP, vnode.text ?? null);
244+
// We only set the `nodeValue` property here (on a comment), so we don't need
245+
// to sanitize the content as HTML using `safelySetProperty`
246+
setProperty(node as Element, NODE_VALUE_PROP, vnode.text ?? null);
244247
vnode.elm = node;
245248

246249
return node;
@@ -310,7 +313,7 @@ function hydrateElement(elm: Node, vnode: VElement, renderer: RendererAPI): Node
310313
} = vnode;
311314
const { getProperty } = renderer;
312315
if (!isUndefined(props) && !isUndefined(props.innerHTML)) {
313-
if (getProperty(elm, 'innerHTML') === props.innerHTML) {
316+
if (isSanitizedHtmlContentEqual(getProperty(elm, 'innerHTML'), props.innerHTML)) {
314317
// Do a shallow clone since VNodeData may be shared across VNodes due to hoist optimization
315318
vnode.data = {
316319
...vnode.data,

‎packages/@lwc/engine-core/src/framework/modules/attrs.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { RendererAPI } from '../renderer';
1616

1717
import { EmptyObject } from '../utils';
1818
import { VBaseElement, VStatic, VStaticPartElement } from '../vnodes';
19+
import { safelySetProperty } from '../sanitized-html-content';
1920

2021
const ColonCharCode = 58;
2122

@@ -51,7 +52,7 @@ export function patchAttributes(
5152
// Use kebabCaseToCamelCase directly because we don't want to set props like `ariaLabel` or `tabIndex`
5253
// on a custom element versus just using the more reliable attribute format.
5354
if (external && (propName = kebabCaseToCamelCase(key)) in elm!) {
54-
setProperty(elm, propName, cur);
55+
safelySetProperty(setProperty, elm!, propName, cur);
5556
} else if (StringCharCodeAt.call(key, 3) === ColonCharCode) {
5657
// Assume xml namespace
5758
setAttribute(elm, key, cur as string, XML_NAMESPACE);

‎packages/@lwc/engine-core/src/framework/modules/props.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { logWarn } from '../../shared/logger';
99
import { RendererAPI } from '../renderer';
1010
import { EmptyObject } from '../utils';
1111
import { VBaseElement } from '../vnodes';
12+
import { safelySetProperty } from '../sanitized-html-content';
1213

1314
function isLiveBindingProp(sel: string, key: string): boolean {
1415
// For properties with live bindings, we read values from the DOM element
@@ -66,7 +67,7 @@ export function patchProps(
6667
);
6768
}
6869
}
69-
setProperty(elm!, key, cur);
70+
safelySetProperty(setProperty, elm!, key, cur);
7071
}
7172
}
7273
}

‎packages/@lwc/engine-core/src/framework/sanitized-html-content.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { create as ObjectCreate, isNull, isObject, isUndefined } from '@lwc/shared';
2+
import { logWarn } from '../shared/logger';
3+
import type { RendererAPI } from './renderer';
4+
5+
const sanitizedHtmlContentSymbol = Symbol('lwc-get-sanitized-html-content');
6+
7+
export type SanitizedHtmlContent = {
8+
[sanitizedHtmlContentSymbol]: unknown;
9+
};
10+
11+
function isSanitizedHtmlContent(object: any): object is SanitizedHtmlContent {
12+
return isObject(object) && !isNull(object) && sanitizedHtmlContentSymbol in object;
13+
}
14+
15+
function unwrapIfNecessary(object: any) {
16+
return isSanitizedHtmlContent(object) ? object[sanitizedHtmlContentSymbol] : object;
17+
}
18+
19+
/**
20+
* Wrap a pre-sanitized string designated for `.innerHTML` via `lwc:inner-html`
21+
* as an object with a Symbol that only we have access to.
22+
* @param sanitizedString
23+
* @returns SanitizedHtmlContent
24+
*/
25+
export function createSanitizedHtmlContent(sanitizedString: unknown): SanitizedHtmlContent {
26+
return ObjectCreate(null, {
27+
[sanitizedHtmlContentSymbol]: {
28+
value: sanitizedString,
29+
configurable: false,
30+
writable: false,
31+
},
32+
});
33+
}
34+
35+
/**
36+
* Safely call setProperty on an Element while handling any SanitizedHtmlContent objects correctly
37+
*
38+
* @param setProperty - renderer.setProperty
39+
* @param elm - Element
40+
* @param key - key to set
41+
* @param value - value to set
42+
*/
43+
export function safelySetProperty(
44+
setProperty: RendererAPI['setProperty'],
45+
elm: Element,
46+
key: string,
47+
value: any
48+
) {
49+
// See W-16614337
50+
// we support setting innerHTML to `undefined` because it's inherently safe
51+
if ((key === 'innerHTML' || key === 'outerHTML') && !isUndefined(value)) {
52+
if (isSanitizedHtmlContent(value)) {
53+
// it's a SanitizedHtmlContent object
54+
setProperty(elm, key, value[sanitizedHtmlContentSymbol]);
55+
} else {
56+
// not a SanitizedHtmlContent object
57+
if (process.env.NODE_ENV !== 'production') {
58+
logWarn(
59+
`Cannot set property "${key}". Instead, use lwc:inner-html or lwc:dom-manual.`
60+
);
61+
}
62+
}
63+
} else {
64+
setProperty(elm, key, value);
65+
}
66+
}
67+
68+
/**
69+
* Given two objects (likely either a string or a SanitizedHtmlContent object), return true if their
70+
* string values are equivalent.
71+
* @param first
72+
* @param second
73+
*/
74+
export function isSanitizedHtmlContentEqual(first: any, second: any) {
75+
return unwrapIfNecessary(first) === unwrapIfNecessary(second);
76+
}

‎packages/@lwc/engine-dom/src/renderer/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ function getProperty(node: Node, key: string): any {
7171
return (node as any)[key];
7272
}
7373

74-
function setProperty(node: Node, key: string, value: any): void {
74+
function setProperty<K extends string>(
75+
node: Node & Record<K, unknown>,
76+
key: K,
77+
value: unknown
78+
): void {
7579
(node as any)[key] = value;
7680
}
7781

‎packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ async function compileFixture({ input, dirname }: { input: string; dirname: stri
5252
],
5353
onwarn({ message, code }) {
5454
// TODO [#3331]: The existing lwc:dynamic fixture test will generate warnings that can be safely suppressed.
55-
if (!message.includes('LWC1187') && code !== 'CIRCULAR_DEPENDENCY') {
55+
const shouldIgnoreWarning =
56+
message.includes('LWC1187') ||
57+
message.includes('-h-t-m-l') ||
58+
code === 'CIRCULAR_DEPENDENCY';
59+
if (!shouldIgnoreWarning) {
5660
throw new Error(message);
5761
}
5862
},

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/error.txt

Whitespace-only changes.

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/expected.html

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
<x-inner>
2+
<template shadowrootmode="open">
3+
<div data-expect-no-warning data-id="div-inner-static" inner-h-t-m-l="replaced">
4+
original
5+
</div>
6+
<div data-expect-no-warning data-id="div-inner-computed" inner-h-t-m-l="injected">
7+
original
8+
</div>
9+
<div data-id="div-inner-spread">
10+
original
11+
</div>
12+
<x-component data-id="cmp-inner-static">
13+
<template shadowrootmode="open">
14+
<slot>
15+
</slot>
16+
</template>
17+
original
18+
</x-component>
19+
<x-component data-id="cmp-inner-computed">
20+
<template shadowrootmode="open">
21+
<slot>
22+
</slot>
23+
</template>
24+
original
25+
</x-component>
26+
<x-component data-id="cmp-inner-spread">
27+
<template shadowrootmode="open">
28+
<slot>
29+
</slot>
30+
</template>
31+
original
32+
</x-component>
33+
<omg-whatever data-id="external-inner-static" inner-h-t-m-l="replaced">
34+
original
35+
</omg-whatever>
36+
<omg-whatever data-id="external-inner-computed" inner-h-t-m-l="injected">
37+
original
38+
</omg-whatever>
39+
<omg-whatever data-id="external-inner-spread">
40+
original
41+
</omg-whatever>
42+
<div data-expect-no-warning data-id="div-outer-static" outer-h-t-m-l="replaced">
43+
original
44+
</div>
45+
<div data-expect-no-warning data-id="div-outer-computed" outer-h-t-m-l="injected">
46+
original
47+
</div>
48+
<div data-id="div-outer-spread">
49+
original
50+
</div>
51+
<x-component data-id="cmp-outer-static">
52+
<template shadowrootmode="open">
53+
<slot>
54+
</slot>
55+
</template>
56+
original
57+
</x-component>
58+
<x-component data-id="cmp-outer-computed">
59+
<template shadowrootmode="open">
60+
<slot>
61+
</slot>
62+
</template>
63+
original
64+
</x-component>
65+
<x-component data-id="cmp-outer-spread">
66+
<template shadowrootmode="open">
67+
<slot>
68+
</slot>
69+
</template>
70+
original
71+
</x-component>
72+
<omg-whatever data-id="external-outer-static">
73+
original
74+
</omg-whatever>
75+
<omg-whatever data-id="external-outer-computed">
76+
original
77+
</omg-whatever>
78+
<omg-whatever data-id="external-outer-spread">
79+
original
80+
</omg-whatever>
81+
<span>
82+
lwc:inner-html
83+
</span>
84+
<p>
85+
injected
86+
</p>
87+
</template>
88+
</x-inner>

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export const tagName = 'x-inner';
2+
export { default } from 'x/inner';
3+
export * from 'x/inner';

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.html

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<template>
2+
<slot></slot>
3+
</template>

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { LightningElement } from 'lwc';
2+
3+
export default class extends LightningElement {}

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.html

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<template>
2+
<div data-id="div-inner-static" data-expect-no-warning inner-h-t-m-l="replaced">original</div>
3+
<div data-id="div-inner-computed" data-expect-no-warning inner-h-t-m-l={computed}>original</div>
4+
<div data-id="div-inner-spread" lwc:spread={spread}>original</div>
5+
6+
<x-component data-id="cmp-inner-static" inner-h-t-m-l="replaced">original</x-component>
7+
<x-component data-id="cmp-inner-computed" inner-h-t-m-l={computed}>original</x-component>
8+
<x-component data-id="cmp-inner-spread" lwc:spread={spread}>original</x-component>
9+
10+
<omg-whatever lwc:external data-id="external-inner-static" inner-h-t-m-l="replaced">original</omg-whatever>
11+
<omg-whatever lwc:external data-id="external-inner-computed" inner-h-t-m-l={computed}>original</omg-whatever>
12+
<omg-whatever lwc:external data-id="external-inner-spread" lwc:spread={spread}>original</omg-whatever>
13+
14+
<div data-id="div-outer-static" data-expect-no-warning outer-h-t-m-l="replaced">original</div>
15+
<div data-id="div-outer-computed" data-expect-no-warning outer-h-t-m-l={computed}>original</div>
16+
<div data-id="div-outer-spread" lwc:spread={spread}>original</div>
17+
18+
<x-component data-id="cmp-outer-static" outer-h-t-m-l="replaced">original</x-component>
19+
<x-component data-id="cmp-outer-computed" outer-h-t-m-l={computed}>original</x-component>
20+
<x-component data-id="cmp-outer-spread" lwc:spread={spread}>original</x-component>
21+
22+
<omg-whatever lwc:external data-id="external-outer-static" outer-h-t-m-l="replaced">original</omg-whatever>
23+
<omg-whatever lwc:external data-id="external-outer-computed" outer-h-t-m-l={computed}>original</omg-whatever>
24+
<omg-whatever lwc:external data-id="external-outer-spread" lwc:spread={spread}>original</omg-whatever>
25+
26+
<span lwc:inner-html="lwc:inner-html"></span>
27+
<p lwc:inner-html={computed}></p>
28+
</template>

‎packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { LightningElement } from 'lwc';
2+
3+
export default class extends LightningElement {
4+
computed = 'injected';
5+
spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' };
6+
}

‎packages/@lwc/integration-karma/test-hydration/inner-outer-html/index.spec.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
export default {
2+
props: {},
3+
advancedTest(target, { consoleSpy }) {
4+
const ids = Object.entries(TestUtils.extractDataIds(target)).filter(
5+
([id]) => !id.endsWith('.shadowRoot')
6+
);
7+
for (const [id, node] of ids) {
8+
expect(node.childNodes.length).toBe(1);
9+
expect(node.firstChild.nodeType).toBe(Node.TEXT_NODE);
10+
const expected = id.startsWith('lwc-inner-html-') ? 'injected' : 'original';
11+
expect(node.firstChild.nodeValue).toBe(expected);
12+
}
13+
expect(consoleSpy.calls.warn).toHaveSize(0);
14+
expect(consoleSpy.calls.error).toHaveSize(0);
15+
},
16+
};

‎packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.html

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<template>
2+
<slot></slot>
3+
</template>

‎packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { LightningElement } from 'lwc';
2+
3+
export default class extends LightningElement {}

‎packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.html

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<template>
2+
<div data-id="div-inner-static" data-expect-no-warning inner-h-t-m-l="replaced">original</div>
3+
<div data-id="div-inner-computed" data-expect-no-warning inner-h-t-m-l={computed}>original</div>
4+
<div data-id="div-inner-spread" lwc:spread={spread}>original</div>
5+
6+
<x-component data-id="cmp-inner-static" inner-h-t-m-l="replaced">original</x-component>
7+
<x-component data-id="cmp-inner-computed" inner-h-t-m-l={computed}>original</x-component>
8+
<x-component data-id="cmp-inner-spread" lwc:spread={spread}>original</x-component>
9+
10+
<omg-whatever lwc:external data-id="external-inner-static" inner-h-t-m-l="replaced">original</omg-whatever>
11+
<omg-whatever lwc:external data-id="external-inner-computed" inner-h-t-m-l={computed}>original</omg-whatever>
12+
<omg-whatever lwc:external data-id="external-inner-spread" lwc:spread={spread}>original</omg-whatever>
13+
14+
<div data-id="div-outer-static" data-expect-no-warning outer-h-t-m-l="replaced">original</div>
15+
<div data-id="div-outer-computed" data-expect-no-warning outer-h-t-m-l={computed}>original</div>
16+
<div data-id="div-outer-spread" lwc:spread={spread}>original</div>
17+
18+
<x-component data-id="cmp-outer-static" outer-h-t-m-l="replaced">original</x-component>
19+
<x-component data-id="cmp-outer-computed" outer-h-t-m-l={computed}>original</x-component>
20+
<x-component data-id="cmp-outer-spread" lwc:spread={spread}>original</x-component>
21+
22+
<omg-whatever lwc:external data-id="external-outer-static" outer-h-t-m-l="replaced">original</omg-whatever>
23+
<omg-whatever lwc:external data-id="external-outer-computed" outer-h-t-m-l={computed}>original</omg-whatever>
24+
<omg-whatever lwc:external data-id="external-outer-spread" lwc:spread={spread}>original</omg-whatever>
25+
26+
<span data-id="lwc-inner-html-static" lwc:inner-html="injected"></span>
27+
<p data-id="lwc-inner-html-computed" lwc:inner-html={computed}></p>
28+
</template>

‎packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { LightningElement } from 'lwc';
2+
3+
export default class extends LightningElement {
4+
computed = 'injected';
5+
spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' };
6+
}

0 commit comments

Comments
 (0)