From bb9fa1a2a554b44b9532eb0195e2e66432ecb2d3 Mon Sep 17 00:00:00 2001 From: FernandoSendMail Date: Sun, 29 Mar 2015 02:08:36 -0700 Subject: [PATCH] fix(typeahead): use ng-bind-html - Switches to use `ng-bind-html` from `bind-html-unsafe` BREAKING CHANGE: This modifies the typeahead to use the the `$sce` service instead when `ngSanitize` is present Closes #3463 Resolves #4073 --- karma.conf.js | 1 + misc/demo/assets/app.js | 2 +- misc/demo/index.html | 1 + package.json | 1 + .../typeahead-highlight-ngsanitize.spec.js | 16 +++++++++ .../test/typeahead-highlight.spec.js | 35 +++++++++++++------ src/typeahead/test/typeahead.spec.js | 1 + src/typeahead/typeahead.js | 26 ++++++++++---- template/typeahead/typeahead-match.html | 2 +- 9 files changed, 66 insertions(+), 19 deletions(-) create mode 100644 src/typeahead/test/typeahead-highlight-ngsanitize.spec.js diff --git a/karma.conf.js b/karma.conf.js index 1fda181b9f..c3da490397 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -18,6 +18,7 @@ module.exports = function(config) { 'misc/test-lib/jquery-1.8.2.min.js', 'node_modules/angular/angular.js', 'node_modules/angular-mocks/angular-mocks.js', + 'node_modules/angular-sanitize/angular-sanitize.js', 'misc/test-lib/helpers.js', 'src/**/*.js', 'template/**/*.js' diff --git a/misc/demo/assets/app.js b/misc/demo/assets/app.js index af0c4a8ff1..8d19f5481b 100644 --- a/misc/demo/assets/app.js +++ b/misc/demo/assets/app.js @@ -1,5 +1,5 @@ /* global FastClick, smoothScroll */ -angular.module('ui.bootstrap.demo', ['ui.bootstrap', 'plunker', 'ngTouch', 'ngAnimate'], function($httpProvider){ +angular.module('ui.bootstrap.demo', ['ui.bootstrap', 'plunker', 'ngTouch', 'ngAnimate', 'ngSanitize'], function($httpProvider){ FastClick.attach(document.body); delete $httpProvider.defaults.headers.common['X-Requested-With']; }).run(['$location', function($location){ diff --git a/misc/demo/index.html b/misc/demo/index.html index 9b5e683239..6cd3b1db0f 100644 --- a/misc/demo/index.html +++ b/misc/demo/index.html @@ -14,6 +14,7 @@ + diff --git a/package.json b/package.json index bf8aa92bec..fab1235491 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "devDependencies": { "angular": "^1.4.3", "angular-mocks": "^1.4.3", + "angular-sanitize": "^1.4.3", "grunt": "^0.4.5", "grunt-contrib-concat": "^0.5.1", "grunt-contrib-copy": "^0.8.0", diff --git a/src/typeahead/test/typeahead-highlight-ngsanitize.spec.js b/src/typeahead/test/typeahead-highlight-ngsanitize.spec.js new file mode 100644 index 0000000000..346a4132b3 --- /dev/null +++ b/src/typeahead/test/typeahead-highlight-ngsanitize.spec.js @@ -0,0 +1,16 @@ +describe('Security concerns', function() { + var highlightFilter, $sanitize, logSpy; + + beforeEach(module('ui.bootstrap.typeahead', 'ngSanitize')); + + beforeEach(inject(function (typeaheadHighlightFilter, _$sanitize_, $log) { + highlightFilter = typeaheadHighlightFilter; + $sanitize = _$sanitize_; + logSpy = spyOn($log, 'warn'); + })); + + it('should not call the $log service when ngSanitize is present', function() { + highlightFilter('before after', 'match'); + expect(logSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/typeahead/test/typeahead-highlight.spec.js b/src/typeahead/test/typeahead-highlight.spec.js index 1a5d8b1c5d..e587ee43c0 100644 --- a/src/typeahead/test/typeahead-highlight.spec.js +++ b/src/typeahead/test/typeahead-highlight.spec.js @@ -1,38 +1,51 @@ -describe('typeaheadHighlight', function() { - var highlightFilter; +describe('typeaheadHighlight', function () { + + var highlightFilter, $log, $sce, logSpy; beforeEach(module('ui.bootstrap.typeahead')); + + beforeEach(inject(function(_$log_, _$sce_) { + $log = _$log_; + $sce = _$sce_; + logSpy = spyOn($log, 'warn'); + })); + beforeEach(inject(function(typeaheadHighlightFilter) { highlightFilter = typeaheadHighlightFilter; })); it('should higlight a match', function() { - expect(highlightFilter('before match after', 'match')).toEqual('before match after'); + expect($sce.getTrustedHtml(highlightFilter('before match after', 'match'))).toEqual('before match after'); }); it('should higlight a match with mixed case', function() { - expect(highlightFilter('before MaTch after', 'match')).toEqual('before MaTch after'); + expect($sce.getTrustedHtml(highlightFilter('before MaTch after', 'match'))).toEqual('before MaTch after'); }); it('should higlight all matches', function() { - expect(highlightFilter('before MaTch after match', 'match')).toEqual('before MaTch after match'); + expect($sce.getTrustedHtml(highlightFilter('before MaTch after match', 'match'))).toEqual('before MaTch after match'); }); it('should do nothing if no match', function() { - expect(highlightFilter('before match after', 'nomatch')).toEqual('before match after'); + expect($sce.getTrustedHtml(highlightFilter('before match after', 'nomatch'))).toEqual('before match after'); }); it('should do nothing if no or empty query', function() { - expect(highlightFilter('before match after', '')).toEqual('before match after'); - expect(highlightFilter('before match after', null)).toEqual('before match after'); - expect(highlightFilter('before match after', undefined)).toEqual('before match after'); + expect($sce.getTrustedHtml(highlightFilter('before match after', ''))).toEqual('before match after'); + expect($sce.getTrustedHtml(highlightFilter('before match after', null))).toEqual('before match after'); + expect($sce.getTrustedHtml(highlightFilter('before match after', undefined))).toEqual('before match after'); }); it('issue 316 - should work correctly for regexp reserved words', function() { - expect(highlightFilter('before (match after', '(match')).toEqual('before (match after'); + expect($sce.getTrustedHtml(highlightFilter('before (match after', '(match'))).toEqual('before (match after'); }); it('issue 1777 - should work correctly with numeric values', function() { - expect(highlightFilter(123, '2')).toEqual('123'); + expect($sce.getTrustedHtml(highlightFilter(123, '2'))).toEqual('123'); + }); + + it('should show a warning when this component is being used unsafely', function() { + highlightFilter('before match after', 'match'); + expect(logSpy).toHaveBeenCalled(); }); }); diff --git a/src/typeahead/test/typeahead.spec.js b/src/typeahead/test/typeahead.spec.js index a5904c20ce..d31a8183dc 100644 --- a/src/typeahead/test/typeahead.spec.js +++ b/src/typeahead/test/typeahead.spec.js @@ -3,6 +3,7 @@ describe('typeahead tests', function() { var changeInputValueTo; beforeEach(module('ui.bootstrap.typeahead')); + beforeEach(module('ngSanitize')); beforeEach(module('template/typeahead/typeahead-popup.html')); beforeEach(module('template/typeahead/typeahead-match.html')); beforeEach(module(function($compileProvider) { diff --git a/src/typeahead/typeahead.js b/src/typeahead/typeahead.js index 3a4608f3d1..d360eb6d48 100644 --- a/src/typeahead/typeahead.js +++ b/src/typeahead/typeahead.js @@ -1,4 +1,4 @@ -angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap.bindHtml']) +angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position']) /** * A helper service that can parse typeahead's syntax (string provided by users) @@ -36,7 +36,6 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap return { require: 'ngModel', link: function(originalScope, element, attrs, modelCtrl) { - //SUPPORTED ATTRIBUTES (OPTIONS) //minimal no of characters that needs to be entered before typeahead kicks-in @@ -347,7 +346,6 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap //bind keyboard events: arrows up(38) / down(40), enter(13) and tab(9), esc(27) element.bind('keydown', function(evt) { - //typeahead is open and an "interesting" key was pressed if (scope.matches.length === 0 || HOT_KEYS.indexOf(evt.which) === -1) { return; @@ -483,12 +481,28 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap }; }]) - .filter('typeaheadHighlight', function() { + .filter('typeaheadHighlight', ['$sce', '$injector', '$log', function($sce, $injector, $log) { + var isSanitizePresent; + isSanitizePresent = $injector.has('$sanitize'); + function escapeRegexp(queryToEscape) { + // Regex: capture the whole query string and replace it with the string that will be used to match + // the results, for example if the capture is "a" the result will be \a return queryToEscape.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1'); } + function containsHtml(matchItem) { + return /<.*>/g.test(matchItem); + } + return function(matchItem, query) { - return query ? ('' + matchItem).replace(new RegExp(escapeRegexp(query), 'gi'), '$&') : matchItem; + if (!isSanitizePresent && containsHtml(matchItem)) { + $log.warn('Unsafe use of typeahead please use ngSanitize'); // Warn the user about the danger + } + matchItem = query? ('' + matchItem).replace(new RegExp(escapeRegexp(query), 'gi'), '$&') : matchItem; // Replaces the capture string with a the same string inside of a "strong" tag + if (!isSanitizePresent) { + matchItem = $sce.trustAsHtml(matchItem); // If $sanitize is not present we pack the string in a $sce object for the ng-bind-html directive + } + return matchItem; }; - }); + }]); diff --git a/template/typeahead/typeahead-match.html b/template/typeahead/typeahead-match.html index 0711518d9f..f8375988ca 100644 --- a/template/typeahead/typeahead-match.html +++ b/template/typeahead/typeahead-match.html @@ -1 +1 @@ - +