fix: log error for illegal property access only once per property

handlebars-lang · Jan 12, 2020 · 3c1e252 · 3c1e252
1 parent 0d5c807
commit 3c1e252
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 7 deletions.
diff --git a/lib/handlebars/base.js b/lib/handlebars/base.js
@@ -3,6 +3,7 @@ import Exception from './exception';
 import { registerDefaultHelpers } from './helpers';
 import { registerDefaultDecorators } from './decorators';
 import logger from './logger';
+import { resetLoggedProperties } from './internal/proto-access';
 
 export const VERSION = '4.7.0';
 export const COMPILER_REVISION = 8;
@@ -78,6 +79,13 @@ HandlebarsEnvironment.prototype = {
   },
   unregisterDecorator: function(name) {
     delete this.decorators[name];
+  },
+  /**
+   * Reset the memory of illegal property accesses that have already been logged.
+   * @deprecated should only be used in handlebars test-cases
+   */
+  resetLoggedPropertyAccesses() {
+    resetLoggedProperties();
   }
 };
 

diff --git a/lib/handlebars/internal/proto-access.js b/lib/handlebars/internal/proto-access.js
@@ -1,6 +1,8 @@
 import { createNewLookupObject } from './create-new-lookup-object';
 import * as logger from '../logger';
 
+const loggedProperties = Object.create(null);
+
 export function createProtoAccessControl(runtimeOptions) {
   let defaultMethodWhiteList = Object.create(null);
   defaultMethodWhiteList['constructor'] = false;
@@ -45,12 +47,24 @@ function checkWhiteList(protoAccessControlForType, propertyName) {
   if (protoAccessControlForType.defaultValue !== undefined) {
     return protoAccessControlForType.defaultValue;
   }
-  // eslint-disable-next-line no-console
-  logger.log(
-    'error',
-    `Handlebars: Access has been denied to resolve the property "${propertyName}" because it is not an "own property" of its parent.\n` +
-      `You can add a runtime option to disable the check or this warning:\n` +
-      `See http://localhost:8080/api-reference/runtime-options.html#options-to-control-prototype-access for details`
-  );
+  logUnexpecedPropertyAccessOnce(propertyName);
   return false;
 }
+
+function logUnexpecedPropertyAccessOnce(propertyName) {
+  if (loggedProperties[propertyName] !== true) {
+    loggedProperties[propertyName] = true;
+    logger.log(
+      'error',
+      `Handlebars: Access has been denied to resolve the property "${propertyName}" because it is not an "own property" of its parent.\n` +
+        `You can add a runtime option to disable the check or this warning:\n` +
+        `See http://localhost:8080/api-reference/runtime-options.html#options-to-control-prototype-access for details`
+    );
+  }
+}
+
+export function resetLoggedProperties() {
+  Object.keys(loggedProperties).forEach(propertyName => {
+    delete loggedProperties[propertyName];
+  });
+}
diff --git a/spec/security.js b/spec/security.js
@@ -190,6 +190,10 @@ describe('security issues', function() {
       return 'returnValue';
     };
 
+    beforeEach(function() {
+      handlebarsEnv.resetLoggedPropertyAccesses();
+    });
+
     afterEach(function() {
       sinon.restore();
     });
@@ -214,6 +218,23 @@ describe('security issues', function() {
           expect(spy.args[0][0]).to.match(/Handlebars: Access has been denied/);
         });
 
+        it('should only log the warning once', function() {
+          var spy = sinon.spy(console, 'error');
+
+          expectTemplate('{{aMethod}}')
+            .withInput(new TestClass())
+            .withCompileOptions(compileOptions)
+            .toCompileTo('');
+
+          expectTemplate('{{aMethod}}')
+            .withInput(new TestClass())
+            .withCompileOptions(compileOptions)
+            .toCompileTo('');
+
+          expect(spy.calledOnce).to.be.true();
+          expect(spy.args[0][0]).to.match(/Handlebars: Access has been denied/);
+        });
+
         it('can be allowed, which disables the warning', function() {
           var spy = sinon.spy(console, 'error');