Skip to content

fix(translate): escape special character % to prevent crash #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(translate): escape special character % to prevent crash #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 51 additions & 45 deletions packages/react-i18n/src/utils/__tests__/i18n.utils.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ describe('i18n translate function', () => {
it('should pluralize with default value', () => {
const lang = { foo: { bar: { one: 'foo bar!', other: 'foos bars!!!' } } };
const t = translate(lang);
expect(t('foo.bar', {number: 0, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 1, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 2, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', { number: 0, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 1, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 2, general: true })).toBe('foos bars!!!');
});

it('should pluralize in french', () => {
Expand All @@ -44,9 +44,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'fr' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 1, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 2, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', { number: 0, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 1, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 2, general: true })).toBe('foos bars!!!');
});

it('should pluralize in english', () => {
Expand All @@ -55,9 +55,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'en' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', {number: 1, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 2, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', { number: 0, general: true })).toBe('foos bars!!!');
expect(t('foo.bar', { number: 1, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 2, general: true })).toBe('foos bars!!!');
});

it('should pluralize in hungarian', () => {
Expand All @@ -66,9 +66,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'hu' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 1, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 2, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', { number: 0, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 1, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 2, general: true })).toBe('foos bars!!!');
});

it('should pluralize in croatian', () => {
Expand All @@ -85,9 +85,9 @@ describe('i18n translate function', () => {
};
const t = translate(lang);

expect(t('foo.bar', {number: 0, general: true})).toBe('first plural');
expect(t('foo.bar', {number: 1, general: true})).toBe('first plural');
expect(t('foo.bar', {number: 2, general: true})).toBe('general plural');
expect(t('foo.bar', { number: 0, general: true })).toBe('first plural');
expect(t('foo.bar', { number: 1, general: true })).toBe('first plural');
expect(t('foo.bar', { number: 2, general: true })).toBe('general plural');
});

it('should pluralize in dutch', () => {
Expand All @@ -96,19 +96,19 @@ describe('i18n translate function', () => {
_i18n: { lang: 'nl' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', {number: 1, general: true})).toBe('foo bar!');
expect(t('foo.bar', {number: 2, general: true})).toBe('foos bars!!!');
expect(t('foo.bar', { number: 0, general: true })).toBe('foos bars!!!');
expect(t('foo.bar', { number: 1, general: true })).toBe('foo bar!');
expect(t('foo.bar', { number: 2, general: true })).toBe('foos bars!!!');
});
});

describe('with number interpolation', () => {
it('should pluralize with default value', () => {
const lang = { foo: { bar: { one: '%(number)d foo bar!', other: '%(number)d foos bars!!!' } } };
const t = translate(lang);
expect(t('foo.bar', {number: 0})).toBe('0 foo bar!');
expect(t('foo.bar', {number: 1})).toBe('1 foo bar!');
expect(t('foo.bar', {number: 2})).toBe('2 foos bars!!!');
expect(t('foo.bar', { number: 0 })).toBe('0 foo bar!');
expect(t('foo.bar', { number: 1 })).toBe('1 foo bar!');
expect(t('foo.bar', { number: 2 })).toBe('2 foos bars!!!');
});

it('should pluralize in french', () => {
Expand All @@ -117,9 +117,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'fr' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0})).toBe('0 foo bar!');
expect(t('foo.bar', {number: 1})).toBe('1 foo bar!');
expect(t('foo.bar', {number: 2})).toBe('2 foos bars!!!');
expect(t('foo.bar', { number: 0 })).toBe('0 foo bar!');
expect(t('foo.bar', { number: 1 })).toBe('1 foo bar!');
expect(t('foo.bar', { number: 2 })).toBe('2 foos bars!!!');
});

it('should pluralize in english', () => {
Expand All @@ -128,9 +128,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'en' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0})).toBe('0 foos bars!!!');
expect(t('foo.bar', {number: 1})).toBe('1 foo bar!');
expect(t('foo.bar', {number: 2})).toBe('2 foos bars!!!');
expect(t('foo.bar', { number: 0 })).toBe('0 foos bars!!!');
expect(t('foo.bar', { number: 1 })).toBe('1 foo bar!');
expect(t('foo.bar', { number: 2 })).toBe('2 foos bars!!!');
});

it('should pluralize in english', () => {
Expand All @@ -139,9 +139,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'en' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0})).toBe('0 foos bars!!!');
expect(t('foo.bar', {number: 1})).toBe('1 foo bar!');
expect(t('foo.bar', {number: 2})).toBe('2 foos bars!!!');
expect(t('foo.bar', { number: 0 })).toBe('0 foos bars!!!');
expect(t('foo.bar', { number: 1 })).toBe('1 foo bar!');
expect(t('foo.bar', { number: 2 })).toBe('2 foos bars!!!');
});

it('should pluralize in hungarian', () => {
Expand All @@ -150,9 +150,9 @@ describe('i18n translate function', () => {
_i18n: { lang: 'hu' },
};
const t = translate(lang);
expect(t('foo.bar', {number: 0})).toBe('0 foo bar!');
expect(t('foo.bar', {number: 1})).toBe('1 foo bar!');
expect(t('foo.bar', {number: 2})).toBe('2 foo bar!');
expect(t('foo.bar', { number: 0 })).toBe('0 foo bar!');
expect(t('foo.bar', { number: 1 })).toBe('1 foo bar!');
expect(t('foo.bar', { number: 2 })).toBe('2 foo bar!');
});

it('should pluralize in croatian', () => {
Expand All @@ -168,17 +168,17 @@ describe('i18n translate function', () => {
};
const t = translate(lang);
// first plural form
[1, 21, 31, 41, 101].forEach(i => expect(t('foo.bar', {number: i})).toBe(`${i} first plural`));
[1, 21, 31, 41, 101].forEach(i => expect(t('foo.bar', { number: i })).toBe(`${i} first plural`));

// second plural form
[2, 3, 4, 22, 23, 24, 32, 33, 34].forEach(i => expect(t('foo.bar', {number: i})).toBe(`${i} second plural`));
[2, 3, 4, 22, 23, 24, 32, 33, 34].forEach(i => expect(t('foo.bar', { number: i })).toBe(`${i} second plural`));

// third plural form
for (let i = 5; i < 21; i++) {
expect(t('foo.bar', {number: i})).toBe(`${i} third plural`);
expect(t('foo.bar', { number: i })).toBe(`${i} third plural`);
}
[0, 25, 26, 27, 28, 29, 30, 35, 36, 37, 38, 39, 40, 45].forEach(i =>
expect(t('foo.bar', {number: i})).toBe(`${i} third plural`),
expect(t('foo.bar', { number: i })).toBe(`${i} third plural`),
);
});
});
Expand All @@ -197,7 +197,7 @@ describe('i18n translate function', () => {
const renderers = { Bold };
const t = translate(lang, undefined, undefined, true);

const result = t('foo.bar', {renderers});
const result = t('foo.bar', { renderers });
const wrapper = mount(<div>{result}</div>);

expect(wrapper).toMatchSnapshot();
Expand Down Expand Up @@ -227,7 +227,7 @@ describe('i18n translate function', () => {
const renderers = { Bold, Italic };
const t = translate(lang, undefined, undefined, true);

const result = t('foo.bar', {renderers});
const result = t('foo.bar', { renderers });
const wrapper = mount(<div>{result}</div>);

expect(wrapper).toMatchSnapshot();
Expand All @@ -242,7 +242,7 @@ describe('i18n translate function', () => {
const renderers = { LineBreak };
const t = translate(lang, undefined, undefined, true);

const result = t('foo.bar', {renderers});
const result = t('foo.bar', { renderers });
const wrapper = mount(<div>{result}</div>);

expect(wrapper).toMatchSnapshot();
Expand All @@ -257,7 +257,7 @@ describe('i18n translate function', () => {
const renderers = { LineBreak, Bold, Italic };
const t = translate(lang, undefined, undefined, true);

const result = t('foo.bar', {renderers});
const result = t('foo.bar', { renderers });
const wrapper = mount(<div>{result}</div>);

expect(wrapper).toMatchSnapshot();
Expand All @@ -271,8 +271,8 @@ describe('i18n translate function', () => {
};
const t = translate(lang, undefined, undefined, true);

expect(t('foo.bar', { renderers: {Bold} })).toMatchSnapshot();
expect(t('foo.bar', { renderers: {Italic} })).toMatchSnapshot();
expect(t('foo.bar', { renderers: { Bold } })).toMatchSnapshot();
expect(t('foo.bar', { renderers: { Italic } })).toMatchSnapshot();
});

it('should interpolate basic html tag', () => {
Expand All @@ -284,7 +284,7 @@ describe('i18n translate function', () => {
const renderers = {};
const t = translate(lang, undefined, undefined, true);

const result = t('foo.bar', { renderers});
const result = t('foo.bar', { renderers });
const wrapper = mount(<div>{result}</div>);

expect(wrapper).toMatchSnapshot();
Expand All @@ -300,7 +300,7 @@ describe('i18n translate function', () => {
const renderers = { Bold, Italic };
const t = translate(lang, undefined, undefined, true);

const result = t('foo.bar', {renderers});
const result = t('foo.bar', { renderers });
const wrapper = mount(<div>{result}</div>);

expect(wrapper).toMatchSnapshot();
Expand Down Expand Up @@ -328,6 +328,12 @@ describe('i18n translate function', () => {
expect(wrapper).toMatchSnapshot();
});
});

it('should not crash when translate key contain special char %', () => {
const lang = { foo: { bar: 'I can dispay %(name)s without crashing !' } };
const t = translate(lang);
expect(t('foo.bar', { data: { name: 'special %chars' } })).toBe('I can dispay special %chars without crashing !');
});
});

describe('i18n listBuilder function', () => {
Expand Down
12 changes: 11 additions & 1 deletion packages/react-i18n/src/utils/i18n.utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import _get from 'lodash/get';
import _has from 'lodash/has';
import _mapValues from 'lodash/mapValues';
import _noop from 'lodash/noop';
import { sprintf } from 'sprintf-js';
import { interpolateHTMLTags } from './html.utils';
Expand Down Expand Up @@ -57,7 +58,16 @@ export const translate = (lang, i18nNames = {}, errorCallback = _noop, parseHTML
translation = combineKey;
}

const translatedResult = sprintf(translation, { ...data, ...i18nNames, number });
// escape special characters (% for now)
const translationValues = _mapValues({ ...data, ...i18nNames, number }, value => {
if (value) {
Copy link

@qtomasicchio qtomasicchio Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the unit test, value can be a number. We should probably check that we have a string, what do you think?

Suggested change
if (value) {
if (typeof value === 'string') {

return value.replace(/%/g, '%%');
}

return value;
});

const translatedResult = sprintf(translation, translationValues).replace(/%%/g, '%');

if (!parseHTML) return translatedResult;

Expand Down
Loading