Cropster / ember-l10n

A GNU gettext based localization workflow for Ember
MIT License
15 stars 7 forks source link

Investigate alternatives to eval #105

Open Ramblurr opened 2 years ago

Ramblurr commented 2 years ago

Currently this (great) addon uses unsafe code evaluation here, so it cannot be used on sites without specifying Content-Security-Policy script-src 'unsafe-eval' ....; This is a no go in applications with higher security requirements.

Ideally we could find a way to refactor the code to not use evaluation.

I'm sure there was a good reason to use eval in the first place, rather than static code, but I'm not sure what those are. Do you remember @mydea ?

mydea commented 2 years ago

This has been using eval since the very beginning I think - I didn't do the initial implementation, but it is AFAIK not that easy to properly rewrite this to static code because gettext allows a lot of kind of weird things in there that are a bit hard to support.

See this list: http://docs.translatehouse.org/projects/localization-guide/en/latest/l10n/pluralforms.html You can add brackets, spaces etc. at will, technically. Which makes it pretty tricky/error prone to try to solve this with a regex or something like this, I fear...

I recently made a kind of soft fork of this addon: https://github.com/fabscale/ember-gettext as the current architecture is not very well suited for the newer Embroider build system - ran into lots of problems there that were kind of hard to solve without some basic architectural changes. ember-gettext is mostly drop in from an application-code perspective (meaning the usage of the service & helpers are more or less the same), but the build/generation is rather different. For the pluralization, it uses the browsers built-in Intl.PluralRules - see here.

However, that is also not without it's problems, as the browser rules don't necessarily always match the gettext rules - as in theory you could define any plural rule for any locale, e.g. I could have a de-translation with a plural rule nplurals=2; plural=(n!=1);, which is not really the correct rule for de, but could technically be defined. Using the Intl stuff should work fine for 98% of cases, though - I added tests for all the common languages I could think of and that should all be working OK. It's a bit of a trade off, I guess - using eval is probably the most safe & exact implementation of the gettext protocol.

Ramblurr commented 2 years ago

Thanks for the rundown @mydea!

Do I understand right that in ember-gettext the eval stage is moved to build/compile time? Using eval during the build process is fine, we'd like to remove it from the browser execution context.

mydea commented 2 years ago

Basically ember-gettext does not use eval at all anymore, but a different approach. Moving it to eval-time is not really possible because as of now this is done based on the string content of the loaded translation file, which is a JSON file. So it's not really possible to do that.

Mulling this over, IMHO the best way to get this working without eval in the current implementation of this addon is to allow to configure the plural methods yourself and to ignore the plural config of the JSON file, thus optionally skipping the eval.

You could realize this yourself realtively easily with the current codebase by putting this in the extended services/l10n.js:

 _pluralFactory(pluralForm) {
  // Manually map the pluralForm string to a method
  // for example:
  switch (pluralForm) {
    case 'nplurals=2; plural=(n != 1);': 
      return pluralFormMethod1;
   case '....':
      return pluralFormMethod2;
}

function pluralFormMethod1(n) {
  // this is basically the pluralForm string
  let nplurals=2; 
  let plural=(n != 1);

// this is always the same, but could also be streamlined for each type of plural
      switch (typeof plural) {
        case 'boolean':
          plural = plural ? 1 : 0;
          break;
        case 'number':
          plural = plural;
          break;
        default:
          plural = 0;
      }
      var max = nplurals - 1;
      if (plural > max) {
        plural = 0;
      }
      return {
        plural: plural,
        nplurals: nplurals
      };
}

I haven't tested this, but I think that should work!