diff --git a/eslint.config.mjs b/eslint.config.mjs index e5a3f1aa96..0d58818edf 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -55,6 +55,7 @@ export default tseslint.config( { argsIgnorePattern: '^_', caughtErrorsIgnorePattern: '^_', + destructuredArrayIgnorePattern: '^_', }, ], diff --git a/packages/@lwc/engine-core/src/framework/api.ts b/packages/@lwc/engine-core/src/framework/api.ts index 1124c54a76..56a5dee3e4 100644 --- a/packages/@lwc/engine-core/src/framework/api.ts +++ b/packages/@lwc/engine-core/src/framework/api.ts @@ -59,6 +59,7 @@ import { VText, } from './vnodes'; import { getComponentRegisteredName } from './component'; +import { createSanitizedHtmlContent, SanitizedHtmlContent } from './sanitized-html-content'; const SymbolIterator: typeof Symbol.iterator = Symbol.iterator; @@ -727,8 +728,9 @@ export function setSanitizeHtmlContentHook(newHookImpl: SanitizeHtmlContentHook) } // [s]anitize [h]tml [c]ontent -function shc(content: unknown): string { - return sanitizeHtmlContentHook(content); +function shc(content: unknown): SanitizedHtmlContent { + const sanitizedString = sanitizeHtmlContentHook(content); + return createSanitizedHtmlContent(sanitizedString); } /** diff --git a/packages/@lwc/engine-core/src/framework/hydration.ts b/packages/@lwc/engine-core/src/framework/hydration.ts index 736b1ec488..a9cb5a1f54 100644 --- a/packages/@lwc/engine-core/src/framework/hydration.ts +++ b/packages/@lwc/engine-core/src/framework/hydration.ts @@ -61,6 +61,7 @@ import { hydrateStaticParts, traverseAndSetElements } from './modules/static-par import { getScopeTokenClass, getStylesheetTokenHost } from './stylesheet'; import { renderComponent } from './component'; import { applyRefs } from './modules/refs'; +import { isSanitizedHtmlContentEqual } from './sanitized-html-content'; // These values are the ones from Node.nodeType (https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType) const enum EnvNodeTypes { @@ -240,7 +241,9 @@ function hydrateComment(node: Node, vnode: VComment, renderer: RendererAPI): Nod } const { setProperty } = renderer; - setProperty(node, NODE_VALUE_PROP, vnode.text ?? null); + // We only set the `nodeValue` property here (on a comment), so we don't need + // to sanitize the content as HTML using `safelySetProperty` + setProperty(node as Element, NODE_VALUE_PROP, vnode.text ?? null); vnode.elm = node; return node; @@ -310,7 +313,7 @@ function hydrateElement(elm: Node, vnode: VElement, renderer: RendererAPI): Node } = vnode; const { getProperty } = renderer; if (!isUndefined(props) && !isUndefined(props.innerHTML)) { - if (getProperty(elm, 'innerHTML') === props.innerHTML) { + if (isSanitizedHtmlContentEqual(getProperty(elm, 'innerHTML'), props.innerHTML)) { // Do a shallow clone since VNodeData may be shared across VNodes due to hoist optimization vnode.data = { ...vnode.data, diff --git a/packages/@lwc/engine-core/src/framework/modules/attrs.ts b/packages/@lwc/engine-core/src/framework/modules/attrs.ts index 7020e06165..1c3b666443 100644 --- a/packages/@lwc/engine-core/src/framework/modules/attrs.ts +++ b/packages/@lwc/engine-core/src/framework/modules/attrs.ts @@ -16,6 +16,7 @@ import { RendererAPI } from '../renderer'; import { EmptyObject } from '../utils'; import { VBaseElement, VStatic, VStaticPartElement } from '../vnodes'; +import { safelySetProperty } from '../sanitized-html-content'; const ColonCharCode = 58; @@ -51,7 +52,7 @@ export function patchAttributes( // Use kebabCaseToCamelCase directly because we don't want to set props like `ariaLabel` or `tabIndex` // on a custom element versus just using the more reliable attribute format. if (external && (propName = kebabCaseToCamelCase(key)) in elm!) { - setProperty(elm, propName, cur); + safelySetProperty(setProperty, elm!, propName, cur); } else if (StringCharCodeAt.call(key, 3) === ColonCharCode) { // Assume xml namespace setAttribute(elm, key, cur as string, XML_NAMESPACE); diff --git a/packages/@lwc/engine-core/src/framework/modules/props.ts b/packages/@lwc/engine-core/src/framework/modules/props.ts index ca51eb687c..ab769d3935 100644 --- a/packages/@lwc/engine-core/src/framework/modules/props.ts +++ b/packages/@lwc/engine-core/src/framework/modules/props.ts @@ -9,6 +9,7 @@ import { logWarn } from '../../shared/logger'; import { RendererAPI } from '../renderer'; import { EmptyObject } from '../utils'; import { VBaseElement } from '../vnodes'; +import { safelySetProperty } from '../sanitized-html-content'; function isLiveBindingProp(sel: string, key: string): boolean { // For properties with live bindings, we read values from the DOM element @@ -66,7 +67,7 @@ export function patchProps( ); } } - setProperty(elm!, key, cur); + safelySetProperty(setProperty, elm!, key, cur); } } } diff --git a/packages/@lwc/engine-core/src/framework/sanitized-html-content.ts b/packages/@lwc/engine-core/src/framework/sanitized-html-content.ts new file mode 100644 index 0000000000..5752c7fa1a --- /dev/null +++ b/packages/@lwc/engine-core/src/framework/sanitized-html-content.ts @@ -0,0 +1,76 @@ +import { create as ObjectCreate, isNull, isObject, isUndefined } from '@lwc/shared'; +import { logWarn } from '../shared/logger'; +import type { RendererAPI } from './renderer'; + +const sanitizedHtmlContentSymbol = Symbol('lwc-get-sanitized-html-content'); + +export type SanitizedHtmlContent = { + [sanitizedHtmlContentSymbol]: unknown; +}; + +function isSanitizedHtmlContent(object: any): object is SanitizedHtmlContent { + return isObject(object) && !isNull(object) && sanitizedHtmlContentSymbol in object; +} + +function unwrapIfNecessary(object: any) { + return isSanitizedHtmlContent(object) ? object[sanitizedHtmlContentSymbol] : object; +} + +/** + * Wrap a pre-sanitized string designated for `.innerHTML` via `lwc:inner-html` + * as an object with a Symbol that only we have access to. + * @param sanitizedString + * @returns SanitizedHtmlContent + */ +export function createSanitizedHtmlContent(sanitizedString: unknown): SanitizedHtmlContent { + return ObjectCreate(null, { + [sanitizedHtmlContentSymbol]: { + value: sanitizedString, + configurable: false, + writable: false, + }, + }); +} + +/** + * Safely call setProperty on an Element while handling any SanitizedHtmlContent objects correctly + * + * @param setProperty - renderer.setProperty + * @param elm - Element + * @param key - key to set + * @param value - value to set + */ +export function safelySetProperty( + setProperty: RendererAPI['setProperty'], + elm: Element, + key: string, + value: any +) { + // See W-16614337 + // we support setting innerHTML to `undefined` because it's inherently safe + if ((key === 'innerHTML' || key === 'outerHTML') && !isUndefined(value)) { + if (isSanitizedHtmlContent(value)) { + // it's a SanitizedHtmlContent object + setProperty(elm, key, value[sanitizedHtmlContentSymbol]); + } else { + // not a SanitizedHtmlContent object + if (process.env.NODE_ENV !== 'production') { + logWarn( + `Cannot set property "${key}". Instead, use lwc:inner-html or lwc:dom-manual.` + ); + } + } + } else { + setProperty(elm, key, value); + } +} + +/** + * Given two objects (likely either a string or a SanitizedHtmlContent object), return true if their + * string values are equivalent. + * @param first + * @param second + */ +export function isSanitizedHtmlContentEqual(first: any, second: any) { + return unwrapIfNecessary(first) === unwrapIfNecessary(second); +} diff --git a/packages/@lwc/engine-dom/src/renderer/index.ts b/packages/@lwc/engine-dom/src/renderer/index.ts index 0ff6fa63e9..64d5e66953 100644 --- a/packages/@lwc/engine-dom/src/renderer/index.ts +++ b/packages/@lwc/engine-dom/src/renderer/index.ts @@ -71,7 +71,11 @@ function getProperty(node: Node, key: string): any { return (node as any)[key]; } -function setProperty(node: Node, key: string, value: any): void { +function setProperty( + node: Node & Record, + key: K, + value: unknown +): void { (node as any)[key] = value; } diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts b/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts index 69083dde6c..9f9e6956c2 100755 --- a/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts +++ b/packages/@lwc/engine-server/src/__tests__/fixtures.spec.ts @@ -52,7 +52,11 @@ async function compileFixture({ input, dirname }: { input: string; dirname: stri ], onwarn({ message, code }) { // TODO [#3331]: The existing lwc:dynamic fixture test will generate warnings that can be safely suppressed. - if (!message.includes('LWC1187') && code !== 'CIRCULAR_DEPENDENCY') { + const shouldIgnoreWarning = + message.includes('LWC1187') || + message.includes('-h-t-m-l') || + code === 'CIRCULAR_DEPENDENCY'; + if (!shouldIgnoreWarning) { throw new Error(message); } }, diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/error.txt b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/error.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/expected.html new file mode 100644 index 0000000000..552399789e --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/expected.html @@ -0,0 +1,88 @@ + + + \ No newline at end of file diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/index.js b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/index.js new file mode 100644 index 0000000000..278d09e0d3 --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/index.js @@ -0,0 +1,3 @@ +export const tagName = 'x-inner'; +export { default } from 'x/inner'; +export * from 'x/inner'; diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.html b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.html new file mode 100644 index 0000000000..6806310398 --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.js b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.js new file mode 100644 index 0000000000..ca8dce94e0 --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/component/component.js @@ -0,0 +1,3 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement {} diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.html b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.html new file mode 100644 index 0000000000..a68a7c2d28 --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.html @@ -0,0 +1,28 @@ + diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.js b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.js new file mode 100644 index 0000000000..dc4b54ded2 --- /dev/null +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/inner-outer-html/modules/x/inner/inner.js @@ -0,0 +1,6 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement { + computed = 'injected'; + spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' }; +} diff --git a/packages/@lwc/integration-karma/test-hydration/inner-outer-html/index.spec.js b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/index.spec.js new file mode 100644 index 0000000000..4d584c357a --- /dev/null +++ b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/index.spec.js @@ -0,0 +1,16 @@ +export default { + props: {}, + advancedTest(target, { consoleSpy }) { + const ids = Object.entries(TestUtils.extractDataIds(target)).filter( + ([id]) => !id.endsWith('.shadowRoot') + ); + for (const [id, node] of ids) { + expect(node.childNodes.length).toBe(1); + expect(node.firstChild.nodeType).toBe(Node.TEXT_NODE); + const expected = id.startsWith('lwc-inner-html-') ? 'injected' : 'original'; + expect(node.firstChild.nodeValue).toBe(expected); + } + expect(consoleSpy.calls.warn).toHaveSize(0); + expect(consoleSpy.calls.error).toHaveSize(0); + }, +}; diff --git a/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.html b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.html new file mode 100644 index 0000000000..6806310398 --- /dev/null +++ b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.js b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.js new file mode 100644 index 0000000000..ca8dce94e0 --- /dev/null +++ b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/component/component.js @@ -0,0 +1,3 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement {} diff --git a/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.html b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.html new file mode 100644 index 0000000000..b0c85440a4 --- /dev/null +++ b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.html @@ -0,0 +1,28 @@ + diff --git a/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.js b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.js new file mode 100644 index 0000000000..dc4b54ded2 --- /dev/null +++ b/packages/@lwc/integration-karma/test-hydration/inner-outer-html/x/main/main.js @@ -0,0 +1,6 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement { + computed = 'injected'; + spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' }; +} diff --git a/packages/@lwc/integration-karma/test-hydration/light-dom/directives/lwc-inner-html/index.spec.js b/packages/@lwc/integration-karma/test-hydration/light-dom/directives/lwc-inner-html/index.spec.js index 366ae6d62e..8b29eddea5 100644 --- a/packages/@lwc/integration-karma/test-hydration/light-dom/directives/lwc-inner-html/index.spec.js +++ b/packages/@lwc/integration-karma/test-hydration/light-dom/directives/lwc-inner-html/index.spec.js @@ -11,12 +11,15 @@ export default { text: p.textContent, }; }, - test(target, snapshot) { + test(target, snapshot, consoleCalls) { const div = target.querySelector('div'); const p = div.querySelector('p'); expect(div).toBe(snapshot.div); expect(p).toBe(snapshot.p); expect(p.textContent).toBe(snapshot.text); + + expect(consoleCalls.warn).toHaveSize(0); + expect(consoleCalls.error).toHaveSize(0); }, }; diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/index.spec.js b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/index.spec.js new file mode 100644 index 0000000000..f23e35c9ef --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/index.spec.js @@ -0,0 +1,48 @@ +import { createElement } from 'lwc'; +import { extractDataIds } from 'test-utils'; +import Inner from 'x/inner'; +import Outer from 'x/outer'; + +beforeAll(() => { + customElements.define('omg-whatever', class extends HTMLElement {}); +}); + +let consoleSpy; +beforeEach(() => { + consoleSpy = spyOn(console, 'warn'); +}); + +for (const whatter of ['inner', 'outer']) { + // See W-16614337 + it(`does not render ${whatter} HTML from attributes`, async () => { + const Whatter = whatter === 'inner' ? Inner : Outer; + const elm = createElement(`x-${whatter}`, { is: Whatter }); + document.body.appendChild(elm); + await Promise.resolve(); + + const ids = Object.entries(extractDataIds(elm)).filter( + ([id]) => !id.endsWith('.shadowRoot') + ); + for (const [_id, node] of ids) { + expect(node.childNodes.length).toBe(1); + expect(node.firstChild.nodeType).toBe(Node.TEXT_NODE); + expect(node.firstChild.nodeValue).toBe('original'); + } + + if (process.env.NODE_ENV === 'production') { + expect(consoleSpy).not.toHaveBeenCalled(); + } else { + const len = ids.filter( + ([_id, elm]) => !elm.hasAttribute('data-expect-no-warning') + ).length; + expect(consoleSpy).toHaveBeenCalledTimes(len); + + const calls = consoleSpy.calls; + for (let i = 0; i < len; i += 1) { + expect(calls.argsFor(i)[0].message).toContain( + `Cannot set property "${whatter}HTML". Instead, use lwc:inner-html or lwc:dom-manual.` + ); + } + } + }); +} diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/component/component.html b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/component/component.html new file mode 100644 index 0000000000..6806310398 --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/component/component.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/component/component.js b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/component/component.js new file mode 100644 index 0000000000..ca8dce94e0 --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/component/component.js @@ -0,0 +1,3 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement {} diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/inner/inner.html b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/inner/inner.html new file mode 100644 index 0000000000..cfd067f042 --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/inner/inner.html @@ -0,0 +1,13 @@ + diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/inner/inner.js b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/inner/inner.js new file mode 100644 index 0000000000..ce76aee280 --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/inner/inner.js @@ -0,0 +1,6 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement { + computed = '
injected
'; + spread = { innerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' }; +} diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/outer/outer.html b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/outer/outer.html new file mode 100644 index 0000000000..9574b542d1 --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/outer/outer.html @@ -0,0 +1,13 @@ + diff --git a/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/outer/outer.js b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/outer/outer.js new file mode 100644 index 0000000000..d7de6a015b --- /dev/null +++ b/packages/@lwc/integration-karma/test/rendering/inner-outer-html/x/outer/outer.js @@ -0,0 +1,6 @@ +import { LightningElement } from 'lwc'; + +export default class extends LightningElement { + computed = '
injected
'; + spread = { outerHTML: 'wheeeeeeeeeeeeeeeeeeeeeeeeeee' }; +} diff --git a/packages/@lwc/integration-karma/test/spread/index.spec.js b/packages/@lwc/integration-karma/test/spread/index.spec.js index f7f05ab7d0..68f69035d1 100644 --- a/packages/@lwc/integration-karma/test/spread/index.spec.js +++ b/packages/@lwc/integration-karma/test/spread/index.spec.js @@ -12,8 +12,9 @@ function setSanitizeHtmlContentHookForTest(impl) { return sanitizeHtmlContent; } describe('lwc:spread', () => { - let elm, simpleChild, overriddenChild, trackedChild, innerHTMLChild, originalHook; + let elm, simpleChild, overriddenChild, trackedChild, innerHTMLChild, originalHook, consoleSpy; beforeEach(() => { + consoleSpy = spyOn(console, 'warn'); originalHook = setSanitizeHtmlContentHookForTest((x) => x); elm = createElement('x-test', { is: Test }); document.body.appendChild(elm); @@ -29,8 +30,17 @@ describe('lwc:spread', () => { it('should render basic test', () => { expect(simpleChild.shadowRoot.querySelector('span').textContent).toEqual('Name: LWC'); }); - it('should override innerHTML from inner-html directive', () => { - expect(innerHTMLChild.innerHTML).toEqual('innerHTML from spread'); + it('should not override innerHTML from inner-html directive', () => { + expect(innerHTMLChild.innerHTML).toEqual(''); + + if (process.env.NODE_ENV === 'production') { + expect(consoleSpy).not.toHaveBeenCalled(); + } else { + expect(consoleSpy).toHaveBeenCalledTimes(1); + expect(consoleSpy.calls.argsFor(0)[0].message).toContain( + `Cannot set property "innerHTML". Instead, use lwc:inner-html or lwc:dom-manual.` + ); + } }); it('should assign onclick', () => { simpleChild.click();