Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 8056749

Browse files
chirayukcaitp
authored andcommitted
fix($parse, events): prevent accidental misuse of properties on $event
1 parent e676d64 commit 8056749

File tree

9 files changed

+263
-49
lines changed

9 files changed

+263
-49
lines changed

src/ng/directive/input.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -954,15 +954,6 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) {
954954
var value = element.val(),
955955
event = ev && ev.type;
956956

957-
// IE (11 and under) seem to emit an 'input' event if the placeholder value changes.
958-
// We don't want to dirty the value when this happens, so we abort here. Unfortunately,
959-
// IE also sends input events for other non-input-related things, (such as focusing on a
960-
// form control), so this change is not entirely enough to solve this.
961-
if (msie && (ev || noevent).type === 'input' && element[0].placeholder !== placeholder) {
962-
placeholder = element[0].placeholder;
963-
return;
964-
}
965-
966957
// By default we will trim the value
967958
// If the attribute ng-trim exists we will avoid trimming
968959
// If input type is 'password', the value is never trimmed

src/ng/directive/ngEventDirs.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ forEach(
5353
return {
5454
restrict: 'A',
5555
compile: function($element, attr) {
56-
var fn = $parse(attr[directiveName]);
56+
// We expose the powerful $event object on the scope that provides access to the Window,
57+
// etc. that isn't protected by the fast paths in $parse. We explicitly request better
58+
// checks at the cost of speed since event handler expressions are not executed as
59+
// frequently as regular change detection.
60+
var fn = $parse(attr[directiveName], /* interceptorFn */ null, /* expensiveChecks */ true);
5761
return function ngEventHandler(scope, element) {
5862
element.on(eventName, function(event) {
5963
var callback = function() {

src/ng/parse.js

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,8 @@ function setter(obj, path, setValue, fullExp) {
852852
return setValue;
853853
}
854854

855-
var getterFnCache = createMap();
855+
var getterFnCacheDefault = createMap();
856+
var getterFnCacheExpensive = createMap();
856857

857858
function isPossiblyDangerousMemberName(name) {
858859
return name == 'constructor';
@@ -863,7 +864,7 @@ function isPossiblyDangerousMemberName(name) {
863864
* - http://jsperf.com/angularjs-parse-getter/4
864865
* - http://jsperf.com/path-evaluation-simplified/7
865866
*/
866-
function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
867+
function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, expensiveChecks) {
867868
ensureSafeMemberName(key0, fullExp);
868869
ensureSafeMemberName(key1, fullExp);
869870
ensureSafeMemberName(key2, fullExp);
@@ -872,11 +873,11 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
872873
var eso = function(o) {
873874
return ensureSafeObject(o, fullExp);
874875
};
875-
var eso0 = isPossiblyDangerousMemberName(key0) ? eso : identity;
876-
var eso1 = isPossiblyDangerousMemberName(key1) ? eso : identity;
877-
var eso2 = isPossiblyDangerousMemberName(key2) ? eso : identity;
878-
var eso3 = isPossiblyDangerousMemberName(key3) ? eso : identity;
879-
var eso4 = isPossiblyDangerousMemberName(key4) ? eso : identity;
876+
var eso0 = (expensiveChecks || isPossiblyDangerousMemberName(key0)) ? eso : identity;
877+
var eso1 = (expensiveChecks || isPossiblyDangerousMemberName(key1)) ? eso : identity;
878+
var eso2 = (expensiveChecks || isPossiblyDangerousMemberName(key2)) ? eso : identity;
879+
var eso3 = (expensiveChecks || isPossiblyDangerousMemberName(key3)) ? eso : identity;
880+
var eso4 = (expensiveChecks || isPossiblyDangerousMemberName(key4)) ? eso : identity;
880881

881882
return function cspSafeGetter(scope, locals) {
882883
var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope;
@@ -911,23 +912,25 @@ function getterFnWithEnsureSafeObject(fn, fullExpression) {
911912
}
912913

913914
function getterFn(path, options, fullExp) {
915+
var expensiveChecks = options.expensiveChecks;
916+
var getterFnCache = (expensiveChecks ? getterFnCacheExpensive : getterFnCacheDefault);
914917
var fn = getterFnCache[path];
915-
916918
if (fn) return fn;
917919

920+
918921
var pathKeys = path.split('.'),
919922
pathKeysLength = pathKeys.length;
920923

921924
// http://jsperf.com/angularjs-parse-getter/6
922925
if (options.csp) {
923926
if (pathKeysLength < 6) {
924-
fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp);
927+
fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp, expensiveChecks);
925928
} else {
926929
fn = function cspSafeGetter(scope, locals) {
927930
var i = 0, val;
928931
do {
929932
val = cspSafeGetterFn(pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++],
930-
pathKeys[i++], fullExp)(scope, locals);
933+
pathKeys[i++], fullExp, expensiveChecks)(scope, locals);
931934

932935
locals = undefined; // clear after first iteration
933936
scope = val;
@@ -937,15 +940,18 @@ function getterFn(path, options, fullExp) {
937940
}
938941
} else {
939942
var code = '';
940-
var needsEnsureSafeObject = false;
943+
if (expensiveChecks) {
944+
code += 's = eso(s, fe);\nl = eso(l, fe);\n';
945+
}
946+
var needsEnsureSafeObject = expensiveChecks;
941947
forEach(pathKeys, function(key, index) {
942948
ensureSafeMemberName(key, fullExp);
943949
var lookupJs = (index
944950
// we simply dereference 's' on any .dot notation
945951
? 's'
946952
// but if we are first then we check locals first, and if so read it first
947953
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key;
948-
if (isPossiblyDangerousMemberName(key)) {
954+
if (expensiveChecks || isPossiblyDangerousMemberName(key)) {
949955
lookupJs = 'eso(' + lookupJs + ', fe)';
950956
needsEnsureSafeObject = true;
951957
}
@@ -1030,15 +1036,20 @@ function getValueOf(value) {
10301036
* service.
10311037
*/
10321038
function $ParseProvider() {
1033-
var cache = createMap();
1039+
var cacheDefault = createMap();
1040+
var cacheExpensive = createMap();
10341041

1035-
var $parseOptions = {
1036-
csp: false
1037-
};
10381042

10391043

10401044
this.$get = ['$filter', '$sniffer', function($filter, $sniffer) {
1041-
$parseOptions.csp = $sniffer.csp;
1045+
var $parseOptions = {
1046+
csp: $sniffer.csp,
1047+
expensiveChecks: false
1048+
},
1049+
$parseOptionsExpensive = {
1050+
csp: $sniffer.csp,
1051+
expensiveChecks: true
1052+
};
10421053

10431054
function wrapSharedExpression(exp) {
10441055
var wrapped = exp;
@@ -1055,13 +1066,14 @@ function $ParseProvider() {
10551066
return wrapped;
10561067
}
10571068

1058-
return function $parse(exp, interceptorFn) {
1069+
return function $parse(exp, interceptorFn, expensiveChecks) {
10591070
var parsedExpression, oneTime, cacheKey;
10601071

10611072
switch (typeof exp) {
10621073
case 'string':
10631074
cacheKey = exp = exp.trim();
10641075

1076+
var cache = (expensiveChecks ? cacheExpensive : cacheDefault);
10651077
parsedExpression = cache[cacheKey];
10661078

10671079
if (!parsedExpression) {
@@ -1070,8 +1082,9 @@ function $ParseProvider() {
10701082
exp = exp.substring(2);
10711083
}
10721084

1073-
var lexer = new Lexer($parseOptions);
1074-
var parser = new Parser(lexer, $filter, $parseOptions);
1085+
var parseOptions = expensiveChecks ? $parseOptionsExpensive : $parseOptions;
1086+
var lexer = new Lexer(parseOptions);
1087+
var parser = new Parser(lexer, $filter, parseOptions);
10751088
parsedExpression = parser.parse(exp);
10761089

10771090
if (parsedExpression.constant) {

src/ng/sniffer.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ function $SnifferProvider() {
6767
// IE9 implements 'input' event it's so fubared that we rather pretend that it doesn't have
6868
// it. In particular the event is not fired when backspace or delete key are pressed or
6969
// when cut operation is performed.
70-
if (event == 'input' && msie == 9) return false;
70+
// IE10+ implements 'input' event but it erroneously fires under various situations,
71+
// e.g. when placeholder changes, or a form is focused.
72+
if (event === 'input' && msie <= 11) return false;
7173

7274
if (isUndefined(eventSupport[event])) {
7375
var divElm = document.createElement('div');

src/ngScenario/dsl.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ angular.scenario.dsl('binding', function() {
199199
*/
200200
angular.scenario.dsl('input', function() {
201201
var chain = {};
202-
var supportInputEvent = 'oninput' in document.createElement('div') && msie != 9;
202+
var supportInputEvent = 'oninput' in document.createElement('div') && !(msie <= 11)
203203

204204
chain.enter = function(value, event) {
205205
return this.addFutureAction("input '" + this.name + "' enter '" + value + "'",

test/ng/directive/inputSpec.js

Lines changed: 179 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,22 +1388,188 @@ describe('input', function() {
13881388
expect(scope.name).toEqual('caitp');
13891389
});
13901390

1391-
it('should not dirty the model on an input event in response to a placeholder change', inject(function($sniffer) {
1392-
if (msie && $sniffer.hasEvent('input')) {
1393-
compileInput('<input type="text" ng-model="name" name="name" />');
1394-
inputElm.attr('placeholder', 'Test');
1395-
browserTrigger(inputElm, 'input');
1391+
describe("IE placeholder input events", function() {
1392+
//IE fires an input event whenever a placeholder visually changes, essentially treating it as a value
1393+
//Events:
1394+
// placeholder attribute change: *input*
1395+
// focus (which visually removes the placeholder value): focusin focus *input*
1396+
// blur (which visually creates the placeholder value): focusout *input* blur
1397+
//However none of these occur if the placeholder is not visible at the time of the event.
1398+
//These tests try simulate various scenerios which do/do-not fire the extra input event
1399+
1400+
it('should not dirty the model on an input event in response to a placeholder change', function() {
1401+
if (msie) {
1402+
compileInput('<input type="text" placeholder="Test" attr-capture ng-model="unsetValue" name="name" />');
1403+
browserTrigger(inputElm, 'input');
1404+
expect(inputElm.attr('placeholder')).toBe('Test');
1405+
expect(inputElm).toBePristine();
1406+
1407+
attrs.$set('placeholder', '');
1408+
browserTrigger(inputElm, 'input');
1409+
expect(inputElm.attr('placeholder')).toBe('');
1410+
expect(inputElm).toBePristine();
1411+
1412+
attrs.$set('placeholder', 'Test Again');
1413+
browserTrigger(inputElm, 'input');
1414+
expect(inputElm.attr('placeholder')).toBe('Test Again');
1415+
expect(inputElm).toBePristine();
1416+
1417+
attrs.$set('placeholder', undefined);
1418+
browserTrigger(inputElm, 'input');
1419+
expect(inputElm.attr('placeholder')).toBe(undefined);
1420+
expect(inputElm).toBePristine();
1421+
1422+
changeInputValueTo('foo');
1423+
expect(inputElm).toBeDirty();
1424+
}
1425+
});
13961426

1397-
expect(inputElm.attr('placeholder')).toBe('Test');
1398-
expect(inputElm).toBePristine();
1427+
it('should not dirty the model on an input event in response to a interpolated placeholder change', inject(function($rootScope) {
1428+
if (msie) {
1429+
compileInput('<input type="text" placeholder="{{ph}}" ng-model="unsetValue" name="name" />');
1430+
browserTrigger(inputElm, 'input');
1431+
expect(inputElm).toBePristine();
13991432

1400-
inputElm.attr('placeholder', 'Test Again');
1401-
browserTrigger(inputElm, 'input');
1433+
$rootScope.ph = 1;
1434+
$rootScope.$digest();
1435+
browserTrigger(inputElm, 'input');
1436+
expect(inputElm).toBePristine();
14021437

1403-
expect(inputElm.attr('placeholder')).toBe('Test Again');
1404-
expect(inputElm).toBePristine();
1405-
}
1406-
}));
1438+
$rootScope.ph = "";
1439+
$rootScope.$digest();
1440+
browserTrigger(inputElm, 'input');
1441+
expect(inputElm).toBePristine();
1442+
1443+
changeInputValueTo('foo');
1444+
expect(inputElm).toBeDirty();
1445+
}
1446+
}));
1447+
1448+
it('should dirty the model on an input event in response to a placeholder change while in focus', inject(function($rootScope) {
1449+
if (msie) {
1450+
$rootScope.ph = 'Test';
1451+
compileInput('<input type="text" ng-attr-placeholder="{{ph}}" ng-model="unsetValue" name="name" />');
1452+
expect(inputElm).toBePristine();
1453+
1454+
browserTrigger(inputElm, 'focusin');
1455+
browserTrigger(inputElm, 'focus');
1456+
browserTrigger(inputElm, 'input');
1457+
expect(inputElm.attr('placeholder')).toBe('Test');
1458+
expect(inputElm).toBePristine();
1459+
1460+
$rootScope.ph = 'Test Again';
1461+
$rootScope.$digest();
1462+
expect(inputElm).toBePristine();
1463+
1464+
changeInputValueTo('foo');
1465+
expect(inputElm).toBeDirty();
1466+
}
1467+
}));
1468+
1469+
it('should not dirty the model on an input event in response to a ng-attr-placeholder change', inject(function($rootScope) {
1470+
if (msie) {
1471+
compileInput('<input type="text" ng-attr-placeholder="{{ph}}" ng-model="unsetValue" name="name" />');
1472+
expect(inputElm).toBePristine();
1473+
1474+
$rootScope.ph = 1;
1475+
$rootScope.$digest();
1476+
browserTrigger(inputElm, 'input');
1477+
expect(inputElm).toBePristine();
1478+
1479+
$rootScope.ph = "";
1480+
$rootScope.$digest();
1481+
browserTrigger(inputElm, 'input');
1482+
expect(inputElm).toBePristine();
1483+
1484+
changeInputValueTo('foo');
1485+
expect(inputElm).toBeDirty();
1486+
}
1487+
}));
1488+
1489+
it('should not dirty the model on an input event in response to a focus', inject(function($sniffer) {
1490+
if (msie) {
1491+
compileInput('<input type="text" placeholder="Test" ng-model="unsetValue" name="name" />');
1492+
browserTrigger(inputElm, 'input');
1493+
expect(inputElm.attr('placeholder')).toBe('Test');
1494+
expect(inputElm).toBePristine();
1495+
1496+
browserTrigger(inputElm, 'focusin');
1497+
browserTrigger(inputElm, 'focus');
1498+
browserTrigger(inputElm, 'input');
1499+
expect(inputElm.attr('placeholder')).toBe('Test');
1500+
expect(inputElm).toBePristine();
1501+
1502+
changeInputValueTo('foo');
1503+
expect(inputElm).toBeDirty();
1504+
}
1505+
}));
1506+
1507+
it('should not dirty the model on an input event in response to a blur', inject(function($sniffer) {
1508+
if (msie) {
1509+
compileInput('<input type="text" placeholder="Test" ng-model="unsetValue" name="name" />');
1510+
browserTrigger(inputElm, 'input');
1511+
expect(inputElm.attr('placeholder')).toBe('Test');
1512+
expect(inputElm).toBePristine();
1513+
1514+
browserTrigger(inputElm, 'focusin');
1515+
browserTrigger(inputElm, 'focus');
1516+
browserTrigger(inputElm, 'input');
1517+
expect(inputElm).toBePristine();
1518+
1519+
browserTrigger(inputElm, 'focusout');
1520+
browserTrigger(inputElm, 'input');
1521+
browserTrigger(inputElm, 'blur');
1522+
expect(inputElm).toBePristine();
1523+
1524+
changeInputValueTo('foo');
1525+
expect(inputElm).toBeDirty();
1526+
}
1527+
}));
1528+
1529+
it('should dirty the model on an input event if there is a placeholder and value', inject(function($rootScope) {
1530+
if (msie) {
1531+
$rootScope.name = 'foo';
1532+
compileInput('<input type="text" placeholder="Test" ng-model="name" value="init" name="name" />');
1533+
expect(inputElm.val()).toBe($rootScope.name);
1534+
expect(inputElm).toBePristine();
1535+
1536+
changeInputValueTo('bar');
1537+
expect(inputElm).toBeDirty();
1538+
}
1539+
}));
1540+
1541+
it('should dirty the model on an input event if there is a placeholder and value after focusing', inject(function($rootScope) {
1542+
if (msie) {
1543+
$rootScope.name = 'foo';
1544+
compileInput('<input type="text" placeholder="Test" ng-model="name" value="init" name="name" />');
1545+
expect(inputElm.val()).toBe($rootScope.name);
1546+
expect(inputElm).toBePristine();
1547+
1548+
browserTrigger(inputElm, 'focusin');
1549+
browserTrigger(inputElm, 'focus');
1550+
changeInputValueTo('bar');
1551+
expect(inputElm).toBeDirty();
1552+
}
1553+
}));
1554+
1555+
it('should dirty the model on an input event if there is a placeholder and value after bluring', inject(function($rootScope) {
1556+
if (msie) {
1557+
$rootScope.name = 'foo';
1558+
compileInput('<input type="text" placeholder="Test" ng-model="name" value="init" name="name" />');
1559+
expect(inputElm.val()).toBe($rootScope.name);
1560+
expect(inputElm).toBePristine();
1561+
1562+
browserTrigger(inputElm, 'focusin');
1563+
browserTrigger(inputElm, 'focus');
1564+
expect(inputElm).toBePristine();
1565+
1566+
browserTrigger(inputElm, 'focusout');
1567+
browserTrigger(inputElm, 'blur');
1568+
changeInputValueTo('bar');
1569+
expect(inputElm).toBeDirty();
1570+
}
1571+
}));
1572+
});
14071573

14081574

14091575
it('should interpolate input names', function() {

0 commit comments

Comments
 (0)