i18next / i18next-icu

i18nFormat plugin to use ICU format with i18next
MIT License
79 stars 17 forks source link

ICU breaks 'replace' and 'replaceObjects: true' & XSS #69

Open hermanbanken opened 6 months ago

hermanbanken commented 6 months ago

When just reading up and trying to use i18next-icu, I failed to understand some details about it, that caused me to think i18next, or i18n-icu was broken/bugged software that would not work reliably. I've now traced it back to my understanding being off. The problems I faced:

  1. For safety I did not wan't to mix properties such as "lng" and "ns" with user generated interpolation dictionaries. So I put the user data in the replace option, to avoid polluting the main options. However, I now understand that IntlMessageFormat is called directly with options and it requires the replacements to be present at the root.
  2. replaceObjects does not work when using ICU. IntlMessageFormat can only work with strings. I expected the objects to be handled with i18next and the individual translations using ICU, however, it appears that ICU is used straight away, so the object is returned without nested fields being translated. This was quite disappointing to learn, as i18next-icu breaks this great feature of i18next itself, and it is documented nowhere.
  3. XSS is suddenly a risk, even though i18n's own options. Because if the user provided values contain "hacked" then this is in the output.

I really want to use ICU, because of CrowdIn plurals (https://support.crowdin.com/icu-message-syntax/?q=plural) but it seems that I can't use this module in the current state.

jamuhl commented 6 months ago

Basically using i18next-icu - non of the i18next specific stuff is available...only what intl-messageformat provides...

so all things that are bound to i18next plurals, interpolation, context does not get applied...only the direct t call with messageformat specific options.

Alternative: use locize.com -> the i18next plurals work perfectly there (beside other benefits). And you directly support the people bringing i18next to you.

jamuhl commented 6 months ago

if not sure if i18next format is mature enough in comparison to intl...just check https://npmtrends.com/react-i18next-vs-react-intl

hermanbanken commented 6 months ago

Note that my use case is NOT React, but something server-side. I need to be able to render to string, not asynchronous via React Suspense, etc.

Alternative: use locize.com -> the i18next plurals work perfectly there (beside other benefits). And you directly support the people bringing i18next to you.

I can't make that decision.

jamuhl commented 6 months ago

i18next with both client and serverside in mind. first version basically was pure clientside js and serverside nodejs with expressjs

--> latest module for serverside: https://github.com/i18next/i18next-http-middleware

hermanbanken commented 6 months ago

Thanks for your quick reply.

Basically using i18next-icu - non of the i18next specific stuff is available...only what intl-messageformat provides...

so all things that are bound to i18next plurals, interpolation, context does not get applied...only the direct t call with messageformat specific options.

I really think this should be part of the docs! A big fat disclaimer a the top.

jamuhl commented 6 months ago

https://github.com/i18next/i18next-icu/tree/master?tab=readme-ov-file#hints

hermanbanken commented 6 months ago

https://github.com/i18next/i18next-icu/commit/8811eb022aaeb33c3ec2a3f13866c11186a29a2c 👍 ❤️

i18next-icu calls Intl immediately. I have the feeling though this is a design choice, and instead the plugin could also be implemented in a way that only the interpolation is handed off to Intl, still supporting objects, don't you agree?

jamuhl commented 6 months ago

just one other sidenote to consider...there is a working group for messageformat 2 https://github.com/unicode-org/message-format-wg so the current icu stuff might be outdated midterm

jamuhl commented 6 months ago

the design choice was supporting some formats beside i18next: https://www.i18next.com/overview/plugins-and-utils#i18n-formats

as those are very different eg. fluent coming with some catalog format not only single strings we made the decision you get stuff like language-detectors, backends, ... but all the interpolation, nesting, formatting,... will be as in the raw format

hermanbanken commented 6 months ago

Clear. It just wasn't clear to me that switching format you then loose all i18n features I was looking for 😅 I have static resources and a language is also already given (set in user profile), so really the only relevant thing was the interpolation & formatting. (Coming from a place where we did dictionary lookups and { } splitting & XSS-protection ourselves).

jamuhl commented 6 months ago

then why not just use https://formatjs.io/docs/intl-messageformat/