Skip to content

Commit 5355a9c

Browse files
authored
fix(engine): avoid non-string scope tokens (#4519)
1 parent 777e84c commit 5355a9c

File tree

6 files changed

+143
-1
lines changed

6 files changed

+143
-1
lines changed

packages/@lwc/engine-core/src/framework/freeze-template.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,13 @@ import { Stylesheet, Stylesheets } from './stylesheet';
2626
import { onReportingEnabled, report, ReportingEventId } from './reporting';
2727

2828
// See @lwc/engine-core/src/framework/template.ts
29-
const TEMPLATE_PROPS = ['slots', 'stylesheetToken', 'stylesheets', 'renderMode'] as const;
29+
const TEMPLATE_PROPS = [
30+
'slots',
31+
'stylesheetToken',
32+
'stylesheets',
33+
'renderMode',
34+
'legacyStylesheetToken',
35+
] as const;
3036

3137
// Expandos that may be placed on a stylesheet factory function, and which are meaningful to LWC at runtime
3238
const STYLESHEET_PROPS = [

packages/@lwc/engine-core/src/framework/template.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,14 @@ function buildParseFragmentFn(
270270
}
271271
}
272272

273+
// See W-16614556
274+
if (
275+
(hasStyleToken && !isString(stylesheetToken)) ||
276+
(hasLegacyToken && !isString(legacyStylesheetToken))
277+
) {
278+
throw new Error('stylesheet token must be a string');
279+
}
280+
273281
// If legacy stylesheet tokens are required, then add them to the rendered string
274282
const stylesheetTokenToRender =
275283
stylesheetToken + (hasLegacyToken ? ` ${legacyStylesheetToken}` : '');
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { createElement, setFeatureFlagForTest } from 'lwc';
2+
import { catchUnhandledRejectionsAndErrors } from 'test-utils';
3+
import Component from 'x/component';
4+
5+
let caughtError;
6+
let logger;
7+
8+
catchUnhandledRejectionsAndErrors((error) => {
9+
caughtError = error;
10+
});
11+
12+
beforeEach(() => {
13+
logger = spyOn(console, 'warn');
14+
});
15+
16+
afterEach(() => {
17+
caughtError = undefined;
18+
});
19+
20+
const props = ['stylesheetToken', 'stylesheetTokens', 'legacyStylesheetToken'];
21+
22+
props.forEach((prop) => {
23+
describe(prop, () => {
24+
beforeEach(() => {
25+
setFeatureFlagForTest('ENABLE_LEGACY_SCOPE_TOKENS', prop === 'legacyStylesheetToken');
26+
});
27+
28+
afterEach(() => {
29+
setFeatureFlagForTest('ENABLE_LEGACY_SCOPE_TOKENS', false);
30+
// We keep a cache of parsed static fragments; these need to be reset
31+
// since they can vary based on whether we use the legacy scope token or not.
32+
window.__lwcResetFragmentCache();
33+
// Reset template object for clean state between tests
34+
Component.resetTemplate();
35+
});
36+
37+
it('W-16614556 should not render arbitrary content via stylesheet token', async () => {
38+
const elm = createElement('x-component', { is: Component });
39+
elm.propToUse = prop;
40+
try {
41+
document.body.appendChild(elm);
42+
} catch (err) {
43+
// In synthetic custom element lifecycle, the error is thrown synchronously on `appendChild`
44+
caughtError = err;
45+
}
46+
47+
await Promise.resolve();
48+
49+
if (process.env.NATIVE_SHADOW && process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION) {
50+
// If we're rendering in native shadow and the static content optimization is disabled,
51+
// then there's no problem with non-string stylesheet tokens because they are only rendered
52+
// as class attribute values using either `classList` or `setAttribute` (and this only applies
53+
// when `*.scoped.css` is being used).
54+
expect(elm.shadowRoot.children.length).toBe(1);
55+
} else {
56+
expect(elm.shadowRoot.children.length).toBe(0); // does not render
57+
58+
expect(caughtError).not.toBeUndefined();
59+
expect(caughtError.message).toMatch(
60+
/stylesheet token must be a string|Failed to execute 'setAttribute'|Invalid qualified name|String contains an invalid character|The string contains invalid characters/
61+
);
62+
63+
if (process.env.NODE_ENV === 'production') {
64+
// no warnings in prod mode
65+
expect(logger).not.toHaveBeenCalled();
66+
} else {
67+
// dev mode
68+
expect(logger).toHaveBeenCalledTimes(1);
69+
expect(logger.calls.allArgs()[0]).toMatch(
70+
new RegExp(`Mutating the "${prop}" property on a template is deprecated`)
71+
);
72+
}
73+
}
74+
});
75+
});
76+
});
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
div {
2+
color: red;
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<template>
2+
<div></div>
3+
</template>
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { LightningElement, api } from 'lwc';
2+
import template from './component.html';
3+
4+
export default class Component extends LightningElement {
5+
@api propToUse;
6+
7+
count = 0;
8+
9+
render() {
10+
const token = {
11+
[Symbol.toPrimitive]: () => {
12+
if (this.count === 0) {
13+
return `yolo-${++this.count}`;
14+
} else {
15+
return `yolo=yolo-${++this.count}`;
16+
}
17+
},
18+
};
19+
20+
const { propToUse } = this;
21+
if (propToUse === 'stylesheetTokens') {
22+
// this legacy format uses an object
23+
template[propToUse] = {
24+
hostAttribute: token,
25+
shadowAttribute: token,
26+
};
27+
} else {
28+
// stylesheetToken or legacyStylesheetToken
29+
// this format uses a string
30+
template[propToUse] = token;
31+
}
32+
33+
return template;
34+
}
35+
}
36+
37+
// Reset template object for clean state between tests
38+
const { stylesheetToken, stylesheetTokens, legacyStylesheetToken } = template;
39+
40+
Component.resetTemplate = () => {
41+
Object.assign(template, {
42+
stylesheetToken,
43+
stylesheetTokens,
44+
legacyStylesheetToken,
45+
});
46+
};

0 commit comments

Comments
 (0)