Skip to content

Commit 038075f

Browse files
authored
fix(engine): avoid invalid scope tokens entirely (#4524)
1 parent 75097cb commit 038075f

File tree

5 files changed

+125
-47
lines changed

5 files changed

+125
-47
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,14 @@ export function setVMBeingRendered(vm: VM | null) {
7676
vmBeingRendered = vm;
7777
}
7878

79+
const VALID_SCOPE_TOKEN_REGEX = /^[a-zA-Z0-9\-_.]+$/;
80+
81+
// See W-16614556
82+
// TODO [#2826]: freeze the template object
83+
function isValidScopeToken(token: any) {
84+
return isString(token) && VALID_SCOPE_TOKEN_REGEX.test(token);
85+
}
86+
7987
function validateSlots(vm: VM) {
8088
assertNotProd(); // this method should never leak to prod
8189

@@ -272,10 +280,10 @@ function buildParseFragmentFn(
272280

273281
// See W-16614556
274282
if (
275-
(hasStyleToken && !isString(stylesheetToken)) ||
276-
(hasLegacyToken && !isString(legacyStylesheetToken))
283+
(hasStyleToken && !isValidScopeToken(stylesheetToken)) ||
284+
(hasLegacyToken && !isValidScopeToken(legacyStylesheetToken))
277285
) {
278-
throw new Error('stylesheet token must be a string');
286+
throw new Error('stylesheet token must be a valid string');
279287
}
280288

281289
// If legacy stylesheet tokens are required, then add them to the rendered string
Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createElement, setFeatureFlagForTest } from 'lwc';
22
import { catchUnhandledRejectionsAndErrors } from 'test-utils';
33
import Component from 'x/component';
4+
import Scoping from 'x/scoping';
45

56
let caughtError;
67
let logger;
@@ -19,58 +20,83 @@ afterEach(() => {
1920

2021
const props = ['stylesheetToken', 'stylesheetTokens', 'legacyStylesheetToken'];
2122

23+
const components = [
24+
{
25+
tagName: 'x-component',
26+
Ctor: Component,
27+
name: 'unscoped styles',
28+
},
29+
{
30+
tagName: 'x-scoping',
31+
Ctor: Scoping,
32+
name: 'scoped styles',
33+
},
34+
];
35+
2236
props.forEach((prop) => {
2337
describe(prop, () => {
24-
beforeEach(() => {
25-
setFeatureFlagForTest('ENABLE_LEGACY_SCOPE_TOKENS', prop === 'legacyStylesheetToken');
26-
});
38+
components.forEach(({ tagName, Ctor, name }) => {
39+
describe(name, () => {
40+
beforeEach(() => {
41+
setFeatureFlagForTest(
42+
'ENABLE_LEGACY_SCOPE_TOKENS',
43+
prop === 'legacyStylesheetToken'
44+
);
45+
});
2746

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-
});
47+
afterEach(() => {
48+
setFeatureFlagForTest('ENABLE_LEGACY_SCOPE_TOKENS', false);
49+
// We keep a cache of parsed static fragments; these need to be reset
50+
// since they can vary based on whether we use the legacy scope token or not.
51+
window.__lwcResetFragmentCache();
52+
// Reset template object for clean state between tests
53+
Ctor.resetTemplate();
54+
});
3655

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-
}
56+
it('W-16614556 should not render arbitrary content via stylesheet token', async () => {
57+
const elm = createElement(tagName, { is: Ctor });
58+
elm.propToUse = prop;
59+
try {
60+
document.body.appendChild(elm);
61+
} catch (err) {
62+
// In synthetic custom element lifecycle, the error is thrown synchronously on `appendChild`
63+
caughtError = err;
64+
}
4665

47-
await Promise.resolve();
66+
await Promise.resolve();
4867

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
68+
if (
69+
process.env.NATIVE_SHADOW &&
70+
process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION
71+
) {
72+
// If we're rendering in native shadow and the static content optimization is disabled,
73+
// then there's no problem with non-string stylesheet tokens because they are only rendered
74+
// as class attribute values using either `classList` or `setAttribute` (and this only applies
75+
// when `*.scoped.css` is being used).
76+
expect(elm.shadowRoot.children.length).toBe(1);
77+
} else {
78+
expect(elm.shadowRoot.children.length).toBe(0); // does not render
5779

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-
);
80+
expect(caughtError).not.toBeUndefined();
81+
expect(caughtError.message).toMatch(
82+
/stylesheet token must be a valid string|Failed to execute 'setAttribute'|Invalid qualified name|String contains an invalid character|The string contains invalid characters/
83+
);
6284

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-
}
85+
if (process.env.NODE_ENV === 'production') {
86+
// no warnings in prod mode
87+
expect(logger).not.toHaveBeenCalled();
88+
} else {
89+
// dev mode
90+
expect(logger).toHaveBeenCalledTimes(1);
91+
expect(logger.calls.allArgs()[0]).toMatch(
92+
new RegExp(
93+
`Mutating the "${prop}" property on a template is deprecated`
94+
)
95+
);
96+
}
97+
}
98+
});
99+
});
74100
});
75101
});
76102
});
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: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { LightningElement, api } from 'lwc';
2+
import template from './scoping.html';
3+
4+
export default class Component extends LightningElement {
5+
@api propToUse;
6+
7+
count = 0;
8+
9+
render() {
10+
const token = `foo"yolo="haha`;
11+
12+
const { propToUse } = this;
13+
if (propToUse === 'stylesheetTokens') {
14+
// this legacy format uses an object
15+
template[propToUse] = {
16+
hostAttribute: token,
17+
shadowAttribute: token,
18+
};
19+
} else {
20+
// stylesheetToken or legacyStylesheetToken
21+
// this format uses a string
22+
template[propToUse] = token;
23+
}
24+
25+
return template;
26+
}
27+
}
28+
29+
// Reset template object for clean state between tests
30+
const { stylesheetToken, stylesheetTokens, legacyStylesheetToken } = template;
31+
32+
Component.resetTemplate = () => {
33+
Object.assign(template, {
34+
stylesheetToken,
35+
stylesheetTokens,
36+
legacyStylesheetToken,
37+
});
38+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
div {
2+
color: red;
3+
}

0 commit comments

Comments
 (0)