Work with SafeString

[gossi]

Hey guys,

I tried useing ember-composable-helpers together with ember-i18n which unfortunately doesn't work (See jamesarosen/ember-i18n#374). The workarounds provided there don't really seem to be enjoyable 😉 (and nothing you want to put in your templates). As suggested the string helpers from here should probably be able to work with SafeString which is why I am creating this issue here, to make those two great addons work together.

Thanks

[codyjroberts]

Simple workaround for this might be:

//addon/-private/create-string-helper.js
export default function(stringFunction) {
  return function([string]) {
    string = string || '';
    return stringFunction(string + '');
  }
}

This doesn't really fix the issue and markup mileage will vary. My guess is the reason why SafeString doesn't support common helpers like capitalize() is that supporting markup would require complex regex with several edge cases.

Take this simple case for example.

"<li>age is foolish and forgetful when it underestimates youth</li>".capitalize()
// "<li>age is foolish and forgetful when it underestimates youth</Li>"

I agree that the extra helpers suggested in https://github.com/jamesarosen/ember-i18n/issues/374 don't really offer a great solution. I'm sure there's a reason ember-i18n chooses to return SafeString, but maybe it should offer the option to return a simple String e.g.

<li>{{capitalize (ts "edit")}}</li>

[gossi]

I'm just on the consumer side of both plugins and would love to use them together. Let's ping @jamesarosen and see what he can bring to the table to find a solution to make it work "the-ember-way".

[jamesarosen]

ember-i18n emits a safe string because translations are generally fed user-generated content, but the translation text often includes markup. The only way to both prevent XSS vulnerabilities and allow app authors to put markup in their translations is for the helper to escape all the interpolations, then mark the result as safe.

In fact, any helper that deals with both user-generated content and markup must do exactly this. To write a helper that doesn't escape its inputs is to provide app authors a security foot-gun.

Rails solved this by making SafeString API-compatible with string. The library doesn't have to know how to capitalize <li>hello</li> because the app author knows the content of that translation and knows that that won't work. (Plus, if you're translating, you should never call capitalize on your own content. Capitalizing or downcasing content in Japanese is nonsensical, and doing it in German destroys the meaning.)

The {{ts}} idea isn't necessarily a bad one, at least until we can convince the core team that SafeSrings should be Liskov-substitutable for Strings. It would still do escaping, so it's not a security risk. If the app author used it with markup, they would just get ugly markup emitted to their page, which is certainly unpleasant for users, but not as bad as the security vulnerability. And whereas an XSS vulnerability is usually invisible during development, an overescaping bug is quite obvious.

[jamesarosen]

Rails's string arithmetic declares unsafety to be infectious:

• string + string => string
• string + SafeString => string
• SafeString + string => string
• SafeString + SafeString => SafeString

This means that if you add an unsafe string to anything, the result is always an unsafe string and will be escaped.

Unfortunately, in ES6, we can't override the + operator:

const { htmlSafe } = Ember.String;
const { escapeExpression } = Ember.Handlebars.Utils;

escapeExpression('<em>foo</em>');
// '&lt;em&gt;foo&lt;/em&gt;'

escapeExpression(htmlSafe('<em>foo</em>'));
// '<em>foo</em>'

escapeExpression(htmlSafe('<em>') + htmlSafe('foo') + htmlSafe('</em>'));
// '&lt;em&gt;foo&lt;/em&gt;'
// because + calls valueOf on the SafeStrings, which coerces them down to strings

Even if we could override the + operator, we can't override the interpolation operator. While Babel compiles

`${foo}bar${baz}`

to

foo + 'bar' + baz

there's no way for a library to intercept that and interrogate the string safety to manipulate the arithmetic, which means even ${safeString}${safeString} would emit a string, and then be escaped.

Thus, even if we could convince the core team that making SafeString Liskov-substitutable for String, we can't implement the correct arithmetic. Without this arithmetic, I don't know of a good general purpose solution to both XSS protection and avoiding overescaping. Any solution will have to involve cooperation from every helper as well as the libraries that compose and use them.

[jamesarosen]

One not-so-happy solution would be for core (or some addon) to provide a concat utility, both as a helper and a pure JS function:

function safeAdd(a, b) {
  if (a instanceof SafeString && b instanceof SafeString) {
    return htmlSafe(a + b);
  }
  return a + b;
}

export function safeConcat(...parts) {
  return parts.reduce((sum, part) => {
    return safeAdd(sum, part);
  }, htmlSafe(''));
}

export default Ember.Helper.helper(function(parts) {
  return safeConcat(...parts);
});

But for that to work, helpers, addons, apps, etc. would all have to use it instead of +. I don't think that's likely.

We might be able to write a Babel plugin that changes a + b to safeConcat(a, b). That would let us fake the operator-overloading we need, though it might be a prohibitive slow-down for apps.

[codyjroberts]

The library doesn't have to know how to capitalize <li>hello</li> because the app author knows the content of that translation and knows that that won't work.

This was pretty much my thought. Why not capitalize translations when they're defined? The best solution to this problem may rely on design by the consumer opposed to the cooperation between the two libraries. A counter-example where this is not possible would be helpful. I don't have much experience in this area. 😄

[gossi]

Well, i can give an example where I am supposed to use both plugins together. My related translations.js:

export default {
  "edit": "edit"
}

I decided to keep "edit" in lower case as it is an english verb (although in the german translations it probably would be "Bearbeiten" - note the already capitalized word here). However, if I'm using this at the beginning of a sentence or at the start of a headline I like to capitalize it. That's what I was trying:

{{capitalize (t "edit")}}

Also I can think of these kind of translation strings:

export default {
  "explanation": "let's do something with <i>{{foo}}</i>"
}

which also may be passed around through various string helpers. From the consumer side I see no problem with that - though the result may be funny or unexpected but calculated. At least no errors.

[jamesarosen]

However, if I'm using this at the beginning of a sentence

I'm not going to say that this is an invalid use-case for ember-i18n, but I would strongly advise against ever building up sentences like this. My rules for translations:

1. if it's a button, call-to-action, link, etc., the translation is the whole text
2. if it's a sentence or paragraph, translations are at least one sentence each; never build a sentence out or parts
3. if you change the semantics (including the number or names of interpolations) of a translation, it gets a new key; typos don't require a new key