fix($parse): allow use of locals in assignments

Fixes angular#4664

Author: jbedard

src/ng/directive/form.js

@@ -489,19 +489,19 @@ var formDirectiveFactory = function(isNgForm) {
                 alias = controller.$name;
 
             if (alias) {
-              setter(scope, alias, controller, alias);
+              setter(scope, null, alias, controller, alias);
               attr.$observe(attr.name ? 'name' : 'ngForm', function(newValue) {
                 if (alias === newValue) return;
-                setter(scope, alias, undefined, alias);
+                setter(scope, null, alias, undefined, alias);
                 alias = newValue;
-                setter(scope, alias, controller, alias);
+                setter(scope, null, alias, controller, alias);
                 parentFormCtrl.$$renameControl(controller, alias);
               });
             }
             formElement.on('$destroy', function() {
               parentFormCtrl.$removeControl(controller);
               if (alias) {
-                setter(scope, alias, undefined, alias);
+                setter(scope, null, alias, undefined, alias);
               }
               extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards
             });

src/ng/parse.js

@@ -663,8 +663,8 @@ Parser.prototype = {
     }, {
       assign: function(scope, value, locals) {
         var o = object(scope, locals);
-        if (!o) object.assign(scope, o = {});
-        return setter(o, field, value, expression);
+        if (!o) object.assign(scope, o = {}, locals);
+        return setter(o, null, field, value, expression);
       }
     });
   },
@@ -689,7 +689,7 @@ Parser.prototype = {
         var key = ensureSafeMemberName(indexFn(self, locals), expression);
         // prevent overwriting of Function.constructor which would break ensureSafeObject check
         var o = ensureSafeObject(obj(self, locals), expression);
-        if (!o) obj.assign(self, o = {});
+        if (!o) obj.assign(self, o = {}, locals);
         return o[key] = value;
       }
     });
@@ -799,18 +799,19 @@ Parser.prototype = {
 // Parser helper functions
 //////////////////////////////////////////////////
 
-function setter(obj, path, setValue, fullExp) {
+function setter(obj, locals, path, setValue, fullExp) {
   ensureSafeObject(obj, fullExp);
+  ensureSafeObject(locals, fullExp);
 
   var element = path.split('.'), key;
   for (var i = 0; element.length > 1; i++) {
     key = ensureSafeMemberName(element.shift(), fullExp);
-    var propertyObj = ensureSafeObject(obj[key], fullExp);
+    var propertyObj = (i === 0 && locals && locals[key]) || obj[key];
     if (!propertyObj) {
       propertyObj = {};
       obj[key] = propertyObj;
     }
-    obj = propertyObj;
+    obj = ensureSafeObject(propertyObj, fullExp);
   }
   key = ensureSafeMemberName(element.shift(), fullExp);
   ensureSafeObject(obj[key], fullExp);
@@ -937,8 +938,8 @@ function getterFn(path, options, fullExp) {
   }
 
   fn.sharedGetter = true;
-  fn.assign = function(self, value) {
-    return setter(self, path, value, path);
+  fn.assign = function(self, value, locals) {
+    return setter(self, locals, path, value, path);
   };
   getterFnCache[path] = fn;
   return fn;

test/ng/parseSpec.js

@@ -488,6 +488,62 @@ describe('parser', function() {
         expect(scope.b).toEqual(234);
       });
 
+      it('should allow use of locals in the left side of an assignment', inject(function($rootScope) {
+        $rootScope.a = {};
+        $rootScope.key = "value";
+        var localA = {};
+
+        //getterFn
+        $rootScope.$eval('a.value = 1', {a: localA});
+        expect(localA.value).toBe(1);
+
+        $rootScope.$eval('w.a.value = 2', {w: {a: localA}});
+        expect(localA.value).toBe(2);
+
+        //field access
+        $rootScope.$eval('(a).value = 3', {a: localA});
+        expect(localA.value).toBe(3);
+
+        $rootScope.$eval('{c: {b: a}}.c.b.value = 4', {a: localA});
+        expect(localA.value).toBe(4);
+
+        //object index
+        $rootScope.$eval('a[key] = 5', {a: localA});
+        expect(localA.value).toBe(5);
+
+        $rootScope.$eval('w.a[key] = 6', {w: {a: localA}});
+        expect(localA.value).toBe(6);
+
+        $rootScope.$eval('{c: {b: a}}.c.b[key] = 7', {a: localA});
+        expect(localA.value).toBe(7);
+
+        //Nothing should have touched the $rootScope.a
+        expect($rootScope.a.value).toBeUndefined();
+      }));
+
+      it('should allow use of locals in sub expressions of the left side of an assignment', inject(function($rootScope, $parse) {
+        delete $rootScope.x;
+        $rootScope.$eval('x[a][b] = true', {a: 'foo', b: 'bar'});
+        expect($rootScope.x.foo.bar).toBe(true);
+
+        delete $rootScope.x;
+        $rootScope.$eval('x[a][b] = true', {a: 'foo', b: 'bar'});
+        expect($rootScope.x.foo.bar).toBe(true);
+
+        delete $rootScope.x;
+        $rootScope.$eval('x[a].bar = true', {a: 'foo'});
+        expect($rootScope.x.foo.bar).toBe(true);
+      }));
+
+      it('should ignore locals beyond the root object of an assignment expression', inject(function($rootScope) {
+        var a = {};
+        var locals = {a: a};
+        $rootScope.b = {a: {value: 123}};
+        $rootScope.$eval('b.a.value = 1', locals);
+        expect(a.value).toBeUndefined();
+        expect($rootScope.b.a.value).toBe(1);
+      }));
+
         it('should evaluate assignments in ternary operator', function() {
           scope.$eval('a = 1 ? 2 : 3');
           expect(scope.a).toBe(2);
@@ -799,6 +855,12 @@ describe('parser', function() {
             }).toThrowMinErr(
                     '$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
                     'Expression: a.toString.constructor');
+
+            expect(function() {
+              scope.$eval("c.a = 1", {c: Function.prototype.constructor});
+            }).toThrowMinErr(
+                    '$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
+                    'Expression: c.a');
           });
 
           it('should disallow traversing the Function object in a setter: E02', function() {
@@ -933,6 +995,14 @@ describe('parser', function() {
                     '$parse', 'isecobj', 'Referencing Object in Angular expressions is disallowed! ' +
                     'Expression: foo["bar"]["keys"](foo)');
           });
+
+          it('should NOT allow access to Object constructor in assignment locals', function() {
+            expect(function() {
+              scope.$eval("O.constructor.a = 1", {O: Object});
+            }).toThrowMinErr(
+                    '$parse', 'isecobj', 'Referencing Object in Angular expressions is disallowed! ' +
+                    'Expression: O.constructor.a');
+          });
         });
 
         describe('Window and $element/node', function() {