From 33a3b46bc205f768f8edbc67241c68591fe3472c Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Mon, 16 Dec 2019 21:44:55 +0100 Subject: [PATCH 1/4] feat: access control to prototype properties via whitelist Disallow access to prototype properties and methods by default. Access to properties is always checked via `Object.prototype.hasOwnProperty.call(parent, propertyName)`. New runtime options: - **allowedProtoMethods**: a string-to-boolean map of property-names that are allowed if they are methods of the parent object. - **allowedProtoProperties**: a string-to-boolean map of property-names that are allowed if they are properties but not methods of the parent object. ```js const template = handlebars.compile('{{aString.trim}}') const result = template({ aString: ' abc ' }) // result is empty, because trim is defined at String prototype ``` ```js const template = handlebars.compile('{{aString.trim}}') const result = template({ aString: ' abc ' }, { allowedProtoMethods: { trim: true } }) // result = 'abc' ``` Implementation details: The method now "container.lookupProperty" handles the prototype-checks and the white-lists. It is used in - JavaScriptCompiler#nameLookup - The "lookup"-helper (passed to all helpers as "options.lookupProperty") - The "lookup" function at the container, which is used for recursive lookups in "compat" mode Compatibility: - **Old precompiled templates work with new runtimes**: The "options.lookupPropery"-function is passed to the helper by a wrapper, not by the compiled templated. - **New templates work with old runtimes**: The template contains a function that is used as fallback if the "lookupProperty"-function cannot be found at the container. However, the runtime-options "allowedProtoProperties" and "allowedProtoMethods" only work with the newest runtime. BREAKING CHANGE: - access to prototype properties is forbidden completely by default --- .../compiler/javascript-compiler.js | 54 +++--- lib/handlebars/helpers/lookup.js | 13 +- .../internal/createNewLookupObject.js | 11 ++ lib/handlebars/internal/wrapHelper.js | 8 + lib/handlebars/runtime.js | 56 +++++- spec/blocks.js | 1 + spec/helpers.js | 11 ++ spec/javascript-compiler.js | 25 +++ spec/regressions.js | 54 +++++- spec/security.js | 182 +++++++++++++++--- types/index.d.ts | 2 + types/test.ts | 10 + 12 files changed, 354 insertions(+), 73 deletions(-) create mode 100644 lib/handlebars/internal/createNewLookupObject.js create mode 100644 lib/handlebars/internal/wrapHelper.js diff --git a/lib/handlebars/compiler/javascript-compiler.js b/lib/handlebars/compiler/javascript-compiler.js index bcf9bb21c..63f90bdc2 100644 --- a/lib/handlebars/compiler/javascript-compiler.js +++ b/lib/handlebars/compiler/javascript-compiler.js @@ -2,7 +2,6 @@ import { COMPILER_REVISION, REVISION_CHANGES } from '../base'; import Exception from '../exception'; import { isArray } from '../utils'; import CodeGen from './code-gen'; -import { dangerousPropertyRegex } from '../helpers/lookup'; function Literal(value) { this.value = value; @@ -13,27 +12,8 @@ function JavaScriptCompiler() {} JavaScriptCompiler.prototype = { // PUBLIC API: You can override these methods in a subclass to provide // alternative compiled forms for name lookup and buffering semantics - nameLookup: function(parent, name /* , type*/) { - if (dangerousPropertyRegex.test(name)) { - const isEnumerable = [ - this.aliasable('container.propertyIsEnumerable'), - '.call(', - parent, - ',', - JSON.stringify(name), - ')' - ]; - return ['(', isEnumerable, '?', _actualLookup(), ' : undefined)']; - } - return _actualLookup(); - - function _actualLookup() { - if (JavaScriptCompiler.isValidJavaScriptVariableName(name)) { - return [parent, '.', name]; - } else { - return [parent, '[', JSON.stringify(name), ']']; - } - } + nameLookup: function(parent, name /*, type */) { + return this.internalNameLookup(parent, name); }, depthedLookup: function(name) { return [this.aliasable('container.lookup'), '(depths, "', name, '")']; @@ -69,6 +49,12 @@ JavaScriptCompiler.prototype = { return this.quotedString(''); }, // END PUBLIC API + internalNameLookup: function(parent, name) { + this.lookupPropertyFunctionIsUsed = true; + return ['lookupProperty(', parent, ',', JSON.stringify(name), ')']; + }, + + lookupPropertyFunctionIsUsed: false, compile: function(environment, options, context, asObject) { this.environment = environment; @@ -131,7 +117,11 @@ JavaScriptCompiler.prototype = { if (!this.decorators.isEmpty()) { this.useDecorators = true; - this.decorators.prepend('var decorators = container.decorators;\n'); + this.decorators.prepend([ + 'var decorators = container.decorators, ', + this.lookupPropertyFunctionVarDeclaration(), + ';\n' + ]); this.decorators.push('return fn;'); if (asObject) { @@ -248,6 +238,10 @@ JavaScriptCompiler.prototype = { } }); + if (this.lookupPropertyFunctionIsUsed) { + varDeclarations += ', ' + this.lookupPropertyFunctionVarDeclaration(); + } + let params = ['container', 'depth0', 'helpers', 'partials', 'data']; if (this.useBlockParams || this.useDepths) { @@ -335,6 +329,17 @@ JavaScriptCompiler.prototype = { return this.source.merge(); }, + lookupPropertyFunctionVarDeclaration: function() { + return ` + lookupProperty = container.lookupProperty || function(parent, propertyName) { + if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { + return parent[propertyName]; + } + return undefined + } + `.trim(); + }, + // [blockValue] // // On stack, before: hash, inverse, program, value @@ -1241,6 +1246,9 @@ JavaScriptCompiler.prototype = { } })(); +/** + * @deprecated May be removed in the next major version + */ JavaScriptCompiler.isValidJavaScriptVariableName = function(name) { return ( !JavaScriptCompiler.RESERVED_WORDS[name] && diff --git a/lib/handlebars/helpers/lookup.js b/lib/handlebars/helpers/lookup.js index 66de538b6..5620aba32 100644 --- a/lib/handlebars/helpers/lookup.js +++ b/lib/handlebars/helpers/lookup.js @@ -1,16 +1,9 @@ -export const dangerousPropertyRegex = /^(constructor|__defineGetter__|__defineSetter__|__lookupGetter__|__proto__)$/; - export default function(instance) { - instance.registerHelper('lookup', function(obj, field) { + instance.registerHelper('lookup', function(obj, field, options) { if (!obj) { + // Note for 5.0: Change to "obj == null" in 5.0 return obj; } - if ( - dangerousPropertyRegex.test(String(field)) && - !Object.prototype.propertyIsEnumerable.call(obj, field) - ) { - return undefined; - } - return obj[field]; + return options.lookupProperty(obj, field); }); } diff --git a/lib/handlebars/internal/createNewLookupObject.js b/lib/handlebars/internal/createNewLookupObject.js new file mode 100644 index 000000000..256f1eca1 --- /dev/null +++ b/lib/handlebars/internal/createNewLookupObject.js @@ -0,0 +1,11 @@ +import { extend } from '../utils'; + +/** + * Create a new object with "null"-prototype to avoid truthy results on prototype properties. + * The resulting object can be used with "object[property]" to check if a property exists + * @param {...object} sources a varargs parameter of source objects that will be merged + * @returns {object} + */ +export function createNewLookupObject(...sources) { + return extend(Object.create(null), ...sources); +} diff --git a/lib/handlebars/internal/wrapHelper.js b/lib/handlebars/internal/wrapHelper.js new file mode 100644 index 000000000..0010eb8c2 --- /dev/null +++ b/lib/handlebars/internal/wrapHelper.js @@ -0,0 +1,8 @@ +export function wrapHelper(helper, transformOptionsFn) { + let wrapper = function(/* dynamic arguments */) { + const options = arguments[arguments.length - 1]; + arguments[arguments.length - 1] = transformOptionsFn(options); + return helper.apply(this, arguments); + }; + return wrapper; +} diff --git a/lib/handlebars/runtime.js b/lib/handlebars/runtime.js index 4d67895af..7d63e87b8 100644 --- a/lib/handlebars/runtime.js +++ b/lib/handlebars/runtime.js @@ -7,6 +7,8 @@ import { REVISION_CHANGES } from './base'; import { moveHelperToHooks } from './helpers'; +import { wrapHelper } from './internal/wrapHelper'; +import { createNewLookupObject } from './internal/createNewLookupObject'; export function checkRevision(compilerInfo) { const compilerRevision = (compilerInfo && compilerInfo[0]) || 1, @@ -69,13 +71,17 @@ export function template(templateSpec, env) { } partial = env.VM.resolvePartial.call(this, partial, context, options); - let optionsWithHooks = Utils.extend({}, options, { hooks: this.hooks }); + let extendedOptions = Utils.extend({}, options, { + hooks: this.hooks, + allowedProtoMethods: this.allowedProtoMethods, + allowedProtoProperties: this.allowedProtoProperties + }); let result = env.VM.invokePartial.call( this, partial, context, - optionsWithHooks + extendedOptions ); if (result == null && env.compile) { @@ -84,7 +90,7 @@ export function template(templateSpec, env) { templateSpec.compilerOptions, env ); - result = options.partials[options.name](context, optionsWithHooks); + result = options.partials[options.name](context, extendedOptions); } if (result != null) { if (options.indent) { @@ -118,10 +124,26 @@ export function template(templateSpec, env) { } return obj[name]; }, + lookupProperty: function(parent, propertyName) { + let result = parent[propertyName]; + if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { + return result; + } + const whitelist = + typeof result === 'function' + ? container.allowedProtoMethods + : container.allowedProtoProperties; + + if (whitelist[propertyName] === true) { + return result; + } + return undefined; + }, lookup: function(depths, name) { const len = depths.length; for (let i = 0; i < len; i++) { - if (depths[i] && depths[i][name] != null) { + let result = depths[i] && container.lookupProperty(depths[i], name); + if (result != null) { return depths[i][name]; } } @@ -229,7 +251,9 @@ export function template(templateSpec, env) { ret._setup = function(options) { if (!options.partial) { - container.helpers = Utils.extend({}, env.helpers, options.helpers); + let mergedHelpers = Utils.extend({}, env.helpers, options.helpers); + wrapHelpersToPassLookupProperty(mergedHelpers, container); + container.helpers = mergedHelpers; if (templateSpec.usePartial) { // Use mergeIfNeeded here to prevent compiling global partials multiple times @@ -247,6 +271,12 @@ export function template(templateSpec, env) { } container.hooks = {}; + container.allowedProtoProperties = createNewLookupObject( + options.allowedProtoProperties + ); + container.allowedProtoMethods = createNewLookupObject( + options.allowedProtoMethods + ); let keepHelperInHelpers = options.allowCallsToHelperMissing || @@ -254,6 +284,8 @@ export function template(templateSpec, env) { moveHelperToHooks(container, 'helperMissing', keepHelperInHelpers); moveHelperToHooks(container, 'blockHelperMissing', keepHelperInHelpers); } else { + container.allowedProtoProperties = options.allowedProtoProperties; + container.allowedProtoMethods = options.allowedProtoMethods; container.helpers = options.helpers; container.partials = options.partials; container.decorators = options.decorators; @@ -405,3 +437,17 @@ function executeDecorators(fn, prog, container, depths, data, blockParams) { } return prog; } + +function wrapHelpersToPassLookupProperty(mergedHelpers, container) { + Object.keys(mergedHelpers).forEach(helperName => { + let helper = mergedHelpers[helperName]; + mergedHelpers[helperName] = passLookupPropertyOption(helper, container); + }); +} + +function passLookupPropertyOption(helper, container) { + const lookupProperty = container.lookupProperty; + return wrapHelper(helper, options => { + return Utils.extend({ lookupProperty }, options); + }); +} diff --git a/spec/blocks.js b/spec/blocks.js index 1ef2d1be2..ad534aaba 100644 --- a/spec/blocks.js +++ b/spec/blocks.js @@ -275,6 +275,7 @@ describe('blocks', function() { 'Goodbye cruel OMG!' ); }); + it('block with deep recursive pathed lookup', function() { var string = '{{#outer}}Goodbye {{#inner}}cruel {{omg.yes}}{{/inner}}{{/outer}}'; diff --git a/spec/helpers.js b/spec/helpers.js index 9d4bc202e..60140a096 100644 --- a/spec/helpers.js +++ b/spec/helpers.js @@ -1328,4 +1328,15 @@ describe('helpers', function() { ); }); }); + + describe('the lookupProperty-option', function() { + it('should be passed to custom helpers', function() { + expectTemplate('{{testHelper}}') + .withHelper('testHelper', function testHelper(options) { + return options.lookupProperty(this, 'testProperty'); + }) + .withInput({ testProperty: 'abc' }) + .toCompileTo('abc'); + }); + }); }); diff --git a/spec/javascript-compiler.js b/spec/javascript-compiler.js index 21b8e2a0a..e97cbb083 100644 --- a/spec/javascript-compiler.js +++ b/spec/javascript-compiler.js @@ -81,4 +81,29 @@ describe('javascript-compiler api', function() { shouldCompileTo('{{foo}}', { foo: 'food' }, 'food_foo'); }); }); + + describe('#isValidJavaScriptVariableName', function() { + // It is there and accessible and could be used by someone. That's why we don't remove it + // it 4.x. But if we keep it, we add a test + // This test should not encourage you to use the function. It is not needed any more + // and might be removed in 5.0 + ['test', 'abc123', 'abc_123'].forEach(function(validVariableName) { + it("should return true for '" + validVariableName + "'", function() { + expect( + handlebarsEnv.JavaScriptCompiler.isValidJavaScriptVariableName( + validVariableName + ) + ).to.be.true(); + }); + }); + [('123test', 'abc()', 'abc.cde')].forEach(function(invalidVariableName) { + it("should return true for '" + invalidVariableName + "'", function() { + expect( + handlebarsEnv.JavaScriptCompiler.isValidJavaScriptVariableName( + invalidVariableName + ) + ).to.be.false(); + }); + }); + }); }); diff --git a/spec/regressions.js b/spec/regressions.js index 821c48dc2..3a84dd7da 100644 --- a/spec/regressions.js +++ b/spec/regressions.js @@ -390,7 +390,7 @@ describe('Regressions', function() { it('should compile and execute templates', function() { var newHandlebarsInstance = Handlebars.create(); - registerTemplate(newHandlebarsInstance); + registerTemplate(newHandlebarsInstance, compiledTemplateVersion7()); newHandlebarsInstance.registerHelper('loud', function(value) { return value.toUpperCase(); }); @@ -405,7 +405,7 @@ describe('Regressions', function() { shouldThrow( function() { - registerTemplate(newHandlebarsInstance); + registerTemplate(newHandlebarsInstance, compiledTemplateVersion7()); newHandlebarsInstance.templates['test.hbs']({}); }, Handlebars.Exception, @@ -413,11 +413,31 @@ describe('Regressions', function() { ); }); - // This is a only slightly modified precompiled templated from compiled with 4.2.1 - function registerTemplate(Handlebars) { + it('should pass "options.lookupProperty" to "lookup"-helper, even with old templates', function() { + var newHandlebarsInstance = Handlebars.create(); + registerTemplate( + newHandlebarsInstance, + compiledTemplateVersion7_usingLookupHelper() + ); + + newHandlebarsInstance.templates['test.hbs']({}); + + expect( + newHandlebarsInstance.templates['test.hbs']({ + property: 'a', + test: { a: 'b' } + }) + ).to.equal('b'); + }); + + function registerTemplate(Handlebars, compileTemplate) { var template = Handlebars.template, templates = (Handlebars.templates = Handlebars.templates || {}); - templates['test.hbs'] = template({ + templates['test.hbs'] = template(compileTemplate); + } + + function compiledTemplateVersion7() { + return { compiler: [7, '>= 4.0.0'], main: function(container, depth0, helpers, partials, data) { return ( @@ -435,7 +455,29 @@ describe('Regressions', function() { ); }, useData: true - }); + }; + } + + function compiledTemplateVersion7_usingLookupHelper() { + // This is the compiled version of "{{lookup test property}}" + return { + compiler: [7, '>= 4.0.0'], + main: function(container, depth0, helpers, partials, data) { + return container.escapeExpression( + helpers.lookup.call( + depth0 != null ? depth0 : container.nullContext || {}, + depth0 != null ? depth0.test : depth0, + depth0 != null ? depth0.property : depth0, + { + name: 'lookup', + hash: {}, + data: data + } + ) + ); + }, + useData: true + }; } }); diff --git a/spec/security.js b/spec/security.js index ed0189281..7bf9d89a0 100644 --- a/spec/security.js +++ b/spec/security.js @@ -19,7 +19,7 @@ describe('security issues', function() { .toCompileTo(''); }); - it('should allow the "constructor" property to be accessed if it is enumerable', function() { + it('should allow the "constructor" property to be accessed if it is an "ownProperty"', function() { shouldCompileTo( '{{constructor.name}}', { @@ -40,7 +40,7 @@ describe('security issues', function() { ); }); - it('should allow the "constructor" property to be accessed if it is enumerable', function() { + it('should allow the "constructor" property to be accessed if it is an "own property"', function() { shouldCompileTo( '{{lookup (lookup this "constructor") "name"}}', { @@ -51,27 +51,6 @@ describe('security issues', function() { 'here we go' ); }); - - it('should allow prototype properties that are not constructors', function() { - function TestClass() {} - - Object.defineProperty(TestClass.prototype, 'abc', { - get: function() { - return 'xyz'; - } - }); - - shouldCompileTo( - '{{#with this as |obj|}}{{obj.abc}}{{/with}}', - new TestClass(), - 'xyz' - ); - shouldCompileTo( - '{{#with this as |obj|}}{{lookup obj "abc"}}{{/with}}', - new TestClass(), - 'xyz' - ); - }); }); describe('GH-1558: Prevent explicit call of helperMissing-helpers', function() { @@ -171,38 +150,183 @@ describe('security issues', function() { }); describe('GH-1595', function() { - it('properties, that are required to be enumerable', function() { + it('properties, that are required to be own properties', function() { expectTemplate('{{constructor}}') .withInput({}) .toCompileTo(''); + expectTemplate('{{__defineGetter__}}') .withInput({}) .toCompileTo(''); + expectTemplate('{{__defineSetter__}}') .withInput({}) .toCompileTo(''); + expectTemplate('{{__lookupGetter__}}') .withInput({}) .toCompileTo(''); + expectTemplate('{{__proto__}}') .withInput({}) .toCompileTo(''); - expectTemplate('{{lookup "constructor"}}') + expectTemplate('{{lookup this "constructor"}}') .withInput({}) .toCompileTo(''); - expectTemplate('{{lookup "__defineGetter__"}}') + + expectTemplate('{{lookup this "__defineGetter__"}}') .withInput({}) .toCompileTo(''); - expectTemplate('{{lookup "__defineSetter__"}}') + + expectTemplate('{{lookup this "__defineSetter__"}}') .withInput({}) .toCompileTo(''); - expectTemplate('{{lookup "__lookupGetter__"}}') + + expectTemplate('{{lookup this "__lookupGetter__"}}') .withInput({}) .toCompileTo(''); - expectTemplate('{{lookup "__proto__"}}') + + expectTemplate('{{lookup this "__proto__"}}') .withInput({}) .toCompileTo(''); }); + + describe('GH-1631: disallow access to prototype functions', function() { + function TestClass() {} + + TestClass.prototype.aProperty = 'propertyValue'; + TestClass.prototype.aMethod = function() { + return 'returnValue'; + }; + + describe('control access to prototype methods via "allowedProtoMethods"', function() { + it('should be prohibited by default', function() { + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .toCompileTo(''); + }); + + it('can be allowed', function() { + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withRuntimeOptions({ + allowedProtoMethods: { + aMethod: true + } + }) + .toCompileTo('returnValue'); + }); + + it('should be prohibited by default (in "compat" mode)', function() { + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withCompileOptions({ compat: true }) + .toCompileTo(''); + }); + + it('can be allowed (in "compat" mode)', function() { + expectTemplate('{{aMethod}}') + .withInput(new TestClass()) + .withCompileOptions({ compat: true }) + .withRuntimeOptions({ + allowedProtoMethods: { + aMethod: true + } + }) + .toCompileTo('returnValue'); + }); + + it('should cause the recursive lookup by default (in "compat" mode)', function() { + expectTemplate('{{#aString}}{{trim}}{{/aString}}') + .withInput({ aString: ' abc ', trim: 'trim' }) + .withCompileOptions({ compat: true }) + .toCompileTo('trim'); + }); + + it('should not cause the recursive lookup if allowed through options(in "compat" mode)', function() { + expectTemplate('{{#aString}}{{trim}}{{/aString}}') + .withInput({ aString: ' abc ', trim: 'trim' }) + .withCompileOptions({ compat: true }) + .withRuntimeOptions({ + allowedProtoMethods: { + trim: true + } + }) + .toCompileTo('abc'); + }); + }); + + describe('control access to prototype non-methods via "allowedProtoProperties"', function() { + it('should be prohibited by default', function() { + expectTemplate('{{aProperty}}') + .withInput(new TestClass()) + .toCompileTo(''); + }); + + it('can be turned on', function() { + expectTemplate('{{aProperty}}') + .withInput(new TestClass()) + .withRuntimeOptions({ + allowedProtoProperties: { + aProperty: true + } + }) + .toCompileTo('propertyValue'); + }); + + it('should be prohibited by default (in "compat" mode)', function() { + expectTemplate('{{aProperty}}') + .withInput(new TestClass()) + .withCompileOptions({ compat: true }) + .toCompileTo(''); + }); + + it('can be turned on (in "compat" mode)', function() { + expectTemplate('{{aProperty}}') + .withInput(new TestClass()) + .withCompileOptions({ compat: true }) + .withRuntimeOptions({ + allowedProtoProperties: { + aProperty: true + } + }) + .toCompileTo('propertyValue'); + }); + }); + + describe('compatibility with old runtimes, that do not provide the function "container.lookupProperty"', function() { + beforeEach(function simulateRuntimeWithoutLookupProperty() { + var oldTemplateMethod = handlebarsEnv.template; + sinon.replace(handlebarsEnv, 'template', function(templateSpec) { + templateSpec.main = wrapToAdjustContainer(templateSpec.main); + return oldTemplateMethod.call(this, templateSpec); + }); + }); + + afterEach(function() { + sinon.restore(); + }); + + it('should work with simple properties', function() { + expectTemplate('{{aProperty}}') + .withInput({ aProperty: 'propertyValue' }) + .toCompileTo('propertyValue'); + }); + + it('should work with Array.prototype.length', function() { + expectTemplate('{{anArray.length}}') + .withInput({ anArray: ['a', 'b', 'c'] }) + .toCompileTo('3'); + }); + }); + }); }); }); + +function wrapToAdjustContainer(precompiledTemplateFunction) { + return function templateFunctionWrapper(container /*, more args */) { + delete container.lookupProperty; + return precompiledTemplateFunction.apply(this, arguments); + }; +} diff --git a/types/index.d.ts b/types/index.d.ts index bb0656a51..606741f5a 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -30,6 +30,8 @@ declare namespace Handlebars { data?: any; blockParams?: any[]; allowCallsToHelperMissing?: boolean; + allowedProtoProperties?: { [name: string]: boolean } + allowedProtoMethods?: { [name: string]: boolean } } export interface HelperOptions { diff --git a/types/test.ts b/types/test.ts index fa9832683..b081ae4fb 100644 --- a/types/test.ts +++ b/types/test.ts @@ -240,3 +240,13 @@ function testExceptionWithNodeTypings() { let fileName: string = exception.fileName; let stack: string | undefined = exception.stack; } + +function testProtoPropertyControlOptions() { + Handlebars.compile('test')( + {}, + { + allowedProtoMethods: { allowedMethod: true, forbiddenMethod: false }, + allowedProtoProperties: { allowedProperty: true, forbiddenProperty: false } + } + ); +} From 461b83746687d725e25f1b43abe6479daeb0c07a Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Wed, 1 Jan 2020 21:21:24 +0100 Subject: [PATCH 2/4] test: show diff when test:bin fails --- package-lock.json | 17 +++++++++++++++++ package.json | 3 ++- tasks/test-bin.js | 13 ++++++++----- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5d119122f..2223f0b86 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1394,6 +1394,23 @@ "type-detect": "^4.0.5" } }, + "chai-diff": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/chai-diff/-/chai-diff-1.0.1.tgz", + "integrity": "sha1-bGaJRwDYDNkDUKtORANiXU9TocE=", + "dev": true, + "requires": { + "diff": "^2.2.1" + }, + "dependencies": { + "diff": { + "version": "2.2.3", + "resolved": "https://registry.npmjs.org/diff/-/diff-2.2.3.tgz", + "integrity": "sha1-YOr9DSjukG5Oj/ClLBIpUhAzv5k=", + "dev": true + } + } + }, "chalk": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", diff --git a/package.json b/package.json index 19f096a5a..c164ea9c2 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "babel-runtime": "^5.1.10", "benchmark": "~1.0", "chai": "^4.2.0", + "chai-diff": "^1.0.1", "concurrently": "^5.0.0", "dirty-chai": "^2.0.1", "dtslint": "^0.5.5", @@ -84,7 +85,7 @@ "scripts": { "format": "prettier --write '**/*.js' && eslint --fix .", "check-format": "prettier --check '**/*.js'", - "lint": "eslint --max-warnings 0 . ", + "lint": "eslint --max-warnings 0 .", "dtslint": "dtslint types", "test": "grunt", "extensive-tests-and-publish-to-aws": "npx mocha tasks/task-tests/ && grunt --stack extensive-tests-and-publish-to-aws", diff --git a/tasks/test-bin.js b/tasks/test-bin.js index 8873c9f01..077e6d4f3 100644 --- a/tasks/test-bin.js +++ b/tasks/test-bin.js @@ -1,7 +1,10 @@ -const childProcess = require('child_process'), - fs = require('fs'), - os = require('os'), - expect = require('chai').expect; +const childProcess = require('child_process'); +const fs = require('fs'); +const os = require('os'); + +const chai = require('chai'); +chai.use(require('chai-diff')); +const expect = chai.expect; module.exports = function(grunt) { grunt.registerTask('test:bin', function() { @@ -18,7 +21,7 @@ module.exports = function(grunt) { const normalizedOutput = normalizeCrlf(stdout); const normalizedExpectedOutput = normalizeCrlf(expectedOutput); - expect(normalizedOutput).to.equal(normalizedExpectedOutput); + expect(normalizedOutput).not.to.be.differentFrom(normalizedExpectedOutput); }); }; From e0b349c8d2f9a9d3766118f578705d0d92888851 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Thu, 2 Jan 2020 15:47:09 +0100 Subject: [PATCH 3/4] test: add path to nodeJs when running test:bin - this allows the test to be run in a debugger without the complete PATH --- tasks/test-bin.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tasks/test-bin.js b/tasks/test-bin.js index 077e6d4f3..3a9c43095 100644 --- a/tasks/test-bin.js +++ b/tasks/test-bin.js @@ -1,6 +1,7 @@ const childProcess = require('child_process'); const fs = require('fs'); const os = require('os'); +const path = require('path'); const chai = require('chai'); chai.use(require('chai-diff')); @@ -37,7 +38,13 @@ function executeBinHandlebars(...args) { } function execFilesSyncUtf8(command, args) { - return childProcess.execFileSync(command, args, { encoding: 'utf-8' }); + const env = process.env; + env.PATH = addPathToNodeJs(env.PATH); + return childProcess.execFileSync(command, args, { encoding: 'utf-8', env }); +} + +function addPathToNodeJs(pathEnvironment) { + return path.dirname(process.argv0) + path.delimiter + pathEnvironment; } function normalizeCrlf(string) { From 26489216bef573799e09818f92ce4652d7b65654 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Thu, 2 Jan 2020 15:49:41 +0100 Subject: [PATCH 4/4] refactor: fix typo in private test method --- spec/env/common.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/env/common.js b/spec/env/common.js index 64451fee4..0ecebcf45 100644 --- a/spec/env/common.js +++ b/spec/env/common.js @@ -167,7 +167,7 @@ HandlebarsTestBench.prototype.withMessage = function(message) { }; HandlebarsTestBench.prototype.toCompileTo = function(expectedOutputAsString) { - expect(this._compileAndExeute()).to.equal(expectedOutputAsString); + expect(this._compileAndExecute()).to.equal(expectedOutputAsString); }; // see chai "to.throw" (https://www.chaijs.com/api/bdd/#method_throw) @@ -178,11 +178,11 @@ HandlebarsTestBench.prototype.toThrow = function( ) { var self = this; expect(function() { - self._compileAndExeute(); + self._compileAndExecute(); }).to.throw(errorLike, errMsgMatcher, msg); }; -HandlebarsTestBench.prototype._compileAndExeute = function() { +HandlebarsTestBench.prototype._compileAndExecute = function() { var compile = Object.keys(this.partials).length > 0 ? CompilerContext.compileWithPartial