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

fix(linky): throw error if input is not a string #13693

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/content/error/linky/notstring.ngdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@ngdoc error
@name linky:notstring
@fullName Not a string
@description

This error occurs when {@link ngSanitize.linky linky} is used with a non-empty, non-string value:
```html
<div ng-bind-html="42 | linky"></div>
```

`linky` is supposed to be used with string values only, and therefore assumes that several methods
(such as `.match()`) are available on the passed in value.
The value can be initialized asynchronously and therefore null or undefined won't throw this error.

If you want to pass non-string values to `linky` (e.g. Objects whose `.toString()` should be
utilized), you need to manually convert them to strings.
7 changes: 6 additions & 1 deletion src/ngSanitize/filter/linky.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,13 @@ angular.module('ngSanitize').filter('linky', ['$sanitize', function($sanitize) {
/((ftp|https?):\/\/|(www\.)|(mailto:)?[A-Za-z0-9._%+-]+@)\S*[^\s.;,(){}<>"\u201d\u2019]/i,
MAILTO_REGEXP = /^mailto:/i;

var linkyMinErr = angular.$$minErr('linky');
var isString = angular.isString;

return function(text, target, attributes) {
if (!text) return text;
if (text == null || text === '') return text;
if (!isString(text)) throw linkyMinErr('notstring', 'Expected string but received: {0}', text);

var match;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am ok one way or another, but should we short-circuit this for the case that text === ''?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well. It is in a separate module and so it doesn't impact on the size of the core angular.js file. It saves a few microseconds per digest if the string is empty as we don't need to run the regex or the sanitizer.

var raw = text;
var html = [];
Expand Down
31 changes: 31 additions & 0 deletions test/ngSanitize/filter/linkySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,37 @@ describe('linky', function() {
expect(linky(undefined)).not.toBeDefined();
});

it('should return `undefined`/`null`/`""` values unchanged', function() {
expect(linky(undefined)).toBe(undefined);
expect(linky(null)).toBe(null);
expect(linky('')).toBe('');
});

it('should throw an error when used with a non-string value (other than `undefined`/`null`)',
function() {
expect(function() { linky(false); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: false');

expect(function() { linky(true); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: true');

expect(function() { linky(0); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: 0');

expect(function() { linky(42); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: 42');

expect(function() { linky({}); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: {}');

expect(function() { linky([]); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: []');

expect(function() { linky(noop); }).
toThrowMinErr('linky', 'notstring', 'Expected string but received: function noop()');
}
);

it('should be case-insensitive', function() {
expect(linky('WWW.example.com')).toEqual('<a href="http://WWW.example.com">WWW.example.com</a>');
expect(linky('WWW.EXAMPLE.COM')).toEqual('<a href="http://WWW.EXAMPLE.COM">WWW.EXAMPLE.COM</a>');
Expand Down