html-safe helper

[mmun]

Rendered

[rwjblue]

I am very much in favor of this!!!

I think it may be useful to mention:

• that this can be easily polyfilled going back quite a ways
• that {{html-safe is massively better and less trolly than {{{
• that {{{ is commonly linted against

Thanks for putting this together!!

[rwjblue]

s/soemtimes/sometimes/

[mmun]

@rwjblue Great point. I hadn't considered it as a complete alternative to {{{ ... }}}, since I usually use it in dynamic styles.

[kellyselden]

I think this should mention the trade-off of new API vs the existing triple curly way of doing it. Also, any confusion of which one is better to use, and if the existing {{{}}} should be a deprecation candidate.

[mmun]

@kellyselden 👍 One of the benefits is that html-safe is a subexpression so you can pass the result around. Syntactically, {{{ }}} is also easy to confuse with {{ }}. Did you have other reasons in mind @rwjblue?

Question: does {{{ }}} work in attributes like style?

[kellyselden]

I didn't think about message passing benefits. That's a good one.

Question: does {{{ }}} work in attributes like style?

Just tested. Yes it does.

[lukemelia]

I think this is a good idea. Pretty much all of our apps have this exact helper.

[ef4]

I’m wondering about the name. It seems like a security troll because it’s easy for someone to assume that this makes a thing safe, when the opposite is true.

(I know we already use this name in JS, and even there it’s not wonderful. But making it even more convenient to find and use means more beginner level devs will see it and use it.)

Some of the comments here are making me wonder if even RFC readers are thinking of the full security implications of what htmlSafe does. Switching from triple curlies to this helper does not change your security — it just makes it harder to notice if you’re doing something dangerous. I’m especially concerned that people think this is a good way to avoid triple curly linter warnings, because it’s just a way to elude those very legitimate security warnings. Any project that lints for triple curlies should also lint for this new helper.

I’m still in favor because I agree it’s better to be able to separate the location where you know a value is safe from the value where it’s ultimately bound into the DOM. But I want us to consider names that convey the gravity of what you’re doing. Just to throw one out:trust-this-html

[balinterdi]

unescape(d) or raw?

[ef4]

One more naming consideration: we should probably keep html as part of the name, because being safe/trusted/raw for use in HTML is different from being safe/trusted/raw for other contexts like CSS. In the future we may want to provide a stronger built-in distinction.

[GavinJoyce]

Just to throw one out: trust-this-html

trusted-html perhaps?

[paddyobrien]

Perhaps dangerously-render-html borrowing from reacts dangerouslySetInnerHTML

[GavinJoyce]

Perhaps dangerously-render-html borrowing from reacts dangerouslySetInnerHTML

I'm not sure that this is a completely apt name as, in Ember's case, the function isn't rendering anything. I do like how explicit the name is otherwise. Perhaps dangerous-html?

If we do use a name other than html-safe (which I think we should), it would be valuable for this name to be used consistently across the framework. If we used a name like trusted-html or dangerous-html, we could also use it as follows in place of htmlSafe. For example:

import { trustedHtml } from '@ember/string';

let html = trustedHtml('<div>someString</div>');

{{trusted-html email.parsedAndCleanedHtml}}

or

import { dangerousHtml } from '@ember/string';

let html = dangerousHtml('<div>someString</div>');

{{dangerous-html email.parsedAndCleanedHtml}}

[cibernox]

My points are:

1. Will the triple curly ({{{}}) be deprecated? If so, I believe it should be part of of this RFC.
2. Others have suggested that safe-html can misguide users. While I may see their point, I think that having the javascript API and the HTMLbars API aligned is good and that potential confusion it's not worth it.

Other than that, I like this RFC a lot.

[ef4]

I think that having the javascript API and the HTMLbars API aligned is good and that potential confusion it's not worth it.

Yeah, I think that's an argument for adding the new name (I think trustedHTML is my favorite of the suggestions so far) as both a helper and as an importable Javascript function, and we would deprecate the old htmlSafe function.

[cibernox]

Yeah, I think that's an argument for adding the new name (I think trustedHTML is my favorite of the suggestions so far) as both a helper and as an importable Javascript function, and we would deprecate the old htmlSafe function

If the RFC includes renaming the JS api to align it with the helper that's better. Although I'm unsure it is worth the churn TBH.

[mmun]

We discussed this at the core team meeting on 2018-03-30 and decided we are not comfortable with this proposal and encourage the author to rework the existing APIs and new template helper to be clearer in their intent.

[localpcguy]

We've had the exact concerns that @ef4 mentioned above about the name implying that it somehow makes the html "safe", and I've had to explain that to coworkers (after I understood it myself.)

I personally like raw - potentially tacking HTML on the end (rawHTML). I like trustedHTML also. And agree the JS helper should also be updated.

[locks]

Rust went the opposite approach for doing something similar, the unsafe block denotes code that isn't checked by the Rust compiler but the programmer pinky swears is actually safe.

[balinterdi]

We discussed this at the core team meeting on 2018-03-30 and decided we are not comfortable with this proposal and encourage the author to rework the existing APIs and new template helper to be clearer in their intent.

@mmun The author is you, right? :) What was the main objection? Would coming up with a good name make the intent clearer?

(FWIW, I really like the unsafe name @locks mentions above.)

[mmun]

@balinterdi yes and yes. I think even the Rust people don't think unsafe blocks are the named accurately. It's more like a manually_verfied_to_be_safe block. We probably want to use something trust related, like in @ef4 and @GavinJoyce.

[wycats]

Rust went the opposite approach for doing something similar, the unsafe block denotes code that isn't checked by the Rust compiler but the programmer pinky swears is actually safe.

I think that, in retrospect, this has caused more confusion than something like trust (because it conflates functions that are unsafe to use and a promise that you are using unsafe functionality safely).

I believe that trust is the right way to express what is happening here. TL;DR "Hey Glimmer, you can trust this string". Typically, you'd use this when the string came from your keyboard, or because you already escaped it for the context in which it will be interpolated.

[wycats]

Also, the thing I don't like about "dangerous" or "unsafe" is that it doesn't really tell you why what you're doing is unsafe, and people are willing to stab at it to make errors go away. On the other hand, "trust" is talking about the string ("you can trust the string"). I still think people will likely stab at it, but when they're doing so, they will more likely understand what they're saying.

[wycats]

I also think that Rust's unsafe is too coarse.

Instead of this current API:

// some bytes, in a vector
let sparkle_heart = vec![240, 159, 146, 150];

let sparkle_heart = unsafe {
    String::from_utf8_unchecked(sparkle_heart)
};

assert_eq!("💖", sparkle_heart);

I think this API would be nicer:

// some bytes, in a vector
let sparkle_heart = unsafe { AssertUTF8(vec![240, 159, 146, 150]) };

let sparkle_heart = String::from(sparkle_heart);
assert_eq!("💖", sparkle_heart);

We have a similar situation with context: I want it to be possible to assert that some string has been validly escaped for HTML, CSS, script or whatever, rather than just marking a string as generally trusted.

[GavinJoyce]

@mmun what's the current status of this RFC? Let me know if you don't have time to rework it, I'd be happy to draft a new RFC based on the discussion here

[locks]

@GavinJoyce if you are still interested, I would suggest you take the wheel!

[GavinJoyce]

@locks 👍 I'll pick this up and work on a new RFC, thanks

[GavinJoyce]

Here's a new RFC which proposes that we:

• Deprecate htmlSafe in favor of a new trustedHtml function.
• Deprecate {{{ in favor of a new trusted-html handlebars helper.

[rwjblue]

@mmun - Any objections to closing in favor of iterating further over in #443?