Merge pull request #72 from yahoo/exclude-SafeString-from-escaping

exclude Handlebars.SafeString from escaping
YahooArchive · Jun 15, 2015 · d9f85c4 · d9f85c4
2 parents 5338907 + 8629fef
commit d9f85c4
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 8 deletions.
diff --git a/src/secure-handlebars.js b/src/secure-handlebars.js
@@ -13,7 +13,7 @@ var Handlebars = require('handlebars'),
     handlebarsUtils = require('./handlebars-utils.js');
 
 var hbsCreate = Handlebars.create,
-    privateFilters = ['y', 'yd', 'yc', 'yavd', 'yavs', 'yavu', 'yu', 'yuc', 'yubl', 'yufull', 'yceu', 'yced', 'yces', 'yceuu', 'yceud', 'yceus'],
+    privateFilters = ['yd', 'yc', 'yavd', 'yavs', 'yavu', 'yu', 'yuc', 'yubl', 'yufull', 'yceu', 'yced', 'yces', 'yceuu', 'yceud', 'yceus'],
     baseContexts = ['HTMLData', 'HTMLComment', 'SingleQuotedAttr', 'DoubleQuotedAttr', 'UnQuotedAttr'],
     contextPrefixes = ['in', 'uriIn', 'uriPathIn', 'uriQueryIn', 'uriComponentIn', 'uriFragmentIn'];
 
@@ -52,9 +52,24 @@ function overrideHbsCreate() {
         return c.call(this, preprocess(template, options.strictMode), options);
     };
 
+    // make y refer to the default escape function
+    h.registerHelper('y', Handlebars.escapeExpression);
+
+    // don't escape SafeStrings, since they're already safe according to Handlebars
+    // Reference: https://github.com/wycats/handlebars.js/blob/master/lib/handlebars/utils.js#L63-L82
+    function safeStringCompatibleFilter (filterName) {
+        return function (s) {
+            // Unlike escapeExpression(), return s instead of s.toHTML() since downstream
+            //  filters of the same chain has to be disabled too.
+            //  Handlebars will invoke SafeString.toString() at last during data binding
+            return (s && s.toHTML) ? s : privFilters[filterName](s);
+        };
+    }
+
+    /*jshint -W083 */
     // register below the filters that are automatically applied by context parser 
     for (i = 0; (filterName = privateFilters[i]); i++) {
-        h.registerHelper(filterName, privFilters[filterName]);
+        h.registerHelper(filterName, safeStringCompatibleFilter(filterName));
     }
 
     // register below the filters that might be manually applied by developers

diff --git a/tests/integration/run-secure-handlebars-spec.js b/tests/integration/run-secure-handlebars-spec.js
@@ -16,7 +16,13 @@ describe("SecureHandlebars: inclusion tests", function() {
 
 });
 
-describe("SecureHandlebars: helpers existence tests", function() {
+function HandlebarsGetOutput(html, json) {
+    var template = Handlebars.compile(html);
+    var output = template(json);
+    return output;
+}
+
+describe("SecureHandlebars: helpers tests", function() {
 
     it('xss-filters registered as helpers', function(){
         [
@@ -39,13 +45,25 @@ describe("SecureHandlebars: helpers existence tests", function() {
         });
     });
 
+    it("SafeString test", function() {
+        var data = {
+            url: 'javascript:alert(1)', 
+            safeUrl: new Handlebars.SafeString('javascript:alert(1)')
+        };
+
+        var template = '<a href="{{safeUrl}}">SafeURL</a><a href="{{url}}">FilteredUrl</a>';
+
+        expect(HandlebarsGetOutput(template, data)).to.be.equal('<a href="javascript:alert(1)">SafeURL</a><a href="x-javascript:alert(1)">FilteredUrl</a>');
+    });
+
+    // given no definition of script in data, expects to output empty string by Handlebars.escapeExpression()
+    it("Use of Handlebars.escapeExpression() as y", function() {
+        var template = '<script>{{script1}}</script><script>{{script2}}</script>';
+        expect(HandlebarsGetOutput(template, {script2: null})).to.be.equal('<script></script><script></script>');
+    });
+
 });
 
-var HandlebarsGetOutput = function(html, json) {
-    var template = Handlebars.compile(html);
-    var output = template(json);
-    return output;
-};
 
 describe("SecureHandlebars: compilation tests", function() {
     it('smoke template', function(){