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

Commit 21a02e7

Browse files
rjametrjamet
committed
fix(*): Move closer to the old URL sanitization plus misc fixes in tests.
Only apply the $sce.URL context where there was previously sanitization, so that the final result matches more closely the old behaviors. Also, fix a few minor mistakes in tests (mismatching tags, wrong expected errors, differences with the surrounding testing style), and mock out $$sanitizeUri where it makes sense.
1 parent c0beaad commit 21a02e7

File tree

4 files changed

+58
-53
lines changed

4 files changed

+58
-53
lines changed

‎src/ng/compile.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,6 +1429,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
14291429

14301430
nodeName = nodeName_(this.$$element);
14311431

1432+
14321433
// img[srcset] is a bit too weird of a beast to handle through $sce.
14331434
// Instead, for now at least, sanitize each of the URIs individually.
14341435
// That works even dynamically, but it's not bypassable through the $sce.
@@ -2929,12 +2930,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
29292930
(tag !== "img" && (attrNormalizedName === "src" ||
29302931
attrNormalizedName === "ngSrc"))) {
29312932
return $sce.RESOURCE_URL;
2932-
} else if (attrNormalizedName == "src" ||
2933-
attrNormalizedName == "href" ||
2934-
attrNormalizedName == "ngHref" ||
2935-
attrNormalizedName == "ngSrc") {
2936-
// All that is not a RESOURCE_URL but still a URL. This might be overkill for some
2937-
// attributes, but the sanitization is permissive, so it should not be troublesome.
2933+
} else if (tag == "img" && (attrNormalizedName == "src" ||
2934+
attrNormalizedName == "ngSrc") ||
2935+
tag == "a" && (attrNormalizedName == "href" ||
2936+
attrNormalizedName == "xlinkHref" ||
2937+
attrNormalizedName == "ngHref")) {
29382938
return $sce.URL;
29392939
}
29402940
}

‎test/ng/compileSpec.js

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9633,39 +9633,6 @@ describe('$compile', function() {
96339633

96349634
describe('img[src] sanitization', function() {
96359635

9636-
it('should NOT require trusted values for img src', inject(function($rootScope, $compile, $sce) {
9637-
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
9638-
$rootScope.testUrl = 'http://example.com/image.png';
9639-
$rootScope.$digest();
9640-
expect(element.attr('src')).toEqual('http://example.com/image.png');
9641-
// But it should accept trusted values anyway.
9642-
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.png');
9643-
$rootScope.$digest();
9644-
expect(element.attr('src')).toEqual('http://example.com/image2.png');
9645-
}));
9646-
9647-
it('should accept trusted values for img src', inject(function($rootScope, $compile, $sce) {
9648-
/* jshint scripturl:true */
9649-
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
9650-
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
9651-
$rootScope.$digest();
9652-
expect(element.attr('src')).toEqual('javascript:foo();');
9653-
}));
9654-
9655-
it('should sanitize concatenated trusted values for img src', inject(function($rootScope, $compile, $sce) {
9656-
/* jshint scripturl:true */
9657-
element = $compile('<img src="{{testUrl}}ponies"></img>')($rootScope);
9658-
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
9659-
$rootScope.$digest();
9660-
expect(element.attr('src')).toEqual('unsafe:javascript:foo();ponies');
9661-
9662-
element = $compile('<img src="http://{{testUrl}}"></img>')($rootScope);
9663-
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
9664-
$rootScope.$digest();
9665-
expect(element.attr('src')).toEqual('http://javascript:foo();');
9666-
}));
9667-
9668-
96699636
it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {
96709637
/* jshint scripturl:true */
96719638
element = $compile('<img title="{{testUrl}}"></img>')($rootScope);
@@ -9724,6 +9691,45 @@ describe('$compile', function() {
97249691
expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl);
97259692
});
97269693
});
9694+
9695+
9696+
it('should sanitize concatenated trusted values', function() {
9697+
/* jshint scripturl:true */
9698+
var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
9699+
module(function($provide) {
9700+
$provide.value('$$sanitizeUri', $$sanitizeUri);
9701+
});
9702+
inject(function($compile, $rootScope, $sce) {
9703+
$$sanitizeUri.and.returnValue('someSanitizedUrl');
9704+
element = $compile('<img src="{{testUrl}}ponies"></img>')($rootScope);
9705+
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
9706+
$rootScope.$digest();
9707+
expect(element.attr('src')).toEqual('someSanitizedUrl');
9708+
9709+
element = $compile('<img src="http://{{testUrl}}"></img>')($rootScope);
9710+
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
9711+
$rootScope.$digest();
9712+
expect(element.attr('src')).toEqual('someSanitizedUrl');
9713+
});
9714+
});
9715+
9716+
it('should not use $$sanitizeUri with trusted values', function() {
9717+
/* jshint scripturl:true */
9718+
var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
9719+
module(function($provide) {
9720+
$provide.value('$$sanitizeUri', $$sanitizeUri);
9721+
});
9722+
inject(function($compile, $rootScope, $sce) {
9723+
9724+
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
9725+
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
9726+
9727+
$$sanitizeUri.and.throwError('Should not have been called');
9728+
$rootScope.$apply();
9729+
9730+
expect(element.attr('src')).toEqual('javascript:foo();');
9731+
});
9732+
});
97279733
});
97289734

97299735
describe('img[srcset] sanitization', function() {
@@ -9793,12 +9799,12 @@ describe('$compile', function() {
97939799

97949800
element = $compile('<img srcset="{{testUrl}}, {{testUrl}}"></img>')($rootScope);
97959801
$rootScope.testUrl = 'javascript:yay';
9796-
$rootScope.$digest();
9802+
$rootScope.$apply();
97979803
expect(element.attr('srcset')).toEqual('someSanitizedUrl ,someSanitizedUrl');
97989804

97999805
element = $compile('<img srcset="java{{testUrl}}"></img>')($rootScope);
98009806
$rootScope.testUrl = 'script:yay, javascript:nay';
9801-
$rootScope.$digest();
9807+
$rootScope.$apply();
98029808
expect(element.attr('srcset')).toEqual('someSanitizedUrl ,someSanitizedUrl');
98039809
});
98049810
});

‎test/ng/directive/booleanAttrsSpec.js

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ describe('boolean attr directives', function() {
118118
describe('ngSrc', function() {
119119
it('should interpolate the expression and bind to src with raw same-domain value',
120120
inject(function($compile, $rootScope) {
121-
var element = $compile('<img ng-src="{{id}}"></div>')($rootScope);
121+
var element = $compile('<img ng-src="{{id}}"></img>')($rootScope);
122122

123123
$rootScope.$digest();
124124
expect(element.attr('src')).toBeUndefined();
@@ -133,7 +133,7 @@ describe('ngSrc', function() {
133133

134134

135135
it('should interpolate the expression and bind to src with a trusted value', inject(function($compile, $rootScope, $sce) {
136-
var element = $compile('<iframe ng-src="{{id}}"></div>')($rootScope);
136+
var element = $compile('<iframe ng-src="{{id}}"></iframe>')($rootScope);
137137

138138
$rootScope.$digest();
139139
expect(element.attr('src')).toBeUndefined();
@@ -181,7 +181,7 @@ describe('ngSrc', function() {
181181

182182
it('should NOT interpolate a wrongly typed expression', inject(function($compile, $rootScope, $sce) {
183183
expect(function() {
184-
var element = $compile('<iframe ng-src="{{id}}"></div>')($rootScope);
184+
var element = $compile('<iframe ng-src="{{id}}"></iframe>')($rootScope);
185185
$rootScope.$apply(function() {
186186
$rootScope.id = $sce.trustAsUrl('http://somewhere');
187187
});
@@ -198,19 +198,19 @@ describe('ngSrc', function() {
198198
// then calling element.setAttribute('src', 'foo') doesn't do anything, so we need
199199
// to set the property as well to achieve the desired effect
200200

201-
var element = $compile('<img ng-src="{{id}}"></div>')($rootScope);
201+
var element = $compile('<img ng-src="{{id}}"></img>')($rootScope);
202202

203203
$rootScope.$digest();
204204
expect(element.prop('src')).toBeUndefined();
205205
dealoc(element);
206206

207-
element = $compile('<img ng-src="some/"></div>')($rootScope);
207+
element = $compile('<img ng-src="some/"></img>')($rootScope);
208208

209209
$rootScope.$digest();
210210
expect(element.prop('src')).toEqual('some/');
211211
dealoc(element);
212212

213-
element = $compile('<img ng-src="{{id}}"></div>')($rootScope);
213+
element = $compile('<img ng-src="{{id}}"></img>')($rootScope);
214214
$rootScope.$apply(function() {
215215
$rootScope.id = $sce.trustAsResourceUrl('http://somewhere');
216216
});
@@ -296,9 +296,8 @@ describe('ngHref', function() {
296296
// IE11/10/Edge fail when setting a href to a URL containing a % that isn't a valid escape sequence
297297
// See https://github.com/angular/angular.js/issues/13388
298298
it('should throw error if ng-href contains a non-escaped percent symbol', inject(function($rootScope, $compile) {
299-
element = $compile('<a ng-href="http://www.google.com/{{\'a%link\'}}">')($rootScope);
300-
301299
expect(function() {
300+
element = $compile('<a ng-href="http://www.google.com/{{\'a%link\'}}">')($rootScope);
302301
$rootScope.$digest();
303302
}).toThrow();
304303
}));

‎test/ngMessageFormat/messageFormatSpec.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ describe('$$ngMessageFormat', function() {
537537
}).toThrowMinErr(
538538
"$interpolate", "noconcat", "Error while interpolating: {{foo}}{{bar}}\n" +
539539
"Strict Contextual Escaping disallows interpolations that concatenate multiple " +
540-
"expressions when a trusted value is required. See " +
540+
"expressions in some secure contexts. See " +
541541
"http://docs.angularjs.org/api/ng.$sce");
542542
}));
543543
});
@@ -626,19 +626,19 @@ describe('$$ngMessageFormat', function() {
626626
}).toThrowMinErr(
627627
"$interpolate", "noconcat", "Error while interpolating: constant/{{var}}\nStrict " +
628628
"Contextual Escaping disallows interpolations that concatenate multiple expressions " +
629-
"when a trusted value is required. See http://docs.angularjs.org/api/ng.$sce");
629+
"in some secure contexts. See http://docs.angularjs.org/api/ng.$sce");
630630
expect(function() {
631631
$interpolate('{{var}}/constant', true, isTrustedContext);
632632
}).toThrowMinErr(
633633
"$interpolate", "noconcat", "Error while interpolating: {{var}}/constant\nStrict " +
634634
"Contextual Escaping disallows interpolations that concatenate multiple expressions " +
635-
"when a trusted value is required. See http://docs.angularjs.org/api/ng.$sce");
635+
"in some secure contexts. See http://docs.angularjs.org/api/ng.$sce");
636636
expect(function() {
637637
$interpolate('{{foo}}{{bar}}', true, isTrustedContext);
638638
}).toThrowMinErr(
639639
"$interpolate", "noconcat", "Error while interpolating: {{foo}}{{bar}}\nStrict " +
640640
"Contextual Escaping disallows interpolations that concatenate multiple expressions " +
641-
"when a trusted value is required. See http://docs.angularjs.org/api/ng.$sce");
641+
"in some secure contexts. See http://docs.angularjs.org/api/ng.$sce");
642642
}));
643643

644644
it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate) {

0 commit comments

Comments
 (0)