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

Commit

Permalink
fix(ngSanitize): follow HTML parser rules for start tags / allow < in…
Browse files Browse the repository at this point in the history
… text content

ngSanitize will now permit opening braces in text content, provided they are not followed by either
an unescaped backslash, or by an ASCII letter (u+0041 - u+005A, u+0061 - u+007A), in compliance with
rules of the parsing spec, without taking insertion mode into account.

BREAKING CHANGE

Previously, $sanitize would "fix" invalid markup in which a space preceded alphanumeric characters
in a start-tag. Following this change, any opening angle bracket which is not followed by either a
forward slash, or by an ASCII letter (a-z | A-Z) will not be considered a start tag delimiter, per
the HTML parsing spec (http://www.whatwg.org/specs/web-apps/current-work/multipage/parsing.html).

Closes #8212
Closes #8193
  • Loading branch information
caitp committed Jul 16, 2014
1 parent 6ce5a04 commit 36d2658
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 15 deletions.
22 changes: 15 additions & 7 deletions src/ngSanitize/sanitize.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ function sanitizeText(chars) {

// Regular Expressions for parsing tags and attributes
var START_TAG_REGEXP =
/^<\s*([\w:-]+)((?:\s+[\w:-]+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|[^>\s]+))?)*)\s*(\/?)\s*>/,
END_TAG_REGEXP = /^<\s*\/\s*([\w:-]+)[^>]*>/,
/^<((?:[a-zA-Z])[\w:-]*)((?:\s+[\w:-]+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|[^>\s]+))?)*)\s*(\/?)\s*(>?)/,
END_TAG_REGEXP = /^<\/\s*([\w:-]+)[^>]*>/,
ATTR_REGEXP = /([\w:-]+)(?:\s*=\s*(?:(?:"((?:[^"])*)")|(?:'((?:[^'])*)')|([^>\s]+)))?/g,
BEGIN_TAG_REGEXP = /^</,
BEGING_END_TAGE_REGEXP = /^<\s*\//,
BEGING_END_TAGE_REGEXP = /^<\//,
COMMENT_REGEXP = /<!--(.*?)-->/g,
DOCTYPE_REGEXP = /<!DOCTYPE([^>]*?)>/i,
CDATA_REGEXP = /<!\[CDATA\[(.*?)]]>/g,
Expand Down Expand Up @@ -232,10 +232,11 @@ function makeMap(str) {
* @param {object} handler
*/
function htmlParser( html, handler ) {
var index, chars, match, stack = [], last = html;
var index, chars, match, stack = [], last = html, text;
stack.last = function() { return stack[ stack.length - 1 ]; };

while ( html ) {
text = '';
chars = true;

// Make sure we're not in a script or style element
Expand Down Expand Up @@ -274,16 +275,23 @@ function htmlParser( html, handler ) {
match = html.match( START_TAG_REGEXP );

if ( match ) {
html = html.substring( match[0].length );
match[0].replace( START_TAG_REGEXP, parseStartTag );
// We only have a valid start-tag if there is a '>'.
if ( match[4] ) {
html = html.substring( match[0].length );
match[0].replace( START_TAG_REGEXP, parseStartTag );
}
chars = false;
} else {
// no ending tag found --- this piece should be encoded as an entity.
text += '<';
html = html.substring(1);
}
}

if ( chars ) {
index = html.indexOf("<");

var text = index < 0 ? html : html.substring( 0, index );
text += index < 0 ? html : html.substring( 0, index );
html = index < 0 ? "" : html.substring( index );

if (handler.chars) handler.chars( decodeEntities(text) );
Expand Down
49 changes: 41 additions & 8 deletions test/ngSanitize/sanitizeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('HTML', function() {

var handler, start, text, comment;
beforeEach(function() {
text = "";
handler = {
start: function(tag, attrs, unary){
start = {
Expand All @@ -35,7 +36,7 @@ describe('HTML', function() {
});
},
chars: function(text_){
text = text_;
text += text_;
},
end:function(tag) {
expect(tag).toEqual(start.tag);
Expand Down Expand Up @@ -81,8 +82,31 @@ describe('HTML', function() {
expect(text).toEqual('text');
});

it('should not treat "<" followed by a non-/ or non-letter as a tag', function() {
expectHTML('<- text1 text2 <1 text1 text2 <{', handler).
toBe('&lt;- text1 text2 &lt;1 text1 text2 &lt;{');
});

it('should throw badparse if text content contains "<" followed by "/" without matching ">"', function() {
expect(function() {
htmlParser('foo </ bar', handler);
}).toThrowMinErr('$sanitize', 'badparse', 'The sanitizer was unable to parse the following block of html: </ bar');
});

it('should throw badparse if text content contains "<" followed by an ASCII letter without matching ">"', function() {
expect(function() {
htmlParser('foo <a bar', handler);
}).toThrowMinErr('$sanitize', 'badparse', 'The sanitizer was unable to parse the following block of html: <a bar');
});

it('should accept tag delimiters such as "<" inside real tags', function() {
// Assert that the < is part of the text node content, and not part of a tag name.
htmlParser('<p> 10 < 100 </p>', handler);
expect(text).toEqual(' 10 < 100 ');
});

it('should parse newlines in tags', function() {
htmlParser('<\ntag\n attr="value"\n>text<\n/\ntag\n>', handler);
htmlParser('<tag\n attr="value"\n>text</\ntag\n>', handler);
expect(start).toEqual({tag:'tag', attrs:{attr:'value'}, unary:false});
expect(text).toEqual('text');
});
Expand Down Expand Up @@ -123,8 +147,9 @@ describe('HTML', function() {
expectHTML('a<!DocTyPe html>c.').toEqual('ac.');
});

it('should remove nested script', function() {
expectHTML('a< SCRIPT >A< SCRIPT >evil< / scrIpt >B< / scrIpt >c.').toEqual('ac.');
it('should escape non-start tags', function() {
expectHTML('a< SCRIPT >A< SCRIPT >evil< / scrIpt >B< / scrIpt >c.').
toBe('a&lt; SCRIPT &gt;A&lt; SCRIPT &gt;evil&lt; / scrIpt &gt;B&lt; / scrIpt &gt;c.');
});

it('should remove attrs', function() {
Expand Down Expand Up @@ -165,14 +190,16 @@ describe('HTML', function() {
expectHTML(everything).toEqual(everything);
});

it('should handle improper html', function() {
it('should mangle improper html', function() {
// This text is encoded more than a real HTML parser would, but it should render the same.
expectHTML('< div rel="</div>" alt=abc dir=\'"\' >text< /div>').
toEqual('<div rel="&lt;/div&gt;" alt="abc" dir="&#34;">text</div>');
toBe('&lt; div rel=&#34;&#34; alt=abc dir=\'&#34;\' &gt;text&lt; /div&gt;');
});

it('should handle improper html2', function() {
it('should mangle improper html2', function() {
// A proper HTML parser would clobber this more in most cases, but it looks reasonable.
expectHTML('< div rel="</div>" / >').
toEqual('<div rel="&lt;/div&gt;"/>');
toBe('&lt; div rel=&#34;&#34; / &gt;');
});

it('should ignore back slash as escape', function() {
Expand All @@ -195,6 +222,12 @@ describe('HTML', function() {
expectHTML('\na\n').toEqual('&#10;a&#10;');
});

it('should accept tag delimiters such as "<" inside real tags (with nesting)', function() {
//this is an integrated version of the 'should accept tag delimiters such as "<" inside real tags' test
expectHTML('<p> 10 < <span>100</span> </p>')
.toEqual('<p> 10 &lt; <span>100</span> </p>');
});

describe('htmlSanitizerWriter', function() {
/* global htmlSanitizeWriter: false */
if (angular.isUndefined(window.htmlSanitizeWriter)) return;
Expand Down

0 comments on commit 36d2658

Please sign in to comment.