Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a SafeValue wrapper for helper functions. #375

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ELLIOTTCABLE
Copy link

Handlebars seems to provide a SafeString wrapper-type for helpers; with it, a helper can create string-y content that is known by the helper (i.e. implementation-code-side) to be ‘escape-safe.’

This means that the template writer needs no knowledge of whether the {{generate}} helper needs to be called with two brackets or three brackets. That's desirable, as having to maintainin that knowledge lessens separation of concerns. (Also, tell me you haven't forgotten at least a billion times, and used the wrong number of brackets due to forgetting which your helper expected? Best case, that forgetfulness will leave some content escaped and showing up to the client as HTML-source … and worst case, you've got an injection vulnerability 'cuz you used triple-brackets one time too often.)


For my implementation approach, I decided upon a duck-typing approach: a helper or library need only wrap values into some sort of structure with an unwrap() method, that returns a Mustache-friendly value; and then set a safe property to a truthy value on that structure. Everything else is ignored by my implementation.

For convenience, I've provided a mustache.SafeValue constructor, to do exactly that. All told, with this patch, all a helper needs to do to prevent their content being escaped, even in double-brackets, is:

view =
   generate: ->
      new mustache.SafeValue "<b>#{generate_content()}</b>"

I have one remaining concern with my approach: helpers, when telling mustache that their content is safe, become responsible for ensuring that it is safe. This means that a helper that's obtaining user-generated content from somewhere has to do its own escaping. That's not exactly world-shocking news or anything, but I'm suspicious that there's an elegant way to reduce the helper/library-writer needing to worry about that.


Feedback appreciated before a merge; this patch isn't quite ready yet.

(This is currently failing a ton of tests on my machine, despite the tiny footprint of the changes … but I'm fairly convinced that's an environmental issue, or due to me not understanding how to set up this project's test-suite. I've done some differentiation, and changes as simple as a noop if/then were causing tests to fail. I have no idea why.)

No user-facing documentation, and failing several tests; but I suspect this is because I don't understand how to set up the project's test-suite (I add a no-op `if/then`, and tests start failing. Something's obviously very strange here.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant