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

Commit b4d5582

Browse files
author
rjamet
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 68bfbf9 commit b4d5582

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
@@ -1321,6 +1321,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
13211321

13221322
nodeName = nodeName_(this.$$element);
13231323

1324+
13241325
// img[srcset] is a bit too weird of a beast to handle through $sce.
13251326
// Instead, for now at least, sanitize each of the URIs individually.
13261327
// That works even dynamically, but it's not bypassable through the $sce.
@@ -2788,12 +2789,11 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
27882789
(tag != "img" && (attrNormalizedName == "src" ||
27892790
attrNormalizedName == "ngSrc"))) {
27902791
return $sce.RESOURCE_URL;
2791-
} else if (attrNormalizedName == "src" ||
2792-
attrNormalizedName == "href" ||
2793-
attrNormalizedName == "ngHref" ||
2794-
attrNormalizedName == "ngSrc") {
2795-
// All that is not a RESOURCE_URL but still a URL. This might be overkill for some
2796-
// attributes, but the sanitization is permissive, so it should not be troublesome.
2792+
} else if (tag == "img" && (attrNormalizedName == "src" ||
2793+
attrNormalizedName == "ngSrc") ||
2794+
tag == "a" && (attrNormalizedName == "href" ||
2795+
attrNormalizedName == "xlinkHref" ||
2796+
attrNormalizedName == "ngHref")) {
27972797
return $sce.URL;
27982798
}
27992799
}

‎test/ng/compileSpec.js

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

88468846
describe('img[src] sanitization', function() {
88478847

8848-
it('should NOT require trusted values for img src', inject(function($rootScope, $compile, $sce) {
8849-
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
8850-
$rootScope.testUrl = 'http://example.com/image.png';
8851-
$rootScope.$digest();
8852-
expect(element.attr('src')).toEqual('http://example.com/image.png');
8853-
// But it should accept trusted values anyway.
8854-
$rootScope.testUrl = $sce.trustAsUrl('http://example.com/image2.png');
8855-
$rootScope.$digest();
8856-
expect(element.attr('src')).toEqual('http://example.com/image2.png');
8857-
}));
8858-
8859-
it('should accept trusted values for img src', inject(function($rootScope, $compile, $sce) {
8860-
/* jshint scripturl:true */
8861-
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
8862-
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
8863-
$rootScope.$digest();
8864-
expect(element.attr('src')).toEqual('javascript:foo();');
8865-
}));
8866-
8867-
it('should sanitize concatenated trusted values for img src', inject(function($rootScope, $compile, $sce) {
8868-
/* jshint scripturl:true */
8869-
element = $compile('<img src="{{testUrl}}ponies"></img>')($rootScope);
8870-
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
8871-
$rootScope.$digest();
8872-
expect(element.attr('src')).toEqual('unsafe:javascript:foo();ponies');
8873-
8874-
element = $compile('<img src="http://{{testUrl}}"></img>')($rootScope);
8875-
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
8876-
$rootScope.$digest();
8877-
expect(element.attr('src')).toEqual('http://javascript:foo();');
8878-
}));
8879-
8880-
88818848
it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {
88828849
/* jshint scripturl:true */
88838850
element = $compile('<img title="{{testUrl}}"></img>')($rootScope);
@@ -8936,6 +8903,45 @@ describe('$compile', function() {
89368903
expect($$sanitizeUri).toHaveBeenCalledWith($rootScope.testUrl);
89378904
});
89388905
});
8906+
8907+
8908+
it('should sanitize concatenated trusted values', function() {
8909+
/* jshint scripturl:true */
8910+
var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
8911+
module(function($provide) {
8912+
$provide.value('$$sanitizeUri', $$sanitizeUri);
8913+
});
8914+
inject(function($compile, $rootScope, $sce) {
8915+
$$sanitizeUri.and.returnValue('someSanitizedUrl');
8916+
element = $compile('<img src="{{testUrl}}ponies"></img>')($rootScope);
8917+
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
8918+
$rootScope.$digest();
8919+
expect(element.attr('src')).toEqual('someSanitizedUrl');
8920+
8921+
element = $compile('<img src="http://{{testUrl}}"></img>')($rootScope);
8922+
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
8923+
$rootScope.$digest();
8924+
expect(element.attr('src')).toEqual('someSanitizedUrl');
8925+
});
8926+
});
8927+
8928+
it('should not use $$sanitizeUri with trusted values', function() {
8929+
/* jshint scripturl:true */
8930+
var $$sanitizeUri = jasmine.createSpy('$$sanitizeUri');
8931+
module(function($provide) {
8932+
$provide.value('$$sanitizeUri', $$sanitizeUri);
8933+
});
8934+
inject(function($compile, $rootScope, $sce) {
8935+
8936+
element = $compile('<img src="{{testUrl}}"></img>')($rootScope);
8937+
$rootScope.testUrl = $sce.trustAsUrl('javascript:foo();');
8938+
8939+
$$sanitizeUri.and.throwError('Should not have been called');
8940+
$rootScope.$apply();
8941+
8942+
expect(element.attr('src')).toEqual('javascript:foo();');
8943+
});
8944+
});
89398945
});
89408946

89418947
describe('img[srcset] sanitization', function() {
@@ -8984,12 +8990,12 @@ describe('$compile', function() {
89848990

89858991
element = $compile('<img srcset="{{testUrl}}, {{testUrl}}"></img>')($rootScope);
89868992
$rootScope.testUrl = 'javascript:yay';
8987-
$rootScope.$digest();
8993+
$rootScope.$apply();
89888994
expect(element.attr('srcset')).toEqual('someSanitizedUrl ,someSanitizedUrl');
89898995

89908996
element = $compile('<img srcset="java{{testUrl}}"></img>')($rootScope);
89918997
$rootScope.testUrl = 'script:yay, javascript:nay';
8992-
$rootScope.$digest();
8998+
$rootScope.$apply();
89938999
expect(element.attr('srcset')).toEqual('someSanitizedUrl ,someSanitizedUrl');
89949000
});
89959001
});

‎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)