Skip to content

Commit

Permalink
fix(engine): avoid non-string scope tokens (#4519)
Browse files Browse the repository at this point in the history
  • Loading branch information
nolanlawson authored Sep 3, 2024
1 parent 777e84c commit 5355a9c
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 1 deletion.
8 changes: 7 additions & 1 deletion packages/@lwc/engine-core/src/framework/freeze-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ import { Stylesheet, Stylesheets } from './stylesheet';
import { onReportingEnabled, report, ReportingEventId } from './reporting';

// See @lwc/engine-core/src/framework/template.ts
const TEMPLATE_PROPS = ['slots', 'stylesheetToken', 'stylesheets', 'renderMode'] as const;
const TEMPLATE_PROPS = [
'slots',
'stylesheetToken',
'stylesheets',
'renderMode',
'legacyStylesheetToken',
] as const;

// Expandos that may be placed on a stylesheet factory function, and which are meaningful to LWC at runtime
const STYLESHEET_PROPS = [
Expand Down
8 changes: 8 additions & 0 deletions packages/@lwc/engine-core/src/framework/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ function buildParseFragmentFn(
}
}

// See W-16614556
if (
(hasStyleToken && !isString(stylesheetToken)) ||
(hasLegacyToken && !isString(legacyStylesheetToken))
) {
throw new Error('stylesheet token must be a string');
}

// If legacy stylesheet tokens are required, then add them to the rendered string
const stylesheetTokenToRender =
stylesheetToken + (hasLegacyToken ? ` ${legacyStylesheetToken}` : '');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { createElement, setFeatureFlagForTest } from 'lwc';
import { catchUnhandledRejectionsAndErrors } from 'test-utils';
import Component from 'x/component';

let caughtError;
let logger;

catchUnhandledRejectionsAndErrors((error) => {
caughtError = error;
});

beforeEach(() => {
logger = spyOn(console, 'warn');
});

afterEach(() => {
caughtError = undefined;
});

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

props.forEach((prop) => {
describe(prop, () => {
beforeEach(() => {
setFeatureFlagForTest('ENABLE_LEGACY_SCOPE_TOKENS', prop === 'legacyStylesheetToken');
});

afterEach(() => {
setFeatureFlagForTest('ENABLE_LEGACY_SCOPE_TOKENS', false);
// We keep a cache of parsed static fragments; these need to be reset
// since they can vary based on whether we use the legacy scope token or not.
window.__lwcResetFragmentCache();
// Reset template object for clean state between tests
Component.resetTemplate();
});

it('W-16614556 should not render arbitrary content via stylesheet token', async () => {
const elm = createElement('x-component', { is: Component });
elm.propToUse = prop;
try {
document.body.appendChild(elm);
} catch (err) {
// In synthetic custom element lifecycle, the error is thrown synchronously on `appendChild`
caughtError = err;
}

await Promise.resolve();

if (process.env.NATIVE_SHADOW && process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION) {
// If we're rendering in native shadow and the static content optimization is disabled,
// then there's no problem with non-string stylesheet tokens because they are only rendered
// as class attribute values using either `classList` or `setAttribute` (and this only applies
// when `*.scoped.css` is being used).
expect(elm.shadowRoot.children.length).toBe(1);
} else {
expect(elm.shadowRoot.children.length).toBe(0); // does not render

expect(caughtError).not.toBeUndefined();
expect(caughtError.message).toMatch(
/stylesheet token must be a string|Failed to execute 'setAttribute'|Invalid qualified name|String contains an invalid character|The string contains invalid characters/
);

if (process.env.NODE_ENV === 'production') {
// no warnings in prod mode
expect(logger).not.toHaveBeenCalled();
} else {
// dev mode
expect(logger).toHaveBeenCalledTimes(1);
expect(logger.calls.allArgs()[0]).toMatch(
new RegExp(`Mutating the "${prop}" property on a template is deprecated`)
);
}
}
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
div {
color: red;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
<div></div>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { LightningElement, api } from 'lwc';
import template from './component.html';

export default class Component extends LightningElement {
@api propToUse;

count = 0;

render() {
const token = {
[Symbol.toPrimitive]: () => {
if (this.count === 0) {
return `yolo-${++this.count}`;
} else {
return `yolo=yolo-${++this.count}`;
}
},
};

const { propToUse } = this;
if (propToUse === 'stylesheetTokens') {
// this legacy format uses an object
template[propToUse] = {
hostAttribute: token,
shadowAttribute: token,
};
} else {
// stylesheetToken or legacyStylesheetToken
// this format uses a string
template[propToUse] = token;
}

return template;
}
}

// Reset template object for clean state between tests
const { stylesheetToken, stylesheetTokens, legacyStylesheetToken } = template;

Component.resetTemplate = () => {
Object.assign(template, {
stylesheetToken,
stylesheetTokens,
legacyStylesheetToken,
});
};

0 comments on commit 5355a9c

Please sign in to comment.