diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 639afcb9dfcb..2c1f7af473cc 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -954,15 +954,6 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) { var value = element.val(), event = ev && ev.type; - // IE (11 and under) seem to emit an 'input' event if the placeholder value changes. - // We don't want to dirty the value when this happens, so we abort here. Unfortunately, - // IE also sends input events for other non-input-related things, (such as focusing on a - // form control), so this change is not entirely enough to solve this. - if (msie && (ev || noevent).type === 'input' && element[0].placeholder !== placeholder) { - placeholder = element[0].placeholder; - return; - } - // By default we will trim the value // If the attribute ng-trim exists we will avoid trimming // If input type is 'password', the value is never trimmed diff --git a/src/ng/directive/ngEventDirs.js b/src/ng/directive/ngEventDirs.js index 35ec339d5f16..c14b173a2b48 100644 --- a/src/ng/directive/ngEventDirs.js +++ b/src/ng/directive/ngEventDirs.js @@ -53,7 +53,11 @@ forEach( return { restrict: 'A', compile: function($element, attr) { - var fn = $parse(attr[directiveName]); + // We expose the powerful $event object on the scope that provides access to the Window, + // etc. that isn't protected by the fast paths in $parse. We explicitly request better + // checks at the cost of speed since event handler expressions are not executed as + // frequently as regular change detection. + var fn = $parse(attr[directiveName], /* interceptorFn */ null, /* expensiveChecks */ true); return function ngEventHandler(scope, element) { element.on(eventName, function(event) { var callback = function() { diff --git a/src/ng/parse.js b/src/ng/parse.js index 3f9396ccab3e..990a960ebda5 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -852,7 +852,8 @@ function setter(obj, path, setValue, fullExp) { return setValue; } -var getterFnCache = createMap(); +var getterFnCacheDefault = createMap(); +var getterFnCacheExpensive = createMap(); function isPossiblyDangerousMemberName(name) { return name == 'constructor'; @@ -863,7 +864,7 @@ function isPossiblyDangerousMemberName(name) { * - http://jsperf.com/angularjs-parse-getter/4 * - http://jsperf.com/path-evaluation-simplified/7 */ -function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) { +function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, expensiveChecks) { ensureSafeMemberName(key0, fullExp); ensureSafeMemberName(key1, fullExp); ensureSafeMemberName(key2, fullExp); @@ -872,11 +873,11 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) { var eso = function(o) { return ensureSafeObject(o, fullExp); }; - var eso0 = isPossiblyDangerousMemberName(key0) ? eso : identity; - var eso1 = isPossiblyDangerousMemberName(key1) ? eso : identity; - var eso2 = isPossiblyDangerousMemberName(key2) ? eso : identity; - var eso3 = isPossiblyDangerousMemberName(key3) ? eso : identity; - var eso4 = isPossiblyDangerousMemberName(key4) ? eso : identity; + var eso0 = (expensiveChecks || isPossiblyDangerousMemberName(key0)) ? eso : identity; + var eso1 = (expensiveChecks || isPossiblyDangerousMemberName(key1)) ? eso : identity; + var eso2 = (expensiveChecks || isPossiblyDangerousMemberName(key2)) ? eso : identity; + var eso3 = (expensiveChecks || isPossiblyDangerousMemberName(key3)) ? eso : identity; + var eso4 = (expensiveChecks || isPossiblyDangerousMemberName(key4)) ? eso : identity; return function cspSafeGetter(scope, locals) { var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope; @@ -911,23 +912,25 @@ function getterFnWithEnsureSafeObject(fn, fullExpression) { } function getterFn(path, options, fullExp) { + var expensiveChecks = options.expensiveChecks; + var getterFnCache = (expensiveChecks ? getterFnCacheExpensive : getterFnCacheDefault); var fn = getterFnCache[path]; - if (fn) return fn; + var pathKeys = path.split('.'), pathKeysLength = pathKeys.length; // http://jsperf.com/angularjs-parse-getter/6 if (options.csp) { if (pathKeysLength < 6) { - fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp); + fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp, expensiveChecks); } else { fn = function cspSafeGetter(scope, locals) { var i = 0, val; do { val = cspSafeGetterFn(pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++], - pathKeys[i++], fullExp)(scope, locals); + pathKeys[i++], fullExp, expensiveChecks)(scope, locals); locals = undefined; // clear after first iteration scope = val; @@ -937,7 +940,10 @@ function getterFn(path, options, fullExp) { } } else { var code = ''; - var needsEnsureSafeObject = false; + if (expensiveChecks) { + code += 's = eso(s, fe);\nl = eso(l, fe);\n'; + } + var needsEnsureSafeObject = expensiveChecks; forEach(pathKeys, function(key, index) { ensureSafeMemberName(key, fullExp); var lookupJs = (index @@ -945,7 +951,7 @@ function getterFn(path, options, fullExp) { ? 's' // but if we are first then we check locals first, and if so read it first : '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key; - if (isPossiblyDangerousMemberName(key)) { + if (expensiveChecks || isPossiblyDangerousMemberName(key)) { lookupJs = 'eso(' + lookupJs + ', fe)'; needsEnsureSafeObject = true; } @@ -1030,15 +1036,20 @@ function getValueOf(value) { * service. */ function $ParseProvider() { - var cache = createMap(); + var cacheDefault = createMap(); + var cacheExpensive = createMap(); - var $parseOptions = { - csp: false - }; this.$get = ['$filter', '$sniffer', function($filter, $sniffer) { - $parseOptions.csp = $sniffer.csp; + var $parseOptions = { + csp: $sniffer.csp, + expensiveChecks: false + }, + $parseOptionsExpensive = { + csp: $sniffer.csp, + expensiveChecks: true + }; function wrapSharedExpression(exp) { var wrapped = exp; @@ -1055,13 +1066,14 @@ function $ParseProvider() { return wrapped; } - return function $parse(exp, interceptorFn) { + return function $parse(exp, interceptorFn, expensiveChecks) { var parsedExpression, oneTime, cacheKey; switch (typeof exp) { case 'string': cacheKey = exp = exp.trim(); + var cache = (expensiveChecks ? cacheExpensive : cacheDefault); parsedExpression = cache[cacheKey]; if (!parsedExpression) { @@ -1070,8 +1082,9 @@ function $ParseProvider() { exp = exp.substring(2); } - var lexer = new Lexer($parseOptions); - var parser = new Parser(lexer, $filter, $parseOptions); + var parseOptions = expensiveChecks ? $parseOptionsExpensive : $parseOptions; + var lexer = new Lexer(parseOptions); + var parser = new Parser(lexer, $filter, parseOptions); parsedExpression = parser.parse(exp); if (parsedExpression.constant) { diff --git a/src/ng/sniffer.js b/src/ng/sniffer.js index 6b2ea859eb4c..81f46503d1fb 100644 --- a/src/ng/sniffer.js +++ b/src/ng/sniffer.js @@ -67,7 +67,9 @@ function $SnifferProvider() { // IE9 implements 'input' event it's so fubared that we rather pretend that it doesn't have // it. In particular the event is not fired when backspace or delete key are pressed or // when cut operation is performed. - if (event == 'input' && msie == 9) return false; + // IE10+ implements 'input' event but it erroneously fires under various situations, + // e.g. when placeholder changes, or a form is focused. + if (event === 'input' && msie <= 11) return false; if (isUndefined(eventSupport[event])) { var divElm = document.createElement('div'); diff --git a/src/ngScenario/dsl.js b/src/ngScenario/dsl.js index 037e1bbb7efa..91d94424e116 100644 --- a/src/ngScenario/dsl.js +++ b/src/ngScenario/dsl.js @@ -199,7 +199,7 @@ angular.scenario.dsl('binding', function() { */ angular.scenario.dsl('input', function() { var chain = {}; - var supportInputEvent = 'oninput' in document.createElement('div') && msie != 9; + var supportInputEvent = 'oninput' in document.createElement('div') && !(msie <= 11) chain.enter = function(value, event) { return this.addFutureAction("input '" + this.name + "' enter '" + value + "'", diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 8d4e47763609..a6745aef1813 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -1388,22 +1388,188 @@ describe('input', function() { expect(scope.name).toEqual('caitp'); }); - it('should not dirty the model on an input event in response to a placeholder change', inject(function($sniffer) { - if (msie && $sniffer.hasEvent('input')) { - compileInput(''); - inputElm.attr('placeholder', 'Test'); - browserTrigger(inputElm, 'input'); + describe("IE placeholder input events", function() { + //IE fires an input event whenever a placeholder visually changes, essentially treating it as a value + //Events: + // placeholder attribute change: *input* + // focus (which visually removes the placeholder value): focusin focus *input* + // blur (which visually creates the placeholder value): focusout *input* blur + //However none of these occur if the placeholder is not visible at the time of the event. + //These tests try simulate various scenerios which do/do-not fire the extra input event + + it('should not dirty the model on an input event in response to a placeholder change', function() { + if (msie) { + compileInput(''); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe('Test'); + expect(inputElm).toBePristine(); + + attrs.$set('placeholder', ''); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe(''); + expect(inputElm).toBePristine(); + + attrs.$set('placeholder', 'Test Again'); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe('Test Again'); + expect(inputElm).toBePristine(); + + attrs.$set('placeholder', undefined); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe(undefined); + expect(inputElm).toBePristine(); + + changeInputValueTo('foo'); + expect(inputElm).toBeDirty(); + } + }); - expect(inputElm.attr('placeholder')).toBe('Test'); - expect(inputElm).toBePristine(); + it('should not dirty the model on an input event in response to a interpolated placeholder change', inject(function($rootScope) { + if (msie) { + compileInput(''); + browserTrigger(inputElm, 'input'); + expect(inputElm).toBePristine(); - inputElm.attr('placeholder', 'Test Again'); - browserTrigger(inputElm, 'input'); + $rootScope.ph = 1; + $rootScope.$digest(); + browserTrigger(inputElm, 'input'); + expect(inputElm).toBePristine(); - expect(inputElm.attr('placeholder')).toBe('Test Again'); - expect(inputElm).toBePristine(); - } - })); + $rootScope.ph = ""; + $rootScope.$digest(); + browserTrigger(inputElm, 'input'); + expect(inputElm).toBePristine(); + + changeInputValueTo('foo'); + expect(inputElm).toBeDirty(); + } + })); + + it('should dirty the model on an input event in response to a placeholder change while in focus', inject(function($rootScope) { + if (msie) { + $rootScope.ph = 'Test'; + compileInput(''); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusin'); + browserTrigger(inputElm, 'focus'); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe('Test'); + expect(inputElm).toBePristine(); + + $rootScope.ph = 'Test Again'; + $rootScope.$digest(); + expect(inputElm).toBePristine(); + + changeInputValueTo('foo'); + expect(inputElm).toBeDirty(); + } + })); + + it('should not dirty the model on an input event in response to a ng-attr-placeholder change', inject(function($rootScope) { + if (msie) { + compileInput(''); + expect(inputElm).toBePristine(); + + $rootScope.ph = 1; + $rootScope.$digest(); + browserTrigger(inputElm, 'input'); + expect(inputElm).toBePristine(); + + $rootScope.ph = ""; + $rootScope.$digest(); + browserTrigger(inputElm, 'input'); + expect(inputElm).toBePristine(); + + changeInputValueTo('foo'); + expect(inputElm).toBeDirty(); + } + })); + + it('should not dirty the model on an input event in response to a focus', inject(function($sniffer) { + if (msie) { + compileInput(''); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe('Test'); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusin'); + browserTrigger(inputElm, 'focus'); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe('Test'); + expect(inputElm).toBePristine(); + + changeInputValueTo('foo'); + expect(inputElm).toBeDirty(); + } + })); + + it('should not dirty the model on an input event in response to a blur', inject(function($sniffer) { + if (msie) { + compileInput(''); + browserTrigger(inputElm, 'input'); + expect(inputElm.attr('placeholder')).toBe('Test'); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusin'); + browserTrigger(inputElm, 'focus'); + browserTrigger(inputElm, 'input'); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusout'); + browserTrigger(inputElm, 'input'); + browserTrigger(inputElm, 'blur'); + expect(inputElm).toBePristine(); + + changeInputValueTo('foo'); + expect(inputElm).toBeDirty(); + } + })); + + it('should dirty the model on an input event if there is a placeholder and value', inject(function($rootScope) { + if (msie) { + $rootScope.name = 'foo'; + compileInput(''); + expect(inputElm.val()).toBe($rootScope.name); + expect(inputElm).toBePristine(); + + changeInputValueTo('bar'); + expect(inputElm).toBeDirty(); + } + })); + + it('should dirty the model on an input event if there is a placeholder and value after focusing', inject(function($rootScope) { + if (msie) { + $rootScope.name = 'foo'; + compileInput(''); + expect(inputElm.val()).toBe($rootScope.name); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusin'); + browserTrigger(inputElm, 'focus'); + changeInputValueTo('bar'); + expect(inputElm).toBeDirty(); + } + })); + + it('should dirty the model on an input event if there is a placeholder and value after bluring', inject(function($rootScope) { + if (msie) { + $rootScope.name = 'foo'; + compileInput(''); + expect(inputElm.val()).toBe($rootScope.name); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusin'); + browserTrigger(inputElm, 'focus'); + expect(inputElm).toBePristine(); + + browserTrigger(inputElm, 'focusout'); + browserTrigger(inputElm, 'blur'); + changeInputValueTo('bar'); + expect(inputElm).toBeDirty(); + } + })); + }); it('should interpolate input names', function() { diff --git a/test/ng/directive/ngEventDirsSpec.js b/test/ng/directive/ngEventDirsSpec.js index d75398228e59..19ad69c96d92 100644 --- a/test/ng/directive/ngEventDirsSpec.js +++ b/test/ng/directive/ngEventDirsSpec.js @@ -90,6 +90,25 @@ describe('event directives', function() { }); + describe('security', function() { + it('should allow access to the $event object', inject(function($rootScope, $compile) { + var scope = $rootScope.$new(); + element = $compile('')(scope); + element.triggerHandler('click'); + expect(scope.e.target).toBe(element[0]); + })); + + it('should block access to DOM nodes (e.g. exposed via $event)', inject(function($rootScope, $compile) { + var scope = $rootScope.$new(); + element = $compile('')(scope); + expect(function() { + element.triggerHandler('click'); + }).toThrowMinErr( + '$parse', 'isecdom', 'Referencing DOM nodes in Angular expressions is disallowed! ' + + 'Expression: e = $event.target'); + })); + }); + describe('blur', function() { describe('call the listener asynchronously during $apply', function() { diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index e83f0e505fa5..2e358bf95642 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -3,9 +3,11 @@ describe('parser', function() { beforeEach(function() { - /* global getterFnCache: true */ - // clear cache - getterFnCache = createMap(); + /* global getterFnCacheDefault: true */ + /* global getterFnCacheExpensive: true */ + // clear caches + getterFnCacheDefault = createMap(); + getterFnCacheExpensive = createMap(); }); @@ -783,6 +785,22 @@ describe('parser', function() { 'Expression: foo["bar"]'); }); + + describe('expensiveChecks', function() { + it('should block access to window object even when aliased', inject(function($parse, $window) { + scope.foo = {w: $window}; + // This isn't blocked for performance. + expect(scope.$eval($parse('foo.w'))).toBe($window); + // Event handlers use the more expensive path for better protection since they expose + // the $event object on the scope. + expect(function() { + scope.$eval($parse('foo.w', null, true)); + }).toThrowMinErr( + '$parse', 'isecwindow', 'Referencing the Window in Angular expressions is disallowed! ' + + 'Expression: foo.w'); + + })); + }); }); describe('Function prototype functions', function() { diff --git a/test/ng/snifferSpec.js b/test/ng/snifferSpec.js index 7d48bc79ffea..382952607686 100644 --- a/test/ng/snifferSpec.js +++ b/test/ng/snifferSpec.js @@ -64,9 +64,10 @@ describe('$sniffer', function() { it('should claim that IE9 doesn\'t have support for "oninput"', function() { // IE9 implementation is fubared, so it's better to pretend that it doesn't have the support + // IE10+ implementation is fubared when mixed with placeholders mockDivElement = {oninput: noop}; - expect($sniffer.hasEvent('input')).toBe((msie == 9) ? false : true); + expect($sniffer.hasEvent('input')).toBe(!(msie && msie <= 11)); }); });