Skip to content

Commit d0e87f3

Browse files
committed
fix($parse): allow assignment on objects in locals
Fixes angular#4664
1 parent d21dff2 commit d0e87f3

File tree

3 files changed

+43
-10
lines changed

3 files changed

+43
-10
lines changed

src/ng/directive/form.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -489,19 +489,19 @@ var formDirectiveFactory = function(isNgForm) {
489489
alias = controller.$name;
490490

491491
if (alias) {
492-
setter(scope, alias, controller, alias);
492+
setter(scope, null, alias, controller, alias);
493493
attr.$observe(attr.name ? 'name' : 'ngForm', function(newValue) {
494494
if (alias === newValue) return;
495-
setter(scope, alias, undefined, alias);
495+
setter(scope, null, alias, undefined, alias);
496496
alias = newValue;
497-
setter(scope, alias, controller, alias);
497+
setter(scope, null, alias, controller, alias);
498498
parentFormCtrl.$$renameControl(controller, alias);
499499
});
500500
}
501501
formElement.on('$destroy', function() {
502502
parentFormCtrl.$removeControl(controller);
503503
if (alias) {
504-
setter(scope, alias, undefined, alias);
504+
setter(scope, null, alias, undefined, alias);
505505
}
506506
extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards
507507
});

src/ng/parse.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ Parser.prototype = {
664664
assign: function(scope, value, locals) {
665665
var o = object(scope, locals);
666666
if (!o) object.assign(scope, o = {});
667-
return setter(o, field, value, expression);
667+
return setter(o, null, field, value, expression);
668668
}
669669
});
670670
},
@@ -799,18 +799,19 @@ Parser.prototype = {
799799
// Parser helper functions
800800
//////////////////////////////////////////////////
801801

802-
function setter(obj, path, setValue, fullExp) {
802+
function setter(obj, locals, path, setValue, fullExp) {
803803
ensureSafeObject(obj, fullExp);
804+
ensureSafeObject(locals, fullExp);
804805

805806
var element = path.split('.'), key;
806807
for (var i = 0; element.length > 1; i++) {
807808
key = ensureSafeMemberName(element.shift(), fullExp);
808-
var propertyObj = ensureSafeObject(obj[key], fullExp);
809+
var propertyObj = (i === 0 && locals && locals[key]) || obj[key];
809810
if (!propertyObj) {
810811
propertyObj = {};
811812
obj[key] = propertyObj;
812813
}
813-
obj = propertyObj;
814+
obj = ensureSafeObject(propertyObj, fullExp);
814815
}
815816
key = ensureSafeMemberName(element.shift(), fullExp);
816817
ensureSafeObject(obj[key], fullExp);
@@ -937,8 +938,8 @@ function getterFn(path, options, fullExp) {
937938
}
938939

939940
fn.sharedGetter = true;
940-
fn.assign = function(self, value) {
941-
return setter(self, path, value, path);
941+
fn.assign = function(self, value, locals) {
942+
return setter(self, locals, path, value, path);
942943
};
943944
getterFnCache[path] = fn;
944945
return fn;

test/ng/parseSpec.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,24 @@ describe('parser', function() {
488488
expect(scope.b).toEqual(234);
489489
});
490490

491+
it('should allow passing object as locals to the left part of an assignment expression', inject(function($rootScope) {
492+
var ob = {};
493+
$rootScope.scopedValue = 1;
494+
$rootScope.$eval('a.value = scopedValue', {a: ob});
495+
expect(ob.value).toBe(1);
496+
expect($rootScope.a).toBeUndefined();
497+
}));
498+
499+
it('should ignore locals beyond the root object of an assignment expression', inject(function($rootScope) {
500+
var ob = {};
501+
var locals = {a: ob};
502+
$rootScope.scopedValue = 1;
503+
$rootScope.$eval('a.value = scopedValue', locals);
504+
expect(ob.value).toBe(1);
505+
expect(ob.b).toBeUndefined();
506+
expect($rootScope.b).toBeUndefined();
507+
}));
508+
491509
it('should evaluate assignments in ternary operator', function() {
492510
scope.$eval('a = 1 ? 2 : 3');
493511
expect(scope.a).toBe(2);
@@ -799,6 +817,12 @@ describe('parser', function() {
799817
}).toThrowMinErr(
800818
'$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
801819
'Expression: a.toString.constructor');
820+
821+
expect(function() {
822+
scope.$eval("c.a = 1", {c: Function.prototype.constructor});
823+
}).toThrowMinErr(
824+
'$parse', 'isecfn', 'Referencing Function in Angular expressions is disallowed! ' +
825+
'Expression: c.a');
802826
});
803827

804828
it('should disallow traversing the Function object in a setter: E02', function() {
@@ -933,6 +957,14 @@ describe('parser', function() {
933957
'$parse', 'isecobj', 'Referencing Object in Angular expressions is disallowed! ' +
934958
'Expression: foo["bar"]["keys"](foo)');
935959
});
960+
961+
it('should NOT allow access to Object constructor in assignment locals', function() {
962+
expect(function() {
963+
scope.$eval("O.constructor.a = 1", {O: Object});
964+
}).toThrowMinErr(
965+
'$parse', 'isecobj', 'Referencing Object in Angular expressions is disallowed! ' +
966+
'Expression: O.constructor.a');
967+
});
936968
});
937969

938970
describe('Window and $element/node', function() {

0 commit comments

Comments
 (0)