toString() and SafeString() work individually within a template, but not combined

[ajeffrey]

I've written a number of classes which need to be insertable into a handlebars template, which should be replaced with HTML generated by the toString() method in the class. Now, these templates can receive either instances of said classes, or regular strings, as data so I need to use double-stash {{}} injection instead of triple-stash {{{}}}. As per my understanding of the documentation and using toString() and SafeString individually, I should be able to have my toString method return a SafeString and this should go into the template unescaped. However, this is not the case as seen below.

Here's a test case (written in coffeescript/node.js using the latest "handlebars" npm module):

handlebars = require('handlebars')
class Test
  toString: () ->
    return new handlebars.SafeString('<u>hello world</u>')

template = handlebars.compile('<b>{{contents}}</b>')
view = template({contents: new Test()})

Expected result:
at the end of the script, "view" should be a string containing "< b >< u >hello world</ u ></ b >" (without spaces)

Actual result:
a TypeError is thrown, with the message "Cannot convert object to primitive value"

Relevant part of stack trace:

at escapeExpression (<project>\node_modules\handlebars\dist\cjs\handlebars\utils.js:69:15)
at Object.eval (<anonymous>:4:7)
at ret (<project>\node_modules\handlebars\dist\cjs\handlebars\runtime.js:137:30)
at ret (<project>\node_modules\handlebars\dist\cjs\handlebars\compiler\compiler.js:422:21)

I will try to put together a pull request for this but my knowledge of the workings of Handlebars is limited. I need to get this fixed fairly soon as I have a looming deadline so let me know if there's anything I can do to help.

Thanks,
Alex

[ajeffrey]

I'm on handlebars v2.0.0 and the error seems to trace to this line:
https://github.com/wycats/handlebars.js/blob/master/lib/handlebars/utils.js#L67

[ajeffrey]

I've tracked down how to fix this, but can't get the handlebars test suite running on my (windows) machine so I'm not going to send a pull request without a matching test case.

the fix is to add the following to the start of the escapeExpression function in utils.js:

if (typeof string == 'object' && 'toString' in string) {
  string = string.toString();
}

I've not explored the security or other implications of this in too much depth, but it doesn't cancel out the existing escaping mechanisms so unless the toString method returns a SafeString (as mine does) the result will still be escaped.

[kpdecker]

This is not the intended API for SafeString. The contents value must literally be a SafeString instance, not the toString proxy that you are doing here.

#886 has some discussions on how we can expand the support for SafeString behaviors to other objects. Moving the discussion there.