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

Commit

Permalink
fix(typeahead): use ng-bind-html
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
fernando-sendMail authored and wesleycho committed Aug 17, 2015
1 parent aaec2f4 commit bb9fa1a
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 19 deletions.
1 change: 1 addition & 0 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion misc/demo/assets/app.js
Original file line number Diff line number Diff line change
@@ -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){
Expand Down
1 change: 1 addition & 0 deletions misc/demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular.min.js"></script>
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-animate.min.js"></script>
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-touch.min.js"></script>
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-sanitize.js"></script>
<script src="ui-bootstrap-tpls-<%= pkg.version%>.min.js"></script>
<script src="assets/plunker.js"></script>
<script src="assets/app.js"></script>
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 16 additions & 0 deletions src/typeahead/test/typeahead-highlight-ngsanitize.spec.js
Original file line number Diff line number Diff line change
@@ -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 <script src="">match</script> after', 'match');
expect(logSpy).not.toHaveBeenCalled();
});
});
35 changes: 24 additions & 11 deletions src/typeahead/test/typeahead-highlight.spec.js
Original file line number Diff line number Diff line change
@@ -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 <strong>match</strong> after');
expect($sce.getTrustedHtml(highlightFilter('before match after', 'match'))).toEqual('before <strong>match</strong> after');
});

it('should higlight a match with mixed case', function() {
expect(highlightFilter('before MaTch after', 'match')).toEqual('before <strong>MaTch</strong> after');
expect($sce.getTrustedHtml(highlightFilter('before MaTch after', 'match'))).toEqual('before <strong>MaTch</strong> after');
});

it('should higlight all matches', function() {
expect(highlightFilter('before MaTch after match', 'match')).toEqual('before <strong>MaTch</strong> after <strong>match</strong>');
expect($sce.getTrustedHtml(highlightFilter('before MaTch after match', 'match'))).toEqual('before <strong>MaTch</strong> after <strong>match</strong>');
});

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 <strong>(match</strong> after');
expect($sce.getTrustedHtml(highlightFilter('before (match after', '(match'))).toEqual('before <strong>(match</strong> after');
});

it('issue 1777 - should work correctly with numeric values', function() {
expect(highlightFilter(123, '2')).toEqual('1<strong>2</strong>3');
expect($sce.getTrustedHtml(highlightFilter(123, '2'))).toEqual('1<strong>2</strong>3');
});

it('should show a warning when this component is being used unsafely', function() {
highlightFilter('<i>before</i> match after', 'match');
expect(logSpy).toHaveBeenCalled();
});
});
1 change: 1 addition & 0 deletions src/typeahead/test/typeahead.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
26 changes: 20 additions & 6 deletions src/typeahead/typeahead.js
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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'), '<strong>$&</strong>') : 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'), '<strong>$&</strong>') : 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;
};
});
}]);
2 changes: 1 addition & 1 deletion template/typeahead/typeahead-match.html
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<a href tabindex="-1" bind-html-unsafe="match.label | typeaheadHighlight:query"></a>
<a href tabindex="-1" ng-bind-html="match.label | typeaheadHighlight:query"></a>

0 comments on commit bb9fa1a

Please sign in to comment.