Skip to content

Commit 134c800

Browse files
authored
umb-localize encode HTML arguments (#18960)
* Moves `escapeHTML` call from localization controller to `umb-localize` element * Adds supporting unit-test * Removed unit-test as it is now expected that the localization controller will return literal HTML markup. * Updated import path * Removed extra call to `text()`
1 parent 86e7343 commit 134c800

File tree

5 files changed

+22
-19
lines changed

5 files changed

+22
-19
lines changed

src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,6 @@ describe('UmbLocalizeController', () => {
175175
expect((controller.term as any)('logout', 'Hello', 'World')).to.equal('Log out');
176176
});
177177

178-
it('should encode HTML entities', () => {
179-
expect(controller.term('withInlineToken', 'Hello', '<script>alert("XSS")</script>'), 'XSS detected').to.equal(
180-
'Hello &lt;script&gt;alert(&#34;XSS&#34;)&lt;/script&gt;',
181-
);
182-
});
183-
184178
it('only reacts to changes of its own localization-keys', async () => {
185179
const element: UmbLocalizationRenderCountElement = await fixture(
186180
html`<umb-localization-render-count></umb-localization-render-count>`,

src/Umbraco.Web.UI.Client/src/libs/localization-api/localization.controller.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import type {
2020
import { umbLocalizationManager } from './localization.manager.js';
2121
import type { LitElement } from '@umbraco-cms/backoffice/external/lit';
2222
import type { UmbController, UmbControllerHost } from '@umbraco-cms/backoffice/controller-api';
23-
import { escapeHTML } from '@umbraco-cms/backoffice/utils';
2423

2524
const LocalizationControllerAlias = Symbol();
2625
/**
@@ -137,20 +136,16 @@ export class UmbLocalizationController<LocalizationSetType extends UmbLocalizati
137136
return String(key);
138137
}
139138

140-
// As translated texts can contain HTML, we will need to render with unsafeHTML.
141-
// But arguments can come from user input, so they should be escaped.
142-
const sanitizedArgs = args.map((a) => escapeHTML(a));
143-
144139
if (typeof term === 'function') {
145-
return term(...sanitizedArgs) as string;
140+
return term(...args) as string;
146141
}
147142

148143
if (typeof term === 'string') {
149-
if (sanitizedArgs.length) {
144+
if (args.length) {
150145
// Replace placeholders of format "%index%" and "{index}" with provided values
151146
term = term.replace(/(%(\d+)%|\{(\d+)\})/g, (match, _p1, p2, p3): string => {
152147
const index = p2 || p3;
153-
return typeof sanitizedArgs[index] !== 'undefined' ? String(sanitizedArgs[index]) : match;
148+
return typeof args[index] !== 'undefined' ? String(args[index]) : match;
154149
});
155150
}
156151
}

src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ describe('umb-localize', () => {
9595
expect(element.shadowRoot?.innerHTML).to.contain('Hello World');
9696
});
9797

98+
it('should localize a key with multiple arguments as encoded HTML', async () => {
99+
element.key = 'general_moreThanOneArgument';
100+
element.args = ['<strong>Hello</strong>', '<em>World</em>'];
101+
await elementUpdated(element);
102+
103+
expect(element.shadowRoot?.innerHTML).to.contain('&lt;strong&gt;Hello&lt;/strong&gt; &lt;em&gt;World&lt;/em&gt;');
104+
});
105+
98106
it('should localize a key with args as an attribute', async () => {
99107
element.key = 'general_moreThanOneArgument';
100108
element.setAttribute('args', '["Hello","World"]');

src/Umbraco.Web.UI.Client/src/packages/core/localization/localize.element.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { css, customElement, html, property, state, unsafeHTML } from '@umbraco-cms/backoffice/external/lit';
2+
import { escapeHTML } from '@umbraco-cms/backoffice/utils';
23
import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element';
34

45
/**
@@ -34,7 +35,11 @@ export class UmbLocalizeElement extends UmbLitElement {
3435

3536
@state()
3637
protected get text(): string {
37-
const localizedValue = this.localize.term(this.key, ...(this.args ?? []));
38+
// As translated texts can contain HTML, we will need to render with unsafeHTML.
39+
// But arguments can come from user input, so they should be escaped.
40+
const escapedArgs = (this.args ?? []).map((a) => escapeHTML(a));
41+
42+
const localizedValue = this.localize.term(this.key, ...escapedArgs);
3843

3944
// If the value is the same as the key, it means the key was not found.
4045
if (localizedValue === this.key) {
@@ -44,12 +49,13 @@ export class UmbLocalizeElement extends UmbLitElement {
4449

4550
(this.getHostElement() as HTMLElement).removeAttribute('data-localize-missing');
4651

47-
return localizedValue;
52+
return localizedValue.trim();
4853
}
4954

5055
override render() {
51-
return this.text.trim()
52-
? html`${unsafeHTML(this.text)}`
56+
const text = this.text;
57+
return text
58+
? unsafeHTML(text)
5359
: this.debug
5460
? html`<span style="color:red">${this.key}</span>`
5561
: html`<slot></slot>`;

src/Umbraco.Web.UI.Client/src/packages/core/utils/sanitize/escape-html.function.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const NON_ALPHANUMERIC_REGEXP = /([^#-~| |!])/g;
44

55
/**
66
* Escapes HTML entities in a string.
7-
* @example escapeHTML('<script>alert("XSS")</script>'), // "&lt;script&gt;alert(&quot;XSS&quot;)&lt;/script&gt;"
7+
* @example escapeHTML('<script>alert("XSS")</script>'), // "&lt;script&gt;alert(&#34;XSS&#34;)&lt;/script&gt;"
88
* @param html The HTML string to escape.
99
* @returns The sanitized HTML string.
1010
*/

0 commit comments

Comments
 (0)