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

fix(ngSrcset): ignore undefined ng-srcset directive #14493

Closed
wants to merge 3 commits into from
Closed

fix(ngSrcset): ignore undefined ng-srcset directive #14493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4befa9
fix(ngSrcset): ignore undefined ng-srcset directive
rphv Apr 22, 2016
80ae614
fix(ngSrcset): fix test of undefined data-ng-attr-srcset
rphv Apr 23, 2016
8b6d940
fix(ngSrcset): add test directly on <img> element
rphv Apr 26, 2016
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
2 changes: 1 addition & 1 deletion src/ng/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1447,7 +1447,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
(nodeName === 'img' && key === 'src')) {
// sanitize a[href] and img[src] values
this[key] = value = $$sanitizeUri(value, key === 'src');
} else if (nodeName === 'img' && key === 'srcset') {
} else if (nodeName === 'img' && key === 'srcset' && isDefined(value)) {
// sanitize img[srcset] values
var result = "";

Expand Down
15 changes: 15 additions & 0 deletions test/ng/compileSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9394,6 +9394,21 @@ describe('$compile', function() {

describe('img[srcset] sanitization', function() {

it('should not error if undefined', function() {
module(function() {
directive({
setter: valueFn(function(scope, element, attr) {
attr.$set('srcset', undefined);
expect(attr.srcset).toBeUndefined();
})
});
});
inject(function($rootScope, $compile) {
element = $compile('<img setter></img>')($rootScope);
expect(element.attr('srcset')).toBeUndefined();
});
});

it('should NOT require trusted values for img srcset', inject(function($rootScope, $compile, $sce) {
element = $compile('<img srcset="{{testUrl}}"></img>')($rootScope);
$rootScope.testUrl = 'http://example.com/image.png';
Expand Down
5 changes: 5 additions & 0 deletions test/ng/directive/ngSrcsetSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,10 @@ describe('ngSrcset', function() {
$rootScope.$digest();
expect(element.attr('srcset')).toBe('http://example.com/image1.png 1x,unsafe:javascript:doEvilStuff() 2x');
}));

it('should not throw an error if undefined', inject(function($rootScope, $compile) {
Copy link
Member

Choose a reason for hiding this comment

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

I didn't try it, but I am pretty sure this test passes without the fix.
Can you confirm that's inded tha case and change the test, so it fails without the fix ?

Copy link
Contributor Author

@rphv rphv Apr 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, you're right @gkalpak. That test wasn't testing the fix.

I've updated the test and verified that it fails without the fix.

element = $compile('<img ng-attr-srcset="{{undefined}}">')($rootScope);
$rootScope.$digest();
}));
});