Merge pull request #14244 from emberjs/deep-freeze

[BUGFIX beta] Ensure params and hash are frozen in debug builds.
emberjs · Sep 9, 2016 · be4fb63 · be4fb63
2 parents 111e354 + 04781c8
commit be4fb63
Show file tree

Hide file tree

Showing 6 changed files with 164 additions and 9 deletions.
diff --git a/ember-cli-build.js b/ember-cli-build.js
@@ -186,8 +186,8 @@ function babelConfigFor(environment) {
   if (isProduction) {
     includeDevHelpers = false;
     plugins.push(filterImports({
-      'ember-metal/debug': ['assert', 'debug', 'deprecate', 'info', 'runInDebug', 'warn', 'debugSeal'],
-      'ember-metal': ['assert', 'debug', 'deprecate', 'info', 'runInDebug', 'warn', 'debugSeal']
+      'ember-metal/debug': ['assert', 'debug', 'deprecate', 'info', 'runInDebug', 'warn', 'debugSeal', 'debugFreeze'],
+      'ember-metal': ['assert', 'debug', 'deprecate', 'info', 'runInDebug', 'warn', 'debugSeal', 'debugFreeze']
     }));
   }
 

diff --git a/packages/ember-debug/lib/index.js b/packages/ember-debug/lib/index.js
@@ -153,6 +153,10 @@ setDebugFunction('debugSeal', function debugSeal(obj) {
   Object.seal(obj);
 });
 
+setDebugFunction('debugFreeze', function debugFreeze(obj) {
+  Object.freeze(obj);
+});
+
 setDebugFunction('deprecate', _deprecate);
 
 setDebugFunction('warn', _warn);

diff --git a/packages/ember-glimmer/lib/utils/references.js b/packages/ember-glimmer/lib/utils/references.js
@@ -7,7 +7,8 @@ import {
   EmptyObject,
   meta as metaFor,
   watchKey,
-  isFeatureEnabled
+  isFeatureEnabled,
+  runInDebug
 } from 'ember-metal';
 import { CONSTANT_TAG, ConstReference, DirtyableTag, UpdatableTag, combine, isConst } from 'glimmer-reference';
 import { ConditionalReference as GlimmerConditionalReference, NULL_REFERENCE, UNDEFINED_REFERENCE } from 'glimmer-runtime';
@@ -276,7 +277,16 @@ export class SimpleHelperReference extends CachedReference {
   static create(helper, args) {
     if (isConst(args)) {
       let { positional, named } = args;
-      let result = helper(positional.value(), named.value());
+
+      let positionalValue = positional.value();
+      let namedValue = named.value();
+
+      runInDebug(() => {
+        Object.freeze(positionalValue);
+        Object.freeze(namedValue);
+      });
+
+      let result = helper(positionalValue, namedValue);
 
       if (result === null) {
         return NULL_REFERENCE;
@@ -302,7 +312,16 @@ export class SimpleHelperReference extends CachedReference {
 
   compute() {
     let { helper, args: { positional, named } } = this;
-    return helper(positional.value(), named.value());
+
+    let positionalValue = positional.value();
+    let namedValue = named.value();
+
+    runInDebug(() => {
+      Object.freeze(positionalValue);
+      Object.freeze(namedValue);
+    });
+
+    return helper(positionalValue, namedValue);
   }
 }
 
@@ -323,7 +342,16 @@ export class ClassBasedHelperReference extends CachedReference {
 
   compute() {
     let { instance, args: { positional, named } } = this;
-    return instance.compute(positional.value(), named.value());
+
+    let positionalValue = positional.value();
+    let namedValue = named.value();
+
+    runInDebug(() => {
+      Object.freeze(positionalValue);
+      Object.freeze(namedValue);
+    });
+
+    return instance.compute(positionalValue, namedValue);
   }
 }
 

diff --git a/packages/ember-glimmer/tests/integration/helpers/custom-helper-test.js b/packages/ember-glimmer/tests/integration/helpers/custom-helper-test.js
@@ -1,3 +1,4 @@
+/* globals EmberDev */
 import { RenderingTest, moduleFor } from '../../utils/test-case';
 import { makeBoundHelper } from '../../utils/helpers';
 import { runDestroy } from 'internal-test-helpers';
@@ -548,5 +549,120 @@ moduleFor('Helpers test: custom helpers', class extends RenderingTest {
 
     equal(destroyCount, 1, 'destroy is called after a view is destroyed');
   }
-
 });
+
+// these feature detects prevent errors in these tests
+// on platforms (*cough* IE9 *cough*) that do not
+// property support `Object.freeze`
+let pushingIntoFrozenArrayThrows = (() => {
+  let array = [];
+  Object.freeze(array);
+
+  try {
+    array.push('foo');
+
+    return false;
+  } catch(e) {
+    return true;
+  }
+})();
+
+let assigningExistingFrozenPropertyThrows = (() => {
+  let obj = { foo: 'asdf' };
+  Object.freeze(obj);
+
+  try {
+    obj.foo = 'derp';
+
+    return false;
+  } catch(e) {
+    return true;
+  }
+})();
+
+let addingPropertyToFrozenObjectThrows = (() => {
+  let obj = { foo: 'asdf' };
+  Object.freeze(obj);
+
+  try {
+    obj.bar = 'derp';
+
+    return false;
+  } catch(e) {
+    return true;
+  }
+})();
+
+if (!EmberDev.runningProdBuild && (
+  pushingIntoFrozenArrayThrows ||
+    assigningExistingFrozenPropertyThrows ||
+    addingPropertyToFrozenObjectThrows
+)) {
+  class HelperMutatingArgsTests extends RenderingTest {
+    buildCompute() {
+      return (params, hash) => {
+        if (pushingIntoFrozenArrayThrows) {
+          this.assert.throws(() => {
+            params.push('foo');
+
+            // cannot assert error message as it varies by platform
+          });
+        }
+
+        if (assigningExistingFrozenPropertyThrows) {
+          this.assert.throws(() => {
+            hash.foo = 'bar';
+
+            // cannot assert error message as it varies by platform
+          });
+        }
+
+        if (addingPropertyToFrozenObjectThrows) {
+          this.assert.throws(() => {
+            hash.someUnusedHashProperty = 'bar';
+
+            // cannot assert error message as it varies by platform
+          });
+        }
+      };
+    }
+
+    ['@test cannot mutate params - no positional specified / named specified']() {
+      this.render('{{test-helper foo=bar}}', { bar: 'derp' });
+    }
+
+    ['@test cannot mutate params - positional specified / no named specified']() {
+      this.render('{{test-helper bar}}', { bar: 'derp' });
+    }
+
+    ['@test cannot mutate params - positional specified / named specified']() {
+      this.render('{{test-helper bar foo=qux}}', { bar: 'derp', qux: 'baz' });
+    }
+
+    ['@test cannot mutate params - no positional specified / no named specified']() {
+      this.render('{{test-helper}}', { bar: 'derp', qux: 'baz' });
+    }
+  }
+
+  moduleFor('Helpers test: mutation triggers errors - class based helper', class extends HelperMutatingArgsTests {
+    constructor() {
+      super();
+
+      let compute = this.buildCompute();
+
+      this.registerHelper('test-helper', {
+        compute
+      });
+    }
+  });
+
+  moduleFor('Helpers test: mutation triggers errors - simple helper', class extends HelperMutatingArgsTests {
+    constructor() {
+      super();
+
+      let compute = this.buildCompute();
+
+      this.registerHelper('test-helper', compute);
+    }
+  });
+}
diff --git a/packages/ember-metal/lib/debug.js b/packages/ember-metal/lib/debug.js
@@ -6,7 +6,8 @@ export let debugFunctions = {
   deprecate() {},
   deprecateFunc(...args) { return args[args.length - 1]; },
   runInDebug() {},
-  debugSeal() {}
+  debugSeal() {},
+  debugFreeze() {}
 };
 
 export function getDebugFunction(name) {
@@ -48,3 +49,7 @@ export function runInDebug() {
 export function debugSeal() {
   return debugFunctions.debugSeal.apply(undefined, arguments);
 }
+
+export function debugFreeze() {
+  return debugFunctions.debugFreeze.apply(undefined, arguments);
+}
diff --git a/packages/ember-metal/lib/index.js b/packages/ember-metal/lib/index.js
@@ -21,7 +21,9 @@ export {
   deprecateFunc,
   runInDebug,
   setDebugFunction,
-  getDebugFunction
+  getDebugFunction,
+  debugSeal,
+  debugFreeze
 } from './debug';
 export {
   instrument,