Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Commit 5a41d48

Browse files
fix(typeahead): Typeahead uses ngSanitize when present
1 parent 575eb85 commit 5a41d48

File tree

9 files changed

+86
-34
lines changed

9 files changed

+86
-34
lines changed

Diff for: karma.conf.js

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ module.exports = function(config) {
1818
'misc/test-lib/jquery-1.8.2.min.js',
1919
'node_modules/angular/angular.js',
2020
'node_modules/angular-mocks/angular-mocks.js',
21+
'node_modules/angular-sanitize/angular-sanitize.js',
2122
'misc/test-lib/helpers.js',
2223
'src/**/*.js',
2324
'template/**/*.js'

Diff for: misc/demo/assets/app.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* global FastClick, smoothScroll */
2-
angular.module('ui.bootstrap.demo', ['ui.bootstrap', 'plunker', 'ngTouch', 'ngAnimate'], function($httpProvider){
2+
angular.module('ui.bootstrap.demo', ['ui.bootstrap', 'plunker', 'ngTouch', 'ngAnimate', 'ngSanitize'], function($httpProvider){
33
FastClick.attach(document.body);
44
delete $httpProvider.defaults.headers.common['X-Requested-With'];
55
}).run(['$location', function($location){

Diff for: misc/demo/index.html

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular.min.js"></script>
1515
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-animate.min.js"></script>
1616
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-touch.min.js"></script>
17+
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-sanitize.js"></script>
1718
<script src="ui-bootstrap-tpls-<%= pkg.version%>.min.js"></script>
1819
<script src="assets/plunker.js"></script>
1920
<script src="assets/app.js"></script>

Diff for: package.json

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"devDependencies": {
1515
"angular": "^1.4.3",
1616
"angular-mocks": "^1.4.3",
17+
"angular-sanitize": "^1.4.3",
1718
"grunt": "^0.4.5",
1819
"grunt-contrib-concat": "^0.5.1",
1920
"grunt-contrib-copy": "^0.8.0",
+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
describe('Security concerns', function() {
2+
var highlightFilter, $sanitize, logSpy;
3+
4+
beforeEach(module('ui.bootstrap.typeahead', 'ngSanitize'));
5+
6+
beforeEach(inject(function (typeaheadHighlightFilter, _$sanitize_, $log) {
7+
highlightFilter = typeaheadHighlightFilter;
8+
$sanitize = _$sanitize_;
9+
logSpy = spyOn($log, 'warn');
10+
}));
11+
12+
it('should not call the $log service when ngSanitize is present', function() {
13+
highlightFilter('before <script src="">match</script> after', 'match');
14+
expect(logSpy).not.toHaveBeenCalled();
15+
});
16+
17+
});

Diff for: src/typeahead/test/typeahead-highlight.spec.js

+33-19
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,52 @@
1-
describe('typeaheadHighlight', function() {
2-
var highlightFilter;
1+
describe('typeaheadHighlight', function () {
2+
3+
var highlightFilter, $log, $sce, logSpy;
34

45
beforeEach(module('ui.bootstrap.typeahead'));
5-
beforeEach(inject(function(typeaheadHighlightFilter) {
6+
7+
beforeEach(inject(function(_$log_, _$sce_) {
8+
$log = _$log_;
9+
$sce = _$sce_;
10+
logSpy = spyOn($log, 'warn');
11+
}));
12+
13+
beforeEach(inject(function (typeaheadHighlightFilter) {
614
highlightFilter = typeaheadHighlightFilter;
715
}));
816

9-
it('should higlight a match', function() {
10-
expect(highlightFilter('before match after', 'match')).toEqual('before <strong>match</strong> after');
17+
it('should higlight a match', function () {
18+
expect($sce.getTrustedHtml(highlightFilter('before match after', 'match'))).toEqual('before <strong>match</strong> after');
1119
});
1220

13-
it('should higlight a match with mixed case', function() {
14-
expect(highlightFilter('before MaTch after', 'match')).toEqual('before <strong>MaTch</strong> after');
21+
it('should higlight a match with mixed case', function () {
22+
expect($sce.getTrustedHtml(highlightFilter('before MaTch after', 'match'))).toEqual('before <strong>MaTch</strong> after');
1523
});
1624

17-
it('should higlight all matches', function() {
18-
expect(highlightFilter('before MaTch after match', 'match')).toEqual('before <strong>MaTch</strong> after <strong>match</strong>');
25+
it('should higlight all matches', function () {
26+
expect($sce.getTrustedHtml(highlightFilter('before MaTch after match', 'match'))).toEqual('before <strong>MaTch</strong> after <strong>match</strong>');
1927
});
2028

21-
it('should do nothing if no match', function() {
22-
expect(highlightFilter('before match after', 'nomatch')).toEqual('before match after');
29+
it('should do nothing if no match', function () {
30+
expect($sce.getTrustedHtml(highlightFilter('before match after', 'nomatch'))).toEqual('before match after');
2331
});
2432

25-
it('should do nothing if no or empty query', function() {
26-
expect(highlightFilter('before match after', '')).toEqual('before match after');
27-
expect(highlightFilter('before match after', null)).toEqual('before match after');
28-
expect(highlightFilter('before match after', undefined)).toEqual('before match after');
33+
it('should do nothing if no or empty query', function () {
34+
expect($sce.getTrustedHtml(highlightFilter('before match after', ''))).toEqual('before match after');
35+
expect($sce.getTrustedHtml(highlightFilter('before match after', null))).toEqual('before match after');
36+
expect($sce.getTrustedHtml(highlightFilter('before match after', undefined))).toEqual('before match after');
2937
});
3038

31-
it('issue 316 - should work correctly for regexp reserved words', function() {
32-
expect(highlightFilter('before (match after', '(match')).toEqual('before <strong>(match</strong> after');
39+
it('issue 316 - should work correctly for regexp reserved words', function () {
40+
expect($sce.getTrustedHtml(highlightFilter('before (match after', '(match'))).toEqual('before <strong>(match</strong> after');
3341
});
3442

35-
it('issue 1777 - should work correctly with numeric values', function() {
36-
expect(highlightFilter(123, '2')).toEqual('1<strong>2</strong>3');
43+
it('issue 1777 - should work correctly with numeric values', function () {
44+
expect($sce.getTrustedHtml(highlightFilter(123, '2'))).toEqual('1<strong>2</strong>3');
3745
});
46+
47+
it('should show a warning when this component is being used unsafely', function() {
48+
highlightFilter('<i>before</i> match after', 'match');
49+
expect(logSpy).toHaveBeenCalled();
50+
});
51+
3852
});

Diff for: src/typeahead/test/typeahead.spec.js

+10-9
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ describe('typeahead tests', function() {
33
var changeInputValueTo;
44

55
beforeEach(module('ui.bootstrap.typeahead'));
6+
beforeEach(module('ngSanitize'));
67
beforeEach(module('template/typeahead/typeahead-popup.html'));
78
beforeEach(module('template/typeahead/typeahead-match.html'));
89
beforeEach(module(function($compileProvider) {
@@ -470,53 +471,53 @@ describe('typeahead tests', function() {
470471
expect($scope.isNoResults).toBeFalsy();
471472
}));
472473
});
473-
474+
474475
describe('select on exact match', function() {
475476
it('should select on an exact match when set', function() {
476477
$scope.onSelect = jasmine.createSpy('onSelect');
477478
var element = prepareInputEl('<div><input ng-model="result" typeahead-editable="false" typeahead-on-select="onSelect()" typeahead="item for item in source | filter:$viewValue" typeahead-select-on-exact="true"></div>');
478479
var inputEl = findInput(element);
479480

480481
changeInputValueTo(element, 'bar');
481-
482+
482483
expect($scope.result).toEqual('bar');
483484
expect(inputEl.val()).toEqual('bar');
484485
expect(element).toBeClosed();
485486
expect($scope.onSelect).toHaveBeenCalled();
486487
});
487-
488+
488489
it('should not select on an exact match by default', function() {
489490
$scope.onSelect = jasmine.createSpy('onSelect');
490491
var element = prepareInputEl('<div><input ng-model="result" typeahead-editable="false" typeahead-on-select="onSelect()" typeahead="item for item in source | filter:$viewValue"></div>');
491492
var inputEl = findInput(element);
492-
493+
493494
changeInputValueTo(element, 'bar');
494-
495+
495496
expect($scope.result).toBeUndefined();
496497
expect(inputEl.val()).toEqual('bar');
497498
expect($scope.onSelect.calls.any()).toBe(false);
498499
});
499-
500+
500501
it('should not be case sensitive when select on an exact match', function() {
501502
$scope.onSelect = jasmine.createSpy('onSelect');
502503
var element = prepareInputEl('<div><input ng-model="result" typeahead-editable="false" typeahead-on-select="onSelect()" typeahead="item for item in source | filter:$viewValue" typeahead-select-on-exact="true"></div>');
503504
var inputEl = findInput(element);
504505

505506
changeInputValueTo(element, 'BaR');
506-
507+
507508
expect($scope.result).toEqual('bar');
508509
expect(inputEl.val()).toEqual('bar');
509510
expect(element).toBeClosed();
510511
expect($scope.onSelect).toHaveBeenCalled();
511512
});
512-
513+
513514
it('should not auto select when not a match with one potential result left', function() {
514515
$scope.onSelect = jasmine.createSpy('onSelect');
515516
var element = prepareInputEl('<div><input ng-model="result" typeahead-editable="false" typeahead-on-select="onSelect()" typeahead="item for item in source | filter:$viewValue" typeahead-select-on-exact="true"></div>');
516517
var inputEl = findInput(element);
517518

518519
changeInputValueTo(element, 'fo');
519-
520+
520521
expect($scope.result).toBeUndefined();
521522
expect(inputEl.val()).toEqual('fo');
522523
expect($scope.onSelect.calls.any()).toBe(false);

Diff for: src/typeahead/typeahead.js

+21-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap.bindHtml'])
1+
angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position'])
22

33
/**
44
* A helper service that can parse typeahead's syntax (string provided by users)
@@ -481,12 +481,29 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position', 'ui.bootstrap
481481
};
482482
}])
483483

484-
.filter('typeaheadHighlight', function() {
484+
.filter('typeaheadHighlight', ['$sce', '$injector', '$log', function($sce, $injector, $log) {
485+
486+
var isSanitizePresent;
487+
isSanitizePresent = $injector.has('$sanitize');
488+
485489
function escapeRegexp(queryToEscape) {
490+
// Regex: capture the whole query string and replace it with the string that will be used to match
491+
// the results, for example if the capture is "a" the result will be \a
486492
return queryToEscape.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1');
487493
}
488494

495+
function containsHtml(matchItem) {
496+
return /<.*>/g.test(matchItem);
497+
}
498+
489499
return function(matchItem, query) {
490-
return query ? ('' + matchItem).replace(new RegExp(escapeRegexp(query), 'gi'), '<strong>$&</strong>') : matchItem;
500+
if(!isSanitizePresent && containsHtml(matchItem)) {
501+
$log.warn('Unsafe use of typeahead please use ngSanitize'); // Warn the user about the danger
502+
}
503+
matchItem = query? ('' + matchItem).replace(new RegExp(escapeRegexp(query), 'gi'), '<strong>$&</strong>') : matchItem; // Replaces the capture string with a the same string inside of a "strong" tag
504+
if(!isSanitizePresent) {
505+
matchItem = $sce.trustAsHtml(matchItem); // If $sanitize is not present we pack the string in a $sce object for the ng-bind-html directive
506+
}
507+
return matchItem;
491508
};
492-
});
509+
}]);

Diff for: template/typeahead/typeahead-match.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
<a href tabindex="-1" bind-html-unsafe="match.label | typeaheadHighlight:query"></a>
1+
<a href tabindex="-1" ng-bind-html="match.label | typeaheadHighlight:query"></a>

0 commit comments

Comments
 (0)