Skip to content

Commit ddd3e37

Browse files
Merge pull request #33 from angular/master
Update upstream
2 parents a413be6 + 7673ca7 commit ddd3e37

File tree

4 files changed

+136
-30
lines changed

4 files changed

+136
-30
lines changed

docs/content/error/$compile/iscp.ngdoc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@ myModule.directive('directiveName', function factory() {
1010
return {
1111
...
1212
scope: {
13-
'attrName': '@', // OK
14-
'attrName2': '=localName', // OK
15-
'attrName3': '<?localName', // OK
16-
'attrName4': ' = name', // OK
17-
'attrName5': 'name', // ERROR: missing mode @&=
18-
'attrName6': 'name=', // ERROR: must be prefixed with @&=
19-
'attrName7': '=name?', // ERROR: ? must come directly after the mode
13+
'localName': '@', // OK
14+
'localName2': '&attr', // OK
15+
'localName3': '<?attr', // OK
16+
'localName4': ' = attr', // OK
17+
'localName5': ' =*attr', // OK
18+
'localName6': 'attr', // ERROR: missing mode @&=<
19+
'localName7': 'attr=', // ERROR: must be prefixed with @&=<
20+
'localName8': '=attr?', // ERROR: ? must come directly after the mode
21+
'localName9': '<*' // ERROR: * is only valid with =
2022
}
2123
...
2224
}

src/ng/compile.js

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -303,21 +303,22 @@
303303
* name. Given `<my-component my-attr="parentModel">` and the isolate scope definition `scope: {
304304
* localModel: '=myAttr' }`, the property `localModel` on the directive's scope will reflect the
305305
* value of `parentModel` on the parent scope. Changes to `parentModel` will be reflected in
306-
* `localModel` and vice versa. Optional attributes should be marked as such with a question mark:
307-
* `=?` or `=?attr`. If the binding expression is non-assignable, or if the attribute isn't
308-
* optional and doesn't exist, an exception ({@link error/$compile/nonassign `$compile:nonassign`})
309-
* will be thrown upon discovering changes to the local value, since it will be impossible to sync
310-
* them back to the parent scope. By default, the {@link ng.$rootScope.Scope#$watch `$watch`}
306+
* `localModel` and vice versa. If the binding expression is non-assignable, or if the attribute
307+
* isn't optional and doesn't exist, an exception
308+
* ({@link error/$compile/nonassign `$compile:nonassign`}) will be thrown upon discovering changes
309+
* to the local value, since it will be impossible to sync them back to the parent scope.
310+
*
311+
* By default, the {@link ng.$rootScope.Scope#$watch `$watch`}
311312
* method is used for tracking changes, and the equality check is based on object identity.
312313
* However, if an object literal or an array literal is passed as the binding expression, the
313314
* equality check is done by value (using the {@link angular.equals} function). It's also possible
314315
* to watch the evaluated value shallowly with {@link ng.$rootScope.Scope#$watchCollection
315-
* `$watchCollection`}: use `=*` or `=*attr` (`=*?` or `=*?attr` if the attribute is optional).
316+
* `$watchCollection`}: use `=*` or `=*attr`
316317
*
317318
* * `<` or `<attr` - set up a one-way (one-directional) binding between a local scope property and an
318319
* expression passed via the attribute `attr`. The expression is evaluated in the context of the
319320
* parent scope. If no `attr` name is specified then the attribute name is assumed to be the same as the
320-
* local name. You can also make the binding optional by adding `?`: `<?` or `<?attr`.
321+
* local name.
321322
*
322323
* For example, given `<my-component my-attr="parentModel">` and directive definition of
323324
* `scope: { localModel:'<myAttr' }`, then the isolated scope property `localModel` will reflect the
@@ -347,6 +348,36 @@
347348
* and values into the expression wrapper fn. For example, if the expression is `increment(amount)`
348349
* then we can specify the amount value by calling the `localFn` as `localFn({amount: 22})`.
349350
*
351+
* All 4 kinds of bindings (`@`, `=`, `<`, and `&`) can be made optional by adding `?` to the expression.
352+
* The marker must come after the mode and before the attribute name.
353+
* See the {@link error/$compile/iscp Invalid Isolate Scope Definition error} for definition examples.
354+
* This is useful to refine the interface directives provide.
355+
* One subtle difference between optional and non-optional happens **when the binding attribute is not
356+
* set**:
357+
* - the binding is optional: the property will not be defined
358+
* - the binding is not optional: the property is defined
359+
*
360+
* ```js
361+
*app.directive('testDir', function() {
362+
return {
363+
scope: {
364+
notoptional: '=',
365+
optional: '=?',
366+
},
367+
bindToController: true,
368+
controller: function() {
369+
this.$onInit = function() {
370+
console.log(this.hasOwnProperty('notoptional')) // true
371+
console.log(this.hasOwnProperty('optional')) // false
372+
}
373+
}
374+
}
375+
})
376+
*```
377+
*
378+
*
379+
* ##### Combining directives with different scope defintions
380+
*
350381
* In general it's possible to apply more than one directive to one element, but there might be limitations
351382
* depending on the type of scope required by the directives. The following points will help explain these limitations.
352383
* For simplicity only two directives are taken into account, but it is also applicable for several directives:

src/ngSanitize/sanitize.js

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -313,16 +313,78 @@ function $SanitizeProvider() {
313313
return obj;
314314
}
315315

316-
var inertBodyElement = (function(window) {
317-
var doc;
318-
if (window.document && window.document.implementation) {
319-
doc = window.document.implementation.createHTMLDocument('inert');
316+
/**
317+
* Create an inert document that contains the dirty HTML that needs sanitizing
318+
* Depending upon browser support we use one of three strategies for doing this.
319+
* Support: Safari 10.x -> XHR strategy
320+
* Support: Firefox -> DomParser strategy
321+
*/
322+
var getInertBodyElement /* function(html: string): HTMLBodyElement */ = (function(window, document) {
323+
var inertDocument;
324+
if (document && document.implementation) {
325+
inertDocument = document.implementation.createHTMLDocument('inert');
320326
} else {
321327
throw $sanitizeMinErr('noinert', 'Can\'t create an inert html document');
322328
}
323-
var docElement = doc.documentElement || doc.getDocumentElement();
324-
return docElement.getElementsByTagName('body')[0];
325-
})(window);
329+
var inertBodyElement = (inertDocument.documentElement || inertDocument.getDocumentElement()).querySelector('body');
330+
331+
// Check for the Safari 10.1 bug - which allows JS to run inside the SVG G element
332+
inertBodyElement.innerHTML = '<svg><g onload="this.parentNode.remove()"></g></svg>';
333+
if (!inertBodyElement.querySelector('svg')) {
334+
return getInertBodyElement_XHR;
335+
} else {
336+
// Check for the Firefox bug - which prevents the inner img JS from being sanitized
337+
inertBodyElement.innerHTML = '<svg><p><style><img src="</style><img src=x onerror=alert(1)//">';
338+
if (inertBodyElement.querySelector('svg img')) {
339+
return getInertBodyElement_DOMParser;
340+
} else {
341+
return getInertBodyElement_InertDocument;
342+
}
343+
}
344+
345+
function getInertBodyElement_XHR(html) {
346+
// We add this dummy element to ensure that the rest of the content is parsed as expected
347+
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the `<head>` tag.
348+
html = '<remove></remove>' + html;
349+
try {
350+
html = encodeURI(html);
351+
} catch (e) {
352+
return undefined;
353+
}
354+
var xhr = new window.XMLHttpRequest();
355+
xhr.responseType = 'document';
356+
xhr.open('GET', 'data:text/html;charset=utf-8,' + html, false);
357+
xhr.send(null);
358+
var body = xhr.response.body;
359+
body.firstChild.remove();
360+
return body;
361+
}
362+
363+
function getInertBodyElement_DOMParser(html) {
364+
// We add this dummy element to ensure that the rest of the content is parsed as expected
365+
// e.g. leading whitespace is maintained and tags like `<meta>` do not get hoisted to the `<head>` tag.
366+
html = '<remove></remove>' + html;
367+
try {
368+
var body = new window.DOMParser().parseFromString(html, 'text/html').body;
369+
body.firstChild.remove();
370+
return body;
371+
} catch (e) {
372+
return undefined;
373+
}
374+
}
375+
376+
function getInertBodyElement_InertDocument(html) {
377+
inertBodyElement.innerHTML = html;
378+
379+
// Support: IE 9-11 only
380+
// strip custom-namespaced attributes on IE<=11
381+
if (document.documentMode) {
382+
stripCustomNsAttrs(inertBodyElement);
383+
}
384+
385+
return inertBodyElement;
386+
}
387+
})(window, window.document);
326388

327389
/**
328390
* @example
@@ -342,7 +404,9 @@ function $SanitizeProvider() {
342404
} else if (typeof html !== 'string') {
343405
html = '' + html;
344406
}
345-
inertBodyElement.innerHTML = html;
407+
408+
var inertBodyElement = getInertBodyElement(html);
409+
if (!inertBodyElement) return '';
346410

347411
//mXSS protection
348412
var mXSSAttempts = 5;
@@ -352,13 +416,9 @@ function $SanitizeProvider() {
352416
}
353417
mXSSAttempts--;
354418

355-
// Support: IE 9-11 only
356-
// strip custom-namespaced attributes on IE<=11
357-
if (window.document.documentMode) {
358-
stripCustomNsAttrs(inertBodyElement);
359-
}
360-
html = inertBodyElement.innerHTML; //trigger mXSS
361-
inertBodyElement.innerHTML = html;
419+
// trigger mXSS if it is going to happen by reading and writing the innerHTML
420+
html = inertBodyElement.innerHTML;
421+
inertBodyElement = getInertBodyElement(html);
362422
} while (html !== inertBodyElement.innerHTML);
363423

364424
var node = inertBodyElement.firstChild;

test/ngSanitize/sanitizeSpec.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ describe('HTML', function() {
4949
comment = comment_;
5050
}
5151
};
52+
// Trigger the $sanitizer provider to execute, which initializes the `htmlParser` function.
53+
inject(function($sanitize) {});
5254
});
5355

5456
it('should not parse comments', function() {
@@ -266,14 +268,25 @@ describe('HTML', function() {
266268
});
267269
});
268270

271+
// See https://github.com/cure53/DOMPurify/blob/a992d3a75031cb8bb032e5ea8399ba972bdf9a65/src/purify.js#L439-L449
272+
it('should not allow JavaScript execution when creating inert document', inject(function($sanitize) {
273+
var doc = $sanitize('<svg><g onload="window.xxx = 100"></g></svg>');
274+
expect(window.xxx).toBe(undefined);
275+
delete window.xxx;
276+
}));
277+
278+
// See https://github.com/cure53/DOMPurify/releases/tag/0.6.7
279+
it('should not allow JavaScript hidden in badly formed HTML to get through sanitization (Firefox bug)', inject(function($sanitize) {
280+
var doc = $sanitize('<svg><p><style><img src="</style><img src=x onerror=alert(1)//">');
281+
expect(doc).toEqual('<p><img src="x"></p>');
282+
}));
269283

270284
describe('SVG support', function() {
271285

272286
beforeEach(module(function($sanitizeProvider) {
273287
$sanitizeProvider.enableSvg(true);
274288
}));
275289

276-
277290
it('should accept SVG tags', function() {
278291
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>')
279292
.toBeOneOf('<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>',

0 commit comments

Comments
 (0)