Skip to content

Commit 558a387

Browse files
authored
Merge pull request #756 from nextcloud-libraries/feat/allow-per-replacement-escaping
feat: Allow setting `escape` option per parameter replacing
2 parents 5cf6b4b + 7775257 commit 558a387

2 files changed

Lines changed: 68 additions & 5 deletions

File tree

lib/translation.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ interface TranslationOptions {
1919
sanitize?: boolean
2020
}
2121

22+
/** @notExported */
23+
interface TranslationVariableReplacementObject<T> {
24+
/** The value to use for the replacement */
25+
value: T
26+
/** Overwrite the `escape` option just for this replacement */
27+
escape: boolean
28+
}
29+
30+
/** @notExported */
31+
type TranslationVariables = Record<string, string | number | TranslationVariableReplacementObject<string | number>>
32+
2233
/**
2334
* Translate a string
2435
*
@@ -27,37 +38,48 @@ interface TranslationOptions {
2738
* @param {object} vars map of placeholder key to value
2839
* @param {number} number to replace %n with
2940
* @param {object} [options] options object
41+
* @param {boolean} options.escape enable/disable auto escape of placeholders (by default enabled)
42+
* @param {boolean} options.sanitize enable/disable sanitization (by default enabled)
43+
*
3044
* @return {string}
3145
*/
3246
export function translate(
3347
app: string,
3448
text: string,
35-
vars?: Record<string, string | number>,
49+
vars?: TranslationVariables,
3650
number?: number,
3751
options?: TranslationOptions,
3852
): string {
39-
const defaultOptions = {
53+
const allOptions = {
54+
// defaults
4055
escape: true,
4156
sanitize: true,
57+
// overwrite with user config
58+
...(options || {}),
4259
}
43-
const allOptions = Object.assign({}, defaultOptions, options || {})
4460

4561
const identity = <T, >(value: T): T => value
4662
const optSanitize = allOptions.sanitize ? DOMPurify.sanitize : identity
4763
const optEscape = allOptions.escape ? escapeHTML : identity
4864

65+
const isValidReplacement = (value: unknown) => typeof value === 'string' || typeof value === 'number'
66+
4967
// TODO: cache this function to avoid inline recreation
5068
// of the same function over and over again in case
5169
// translate() is used in a loop
52-
const _build = (text: string, vars?: Record<string, string | number>, number?: number) => {
70+
const _build = (text: string, vars?: TranslationVariables, number?: number) => {
5371
return text.replace(/%n/g, '' + number).replace(/{([^{}]*)}/g, (match, key) => {
5472
if (vars === undefined || !(key in vars)) {
5573
return optEscape(match)
5674
}
5775

5876
const replacement = vars[key]
59-
if (typeof replacement === 'string' || typeof replacement === 'number') {
77+
if (isValidReplacement(replacement)) {
6078
return optEscape(`${replacement}`)
79+
} else if (typeof replacement === 'object' && isValidReplacement(replacement.value)) {
80+
// Replacement is an object so indiviual escape handling
81+
const escape = replacement.escape !== false ? escapeHTML : identity
82+
return escape(`${replacement.value}`)
6183
} else {
6284
/* This should not happen,
6385
* but the variables are used defined so not allowed types could still be given,

tests/translation.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,47 @@ describe('translate', () => {
5757
expect(translation).toBe('Hallo &lt;del&gt;Name&lt;/del&gt;')
5858
})
5959

60+
it('with global placeholder HTML escaping and enabled on parameter', () => {
61+
const text = 'Hello {name}'
62+
const translation = translate('core', text, { name: { value: '<del>Name</del>', escape: true } }, undefined, { escape: true })
63+
expect(translation).toBe('Hallo &lt;del&gt;Name&lt;/del&gt;')
64+
})
65+
66+
it('with global placeholder HTML escaping but disabled on parameter', () => {
67+
const text = 'Hello {name}'
68+
const translation = translate('core', text, { name: { value: '<del>Name</del>', escape: false } }, undefined, { escape: true })
69+
expect(translation).toBe('Hallo <del>Name</del>')
70+
})
71+
72+
it('without global placeholder HTML escaping but enabled on parameter', () => {
73+
const text = 'Hello {name}'
74+
const translation = translate('core', text, { name: { value: '<del>Name</del>', escape: true } }, undefined, { escape: false })
75+
expect(translation).toBe('Hallo &lt;del&gt;Name&lt;/del&gt;')
76+
})
77+
78+
it('without global placeholder HTML escaping and disabled on parameter', () => {
79+
const text = 'Hello {name}'
80+
const translation = translate('core', text, { name: { value: '<del>Name</del>', escape: false } }, undefined, { escape: false })
81+
expect(translation).toBe('Hallo <del>Name</del>')
82+
})
83+
84+
it('with global placeholder HTML escaping and invalid per-parameter escaping', () => {
85+
const text = 'Hello {name}'
86+
// @ts-expect-error We test calling it with an invalid value (missing)
87+
const translation = translate('core', text, { name: { value: '<del>Name</del>' } }, undefined, { escape: true })
88+
// `escape` needs to be an boolean, otherwise we fallback to `false` to prevent security issues
89+
// So in this case `undefined` is falsy but we still enforce escaping as we only accept `false`
90+
expect(translation).toBe('Hallo &lt;del&gt;Name&lt;/del&gt;')
91+
})
92+
93+
it('witout global placeholder HTML escaping and invalid per-parameter escaping', () => {
94+
const text = 'Hello {name}'
95+
// @ts-expect-error We test calling it with an invalid value
96+
const translation = translate('core', text, { name: { value: '<del>Name</del>', escape: 0 } }, undefined, { escape: false })
97+
// `escape` needs to be an boolean, otherwise we fallback to `false` to prevent security issues
98+
expect(translation).toBe('Hallo &lt;del&gt;Name&lt;/del&gt;')
99+
})
100+
60101
it('without placeholder XSS sanitizing', () => {
61102
const text = 'Hello {name}'
62103
const translation = translate('core', text, { name: '<img src=x onerror=alert(1)//>' }, undefined, { sanitize: false, escape: false })

0 commit comments

Comments
 (0)