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

Commit 569e906

Browse files
petebacondarwinpetebacondarwin
committed
fix(http): do not allow encoded callback params in jsonp requests
1 parent 18b8a63 commit 569e906

File tree

2 files changed

+77
-18
lines changed

2 files changed

+77
-18
lines changed

‎src/ng/http.js

+28-18
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,7 @@ function $HttpProvider() {
10991099
*
11001100
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11011101
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1102-
* @param {Object=} config Optional configuration object
1102+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11031103
* @returns {HttpPromise} Future object
11041104
*/
11051105

@@ -1112,7 +1112,7 @@ function $HttpProvider() {
11121112
*
11131113
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11141114
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1115-
* @param {Object=} config Optional configuration object
1115+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11161116
* @returns {HttpPromise} Future object
11171117
*/
11181118

@@ -1125,7 +1125,7 @@ function $HttpProvider() {
11251125
*
11261126
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11271127
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1128-
* @param {Object=} config Optional configuration object
1128+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11291129
* @returns {HttpPromise} Future object
11301130
*/
11311131

@@ -1142,6 +1142,10 @@ function $HttpProvider() {
11421142
* {@link $sceDelegateProvider#resourceUrlWhitelist `$sceDelegateProvider.resourceUrlWhitelist`} or
11431143
* by explicitly trusting the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}.
11441144
*
1145+
* You should avoid generating the URL for the JSONP request from user provided data.
1146+
* Provide additional query parameters via `params` property of the `config` parameter, rather than
1147+
* modifying the URL itself.
1148+
*
11451149
* JSONP requests must specify a callback to be used in the response from the server. This callback
11461150
* is passed as a query parameter in the request. You must specify the name of this parameter by
11471151
* setting the `jsonpCallbackParam` property on the request config object.
@@ -1163,7 +1167,7 @@ function $HttpProvider() {
11631167
*
11641168
* @param {string|TrustedObject} url Absolute or relative URL of the resource that is being requested;
11651169
* or an object created by a call to `$sce.trustAsResourceUrl(url)`.
1166-
* @param {Object=} config Optional configuration object
1170+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11671171
* @returns {HttpPromise} Future object
11681172
*/
11691173
createShortMethods('get', 'delete', 'head', 'jsonp');
@@ -1177,7 +1181,7 @@ function $HttpProvider() {
11771181
*
11781182
* @param {string} url Relative or absolute URL specifying the destination of the request
11791183
* @param {*} data Request content
1180-
* @param {Object=} config Optional configuration object
1184+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11811185
* @returns {HttpPromise} Future object
11821186
*/
11831187

@@ -1190,7 +1194,7 @@ function $HttpProvider() {
11901194
*
11911195
* @param {string} url Relative or absolute URL specifying the destination of the request
11921196
* @param {*} data Request content
1193-
* @param {Object=} config Optional configuration object
1197+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
11941198
* @returns {HttpPromise} Future object
11951199
*/
11961200

@@ -1203,7 +1207,7 @@ function $HttpProvider() {
12031207
*
12041208
* @param {string} url Relative or absolute URL specifying the destination of the request
12051209
* @param {*} data Request content
1206-
* @param {Object=} config Optional configuration object
1210+
* @param {Object=} config Optional configuration object. See https://docs.angularjs.org/api/ng/service/$http#usage
12071211
* @returns {HttpPromise} Future object
12081212
*/
12091213
createShortMethodsWithData('post', 'put', 'patch');
@@ -1417,20 +1421,26 @@ function $HttpProvider() {
14171421
return url;
14181422
}
14191423

1420-
function sanitizeJsonpCallbackParam(url, key) {
1421-
if (/[&?][^=]+=JSON_CALLBACK/.test(url)) {
1422-
// Throw if the url already contains a reference to JSON_CALLBACK
1423-
throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url);
1424-
}
1425-
1426-
var callbackParamRegex = new RegExp('[&?]' + key + '=');
1427-
if (callbackParamRegex.test(url)) {
1428-
// Throw if the callback param was already provided
1429-
throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', key, url);
1424+
function sanitizeJsonpCallbackParam(url, cbKey) {
1425+
var parts = url.split('?');
1426+
if (parts.length > 2) {
1427+
// Throw if the url contains more than one `?` query indicator
1428+
throw $httpMinErr('badjsonp', 'Illegal use more than one "?", in url, "{1}"', url);
14301429
}
1430+
var params = parseKeyValue(parts[1]);
1431+
forEach(params, function(value, key) {
1432+
if (value === 'JSON_CALLBACK') {
1433+
// Throw if the url already contains a reference to JSON_CALLBACK
1434+
throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url);
1435+
}
1436+
if (key === cbKey) {
1437+
// Throw if the callback param was already provided
1438+
throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', cbKey, url);
1439+
}
1440+
});
14311441

14321442
// Add in the JSON_CALLBACK callback param value
1433-
url += ((url.indexOf('?') === -1) ? '?' : '&') + key + '=JSON_CALLBACK';
1443+
url += ((url.indexOf('?') === -1) ? '?' : '&') + cbKey + '=JSON_CALLBACK';
14341444

14351445
return url;
14361446
}

‎test/ng/httpSpec.js

+49
Original file line numberDiff line numberDiff line change
@@ -979,18 +979,38 @@ describe('$http', function() {
979979
$http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}});
980980
});
981981

982+
it('should error if the URL contains more than one `?` query indicator', function() {
983+
var error;
984+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?a=b?c=d')})
985+
.catch(function(e) { error = e; });
986+
$rootScope.$digest();
987+
expect(error).toEqualMinErr('$http', 'badjsonp');
988+
});
989+
982990
it('should error if the URL contains a JSON_CALLBACK parameter', function() {
983991
var error;
984992
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_CALLBACK')})
985993
.catch(function(e) { error = e; });
986994
$rootScope.$digest();
987995
expect(error).toEqualMinErr('$http', 'badjsonp');
988996

997+
error = undefined;
998+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_C%41LLBACK')})
999+
.catch(function(e) { error = e; });
1000+
$rootScope.$digest();
1001+
expect(error).toEqualMinErr('$http', 'badjsonp');
1002+
9891003
error = undefined;
9901004
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_CALLBACK')})
9911005
.catch(function(e) { error = e; });
9921006
$rootScope.$digest();
9931007
expect(error).toEqualMinErr('$http', 'badjsonp');
1008+
1009+
error = undefined;
1010+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_C%41LLBACK')})
1011+
.catch(function(e) { error = e; });
1012+
$rootScope.$digest();
1013+
expect(error).toEqualMinErr('$http', 'badjsonp');
9941014
});
9951015

9961016
it('should error if a param contains a JSON_CALLBACK value', function() {
@@ -1007,18 +1027,47 @@ describe('$http', function() {
10071027
expect(error).toEqualMinErr('$http', 'badjsonp');
10081028
});
10091029

1030+
it('should allow encoded params that look like they contain the value JSON_CALLBACK or the configured callback key', function() {
1031+
var error;
1032+
error = undefined;
1033+
$httpBackend.expect('JSONP', 'http://example.org/path?other=JSON_C%2541LLBACK&callback=JSON_CALLBACK').respond('');
1034+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {other: 'JSON_C%41LLBACK'}})
1035+
.catch(function(e) { error = e; });
1036+
$rootScope.$digest();
1037+
expect(error).toBeUndefined();
1038+
1039+
error = undefined;
1040+
$httpBackend.expect('JSONP', 'http://example.org/path?c%2561llback=evilThing&callback=JSON_CALLBACK').respond('');
1041+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {'c%61llback': 'evilThing'}})
1042+
.catch(function(e) { error = e; });
1043+
$rootScope.$digest();
1044+
expect(error).toBeUndefined();
1045+
});
1046+
10101047
it('should error if there is already a param matching the jsonpCallbackParam key', function() {
10111048
var error;
10121049
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {callback: 'evilThing'}})
10131050
.catch(function(e) { error = e; });
10141051
$rootScope.$digest();
10151052
expect(error).toEqualMinErr('$http', 'badjsonp');
10161053

1054+
error = undefined;
1055+
$http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?c%61llback=evilThing')})
1056+
.catch(function(e) { error = e; });
1057+
$rootScope.$digest();
1058+
expect(error).toEqualMinErr('$http', 'badjsonp');
1059+
10171060
error = undefined;
10181061
$http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {cb: 'evilThing'}})
10191062
.catch(function(e) { error = e; });
10201063
$rootScope.$digest();
10211064
expect(error).toEqualMinErr('$http', 'badjsonp');
1065+
1066+
error = undefined;
1067+
$http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path?c%62=evilThing')})
1068+
.catch(function(e) { error = e; });
1069+
$rootScope.$digest();
1070+
expect(error).toEqualMinErr('$http', 'badjsonp');
10221071
});
10231072
});
10241073

0 commit comments

Comments
 (0)