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

Commit 0b39333

Browse files
lgalfasolgalfaso
committed
fix(sanitize): Do not duplicate self closing elements
For HTML self closing elements: * Do not create an end element * Create the element as self closed
1 parent edb304d commit 0b39333

File tree

3 files changed

+25
-18
lines changed

3 files changed

+25
-18
lines changed

‎src/ngSanitize/sanitize.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,9 @@ var svgElements = toMap("circle,defs,desc,ellipse,font-face,font-face-name,font-
189189
// Special Elements (can contain anything)
190190
var specialElements = toMap("script,style");
191191

192+
var selfClosingElements = toMap("area,base,br,col,command,embed,hr,img,input,keygen,link,meta,param,source," +
193+
"track,wbr");
194+
192195
var validElements = angular.extend({},
193196
voidElements,
194197
blockElements,
@@ -283,7 +286,8 @@ function htmlParser(html, handler) {
283286
while (node) {
284287
switch (node.nodeType) {
285288
case 1: // ELEMENT_NODE
286-
handler.start(node.nodeName.toLowerCase(), attrToMap(node.attributes));
289+
handler.start(node.nodeName.toLowerCase(), attrToMap(node.attributes),
290+
selfClosingElements[node.nodeName.toLowerCase()]);
287291
break;
288292
case 3: // TEXT NODE
289293
handler.chars(node.textContent);
@@ -295,18 +299,18 @@ function htmlParser(html, handler) {
295299
var nextNode;
296300
if (!(nextNode = node.firstChild)) {
297301
if (nextNode = node.nextSibling) {
298-
if (node.nodeType == 1) {
302+
if (node.nodeType == 1 && !selfClosingElements[node.nodeName.toLowerCase()]) {
299303
handler.end(node.nodeName.toLowerCase());
300304
}
301305
} else {
302-
if (node.nodeType == 1) {
306+
if (node.nodeType == 1 && !selfClosingElements[node.nodeName.toLowerCase()]) {
303307
handler.end(node.nodeName.toLowerCase());
304308
}
305309
while (nextNode == null) {
306310
node = node.parentNode;
307311
if (node === baseNode) break;
308312
nextNode = node.nextSibling;
309-
if (node.nodeType == 1) {
313+
if (node.nodeType == 1 && !selfClosingElements[node.nodeName.toLowerCase()]) {
310314
handler.end(node.nodeName.toLowerCase());
311315
}
312316
}
@@ -370,7 +374,7 @@ function encodeEntities(value) {
370374
* create an HTML/XML writer which writes to buffer
371375
* @param {Array} buf use buf.jain('') to get out sanitized html string
372376
* @returns {object} in the form of {
373-
* start: function(tag, attrs) {},
377+
* start: function(tag, attrs, selfClose) {},
374378
* end: function(tag) {},
375379
* chars: function(text) {},
376380
* comment: function(text) {}
@@ -380,15 +384,16 @@ function htmlSanitizeWriter(buf, uriValidator) {
380384
var ignore = false;
381385
var out = angular.bind(buf, buf.push);
382386
return {
383-
start: function(tag, attrs) {
387+
start: function(tag, attrs, selfClose) {
384388
tag = angular.lowercase(tag);
385389
if (!ignore && specialElements[tag]) {
386390
ignore = tag;
387391
}
388392
if (!ignore && validElements[tag] === true) {
389393
out('<');
390394
out(tag);
391-
angular.forEach(attrs, function(value, key) {
395+
angular.forEach(Object.keys(attrs).sort(), function(key) {
396+
var value = attrs[key];
392397
var lkey=angular.lowercase(key);
393398
var isImage = (tag === 'img' && lkey === 'src') || (lkey === 'background');
394399
if (validAttrs[lkey] === true &&
@@ -400,7 +405,7 @@ function htmlSanitizeWriter(buf, uriValidator) {
400405
out('"');
401406
}
402407
});
403-
out('>');
408+
out(selfClose ? '/>' : '>');
404409
}
405410
},
406411
end: function(tag) {

‎test/ngSanitize/filter/linkySpec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ describe('linky', function() {
5050

5151
it('should handle target:', function() {
5252
expect(linky("http://example.com", "_blank")).
53-
toEqual('<a target="_blank" href="http://example.com">http://example.com</a>');
53+
toEqual('<a href="http://example.com" target="_blank">http://example.com</a>');
5454
expect(linky("http://example.com", "someNamedIFrame")).
55-
toEqual('<a target="someNamedIFrame" href="http://example.com">http://example.com</a>');
55+
toEqual('<a href="http://example.com" target="someNamedIFrame">http://example.com</a>');
5656
});
5757
});

‎test/ngSanitize/sanitizeSpec.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('HTML', function() {
112112
// THESE TESTS ARE EXECUTED WITH COMPILED ANGULAR
113113
it('should echo html', function() {
114114
expectHTML('hello<b class="1\'23" align=\'""\'>world</b>.').
115-
toEqual('hello<b class="1\'23" align="&#34;&#34;">world</b>.');
115+
toEqual('hello<b align="&#34;&#34;" class="1\'23">world</b>.');
116116
});
117117

118118
it('should remove script', function() {
@@ -165,7 +165,9 @@ describe('HTML', function() {
165165
});
166166

167167
it('should handle self closed elements', function() {
168-
expectHTML('a<hr/>c').toEqual('a<hr></hr>c');
168+
expectHTML('a<hr/>c').toEqual('a<hr/>c');
169+
expectHTML('<br/>').toEqual('<br/>');
170+
expectHTML('<br>').toEqual('<br/>');
169171
});
170172

171173
it('should handle namespace', function() {
@@ -192,7 +194,7 @@ describe('HTML', function() {
192194

193195
it('should ignore back slash as escape', function() {
194196
expectHTML('<img alt="xxx\\" title="><script>....">').
195-
toEqual('<img alt="xxx\\" title="&gt;&lt;script&gt;...."></img>');
197+
toEqual('<img alt="xxx\\" title="&gt;&lt;script&gt;...."/>');
196198
});
197199

198200
it('should ignore object attributes', function() {
@@ -227,8 +229,8 @@ describe('HTML', function() {
227229
});
228230

229231
it('should accept SVG tags', function() {
230-
expectHTML('<svg width="400px" height="150px" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red"></svg>')
231-
.toEqual('<svg width="400px" height="150px" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red"></circle></svg>');
232+
expectHTML('<svg width="400px" height="150px" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red"></svg>')
233+
.toEqual('<svg height="150px" width="400px" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" fill="red" r="40" stroke="black" stroke-width="3"></circle></svg>');
232234
});
233235

234236
it('should not ignore white-listed svg camelCased attributes', function() {
@@ -259,7 +261,7 @@ describe('HTML', function() {
259261

260262
expectHTML('<svg><a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="?"><circle r="400"></circle>' +
261263
'<animate attributeName="xlink:href" begin="0" from="javascript:alert(1)" to="&" /></a></svg>')
262-
.toEqual('<svg><a xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="?"><circle r="400"></circle></a></svg>');
264+
.toEqual('<svg><a xlink:href="?" xmlns:xlink="http://www.w3.org/1999/xlink"><circle r="400"></circle></a></svg>');
263265
});
264266

265267
describe('htmlSanitizerWriter', function() {
@@ -415,11 +417,11 @@ describe('HTML', function() {
415417
inject(function() {
416418
$$sanitizeUri.andReturn('someUri');
417419

418-
expectHTML('<img src="someUri"/>').toEqual('<img src="someUri"></img>');
420+
expectHTML('<img src="someUri"/>').toEqual('<img src="someUri"/>');
419421
expect($$sanitizeUri).toHaveBeenCalledWith('someUri', true);
420422

421423
$$sanitizeUri.andReturn('unsafe:someUri');
422-
expectHTML('<img src="someUri"/>').toEqual('<img></img>');
424+
expectHTML('<img src="someUri"/>').toEqual('<img/>');
423425
});
424426
});
425427

0 commit comments

Comments
 (0)