improvement over PR#23

package.json

@@ -38,6 +38,8 @@
   "dependencies": {
     "express-handlebars": "^2.0.1",
     "handlebars": "^3.0.3",
+    "object.assign": "^3.0.1",
+    "promise": "^7.0.3",
     "secure-handlebars": "^1.1.1",
     "xss-filters": "^1.2.4"
   },

src/express-secure-handlebars.js

@@ -9,8 +9,11 @@ Authors: Nera Liu <[email protected]>
 */
 /*jshint -W030 */
 var util = require("util"),
-    expressHandlebars = require('express-handlebars').ExpressHandlebars,
-    secureHandlebars = require('secure-handlebars');
+    ExpressHandlebars = require('express-handlebars').ExpressHandlebars,
+    secureHandlebars = require('secure-handlebars'),
+    assign = Object.assign || require('object.assign'),
+    Promise = require('promise'),
+    path = require('path');
 
 function ExpressSecureHandlebars(config) {
 
@@ -20,20 +23,123 @@ function ExpressSecureHandlebars(config) {
 
     /* calling super constructor */
     this.constructor.super_.call(this, config);
+
+    // dedicate a special express-handlebars for fetching raw partials
+    this._exphbsForRawPartials = new ExpressHandlebarsForRawPartials(config);
+    this._partialsCache = {
+        preprocessed: {}
+    };
+
+    // compilerOptions is being used for passing params to secure-handlebars
+    this.compilerOptions || (this.compilerOptions = {});
+    this.compilerOptions.shbsPartialsCache = this._partialsCache;
 }
 
 /* inheriting the express-handlebars */
-util.inherits(ExpressSecureHandlebars, expressHandlebars);
+util.inherits(ExpressSecureHandlebars, ExpressHandlebars);
+
+// always returns an empty cache, since the partial is loaded and analyzed on-demand
+// TODO: preprocess all templates, and export all partials for compatibility
+ExpressSecureHandlebars.prototype.getPartials = function (options) {
+    return {};
+};
+
+// retrieve file content directly from the preprocessed partial cache if it exists
+ExpressSecureHandlebars.prototype._getFile = function (filePath, options) {
+
+    var file = this._partialsCache.preprocessed[path.relative('.', filePath)];
+    return file ? Promise.resolve(file) : ExpressHandlebars.prototype._getFile.call(this, filePath, options);
+
+};
+
+
+// _getTemplates literally has the core of getTemplates, except that it takes an array of filePaths as input
+/*
+// getTemplates can be rewritten to use _getTemplates in the following way
+ExpressSecureHandlebars.prototype.getTemplates = function (dirPath, options) {
+    options || (options = {});
+    
+    return this._getDir(dirPath, {cache: options.cache}).map(function (filePath) {
+        return path.join(dirPath, filePath);
+    }).then(function(filePaths) {
+        return this._getTemplates(filePaths, options);
+    }.bind(this));
+};
+*/
+ExpressSecureHandlebars.prototype._getTemplates = function (filePaths, options) {
+    options || (options = {});
+
+    var templates = filePaths.map(function (filePath) {
+        return this.getTemplate(filePath, options);
+    }, this);
+
+    return Promise.all(templates).then(function (templates) {
+        return filePaths.reduce(function (hash, filePath, i) {
+            hash[filePath] = templates[i];
+            return hash;
+        }, {});
+    });
+};
+
 
-/* override ExpressHandlebars.render() to expose filePath as compilerOptions */
+// override render() for on-demand partial preprocessing and (pre-)compilation
 ExpressSecureHandlebars.prototype.render = function (filePath, context, options) {
+    options || (options = {});
+
     // expose filePath as processingFile in compilerOptions for secure-handlebars
-    this.compilerOptions || (this.compilerOptions = {});
     this.compilerOptions.processingFile = filePath;
 
-    return expressHandlebars.prototype.render.call(this, filePath, context, options);
+    // ensure getPartials() is called to fetch raw partials
+    return this._exphbsForRawPartials.getPartials(options).then(function(rawPartialsCache) {
+
+        // in constructor, this.compilerOptions.shbsPartialsCache = this._partialsCache
+        this._partialsCache.raw = rawPartialsCache;
+
+        // this._partialsCache.preprocessed is filled by the underlying (pre-)compile() in getTemplate()
+        // TODO: improve this by keeping the returned (pre-)compiled template
+        return this.getTemplate(filePath, options).catch(function(err){
+            throw err;
+        });
+
+    }.bind(this)).then(function() {
+
+        var partialKeys = Object.keys(this._partialsCache.preprocessed);
+
+        // disable preprocessing partials in partial
+        this.compilerOptions.enablePartialProcessing = false;
+
+        // Merge render-level and (pre-)compiled pre-processed partials together
+        return Promise.all([
+            this._getTemplates(partialKeys, options),   // (pre-)compile pre-processed partials
+            options.partials                            // collected at renderView
+        ]).then(function (partials) {
+            return assign.apply(null, [{}].concat(partials));
+        }).catch(function(err) {
+            throw err;
+        }).finally(function() {
+            // re-enable partial handling
+            this.compilerOptions.enablePartialProcessing = true;
+        }.bind(this));
+
+    }.bind(this)).then(function(partials) {
+
+        options.partials = partials;
+
+        return ExpressHandlebars.prototype.render.call(this, filePath, context, options);
+
+    }.bind(this));
+};
+
+
+function ExpressHandlebarsForRawPartials(config) {
+    this.constructor.super_.call(this, config);
+}
+util.inherits(ExpressHandlebarsForRawPartials, ExpressHandlebars);
+ExpressHandlebarsForRawPartials.prototype._precompileTemplate = ExpressHandlebarsForRawPartials.prototype._compileTemplate = function (template, options) {
+    return template;
 };
 
+
 /* exporting the same signature of express-handlebars */
 exports = module.exports  = exphbs;
 exports.create            = create;

tests/express/app.js

@@ -6,8 +6,13 @@ var express = require('express'),
 var routes = require('./routes/index');
 var routesyd = require('./routes/yd');
 var routesundefined = require('./routes/undefined');
+var routespartial = require('./routes/partial');
+var routeslooppartial = require('./routes/looppartial');
+
+app.expressSecureHandlebars = expressHbs.create({ partialsDir: __dirname + '/views/partials',
+                               extname:'hbs' });
+app.engine('hbs', app.expressSecureHandlebars.engine);
 
-app.engine('hbs', expressHbs({ extname:'hbs' }));
 app.set('views', path.join(__dirname, 'views'));
 app.set('view engine', 'hbs');
 
@@ -16,6 +21,8 @@ process.chdir(__dirname);
 app.use('/', routes);
 app.use('/yd', routesyd);
 app.use('/undefined', routesundefined);
+app.use('/partial', routespartial);
+app.use('/looppartial', routeslooppartial);
 app.get('/ok', function(req, res){
     res.status(200).send('ok');
 });

tests/express/routes/looppartial.js

@@ -0,0 +1,9 @@
+var express = require('express');
+var router = express.Router();
+router.get('/', function(req, res) {
+    var data = {
+    };
+    res.render('looppartial', data);
+});
+
+module.exports = router;

tests/express/routes/partial.js

@@ -0,0 +1,11 @@
+var express = require('express');
+var router = express.Router();
+router.get('/', function(req, res) {
+    var data = {
+        'exp': 'header',
+        'js':  'js',
+    };
+    res.render('partial', data);
+});
+
+module.exports = router;

tests/express/views/looppartial.hbs

@@ -0,0 +1 @@
+{{> l1 }}

tests/express/views/partial.hbs

@@ -0,0 +1,5 @@
+{{>header}}
+
+<script>
+{{>script}}
+</script>

tests/express/views/partials/header.hbs

@@ -0,0 +1,2 @@
+{{exp}}
+

tests/express/views/partials/l1.hbs

@@ -0,0 +1 @@
+{{> l2 }}

tests/express/views/partials/l2.hbs

@@ -0,0 +1 @@
+{{> l1 }}

tests/express/views/partials/script.hbs

@@ -0,0 +1 @@
+{{js}}

tests/integration/run-express-data-binding-spec.js

@@ -23,7 +23,6 @@ Authors: Nera Liu <[email protected]>
         it("Express Secure Handlebars server up test", function(done) {
             request(app)
                 .get('/ok')
-                .expect(200)
                 .end(function(err, res) {
                     expect(res.text).to.be.equal('ok');
                     expect(res.status).to.be.equal(200);
@@ -34,7 +33,6 @@ Authors: Nera Liu <[email protected]>
         it("Express Secure Handlebars data binding test", function(done) {
             request(app)
                 .get('/')
-                .expect(200)
                 .end(function(err, res) {
                     expect(res.text).to.be.equal('<h1>express secure handlebars</h1>\n');
                     expect(res.status).to.be.equal(200);
@@ -45,7 +43,6 @@ Authors: Nera Liu <[email protected]>
         it("Express Secure Handlebars data binding / yd filter test", function(done) {
             request(app)
                 .get('/yd')
-                .expect(200)
                 .end(function(err, res) {
                     expect(res.text).to.be.equal("<div>>&lt;'\"& </div>\n");
                     expect(res.status).to.be.equal(200);
@@ -56,13 +53,58 @@ Authors: Nera Liu <[email protected]>
         it("Express Secure Handlebars undefined data binding test", function(done) {
             request(app)
                 .get('/undefined')
-                .expect(200)
                 .end(function(err, res) {
                     expect(res.text).to.be.match(/<div><\/div>/);
                     expect(res.text).to.be.match(/<input id=\ufffd>/);
                     expect(res.status).to.be.equal(200);
                     done();
                 });
         });
+
+        it("Express Secure Handlebars partial test", function(done) {
+            request(app)
+                .get('/partial')
+                .end(function(err, res) {
+                    expect(res.text).to.be.match(/header/);
+                    expect(res.text).to.be.match(/js/);
+                    expect(res.status).to.be.equal(200);
+                    done();
+                });
+        });
+
+        it("Express Secure Handlebars partial cache test", function(done) {
+            var cacheTest = function() {
+                var esh = app.expressSecureHandlebars;
+                expect(esh.compilerOptions).to.be.ok();
+                expect(esh.compilerOptions.shbsPartialsCache).to.be.ok();
+                expect(esh.compilerOptions.shbsPartialsCache.raw['header']).to.equal('{{exp}}\n\n');
+                expect(esh.compilerOptions.shbsPartialsCache.raw['l1']).to.equal('{{> l2 }}\n');
+                expect(esh.compilerOptions.shbsPartialsCache.raw['l2']).to.equal('{{> l1 }}\n');
+                expect(esh.compilerOptions.shbsPartialsCache.raw['script']).to.equal('{{js}}\n');
+                expect(esh.compilerOptions.shbsPartialsCache.preprocessed['SJST/1/header']).to.equal('{{{yd exp}}}\n\n');
+                expect(esh.compilerOptions.shbsPartialsCache.preprocessed['SJST/6/script']).to.equal('{{{y js}}}\n');
+
+                // TODO: compiled is available only when options.cache is true
+                // expect(esh.compilerOptions.shbsPartialsCache.compiled['SJST/1/header']).to.be.ok();
+                // expect(esh.compilerOptions.shbsPartialsCache.compiled['SJST/6/script']).to.be.ok();
+
+                done();
+            }
+            request(app)
+                .get('/partial')
+                .end(function(err, res) {
+                    cacheTest();
+                });
+        });
+
+        // make sure it won't be infinite loop
+        it("Express Secure Handlebars loop partial test", function(done) {
+            request(app)
+                .get('/looppartial')
+                .end(function(err, res) {
+                    expect(res.status).to.be.equal(500);
+                    done();
+                });
+        });
     });
 }());

tests/unit/run-express-secure-handlebars.js

@@ -18,6 +18,8 @@ Authors: Nera Liu <[email protected]>
 
     describe("Express Secure Handlebars test suite", function() {
 
+        this.timeout(5000);
+
         it("same signature test", function() {
             // console.log(expressHandlebars);
             expect(typeof expressHandlebars).to.be.equal('function');
@@ -26,7 +28,6 @@ Authors: Nera Liu <[email protected]>
             expect(expressHandlebars.create).to.be.ok();
             expect(expressHandlebars.ExpressHandlebars).to.be.ok();
 
-
             // console.log(expressSecureHandlebars);
             expect(typeof expressSecureHandlebars).to.be.equal('function');
             expect(typeof expressSecureHandlebars.create).to.be.equal('function');
@@ -80,14 +81,24 @@ Authors: Nera Liu <[email protected]>
             expect(t1(data)).not.to.be.equal(t2(data));
         });
 
-        it("handlebars getTemplate test", function() {
-            var templateFile = path.resolve("views/yd.hbs");
+        it("handlebars render test", function(done) {
+
+            var filePath = path.resolve('../express/views/yd.hbs');
             var expSecureHbs = expressSecureHandlebars.create();
-            expSecureHbs.render(templateFile);
+
             expect(expSecureHbs.compilerOptions).to.be.ok();
-            expect(expSecureHbs.compilerOptions.processingFile).to.be.ok();
-            expect(expSecureHbs.compilerOptions.processingFile).to.be.match(/yd\.hbs/);
+            expect(expSecureHbs.compilerOptions.shbsPartialsCache).to.be.ok();
+
+            expSecureHbs.render(filePath, {input: '<script>alert(1)</script>'}).then(function(output){
+                if (output === '<div>&lt;script>alert(1)&lt;/script></div>\n' && 
+                    expSecureHbs.compilerOptions.processingFile === filePath) {
+                    done();
+                }
+
+            });
+
+
         });
-    });
 
+    });
 }());