feat($http): JSONP callback must be specified by jsonpCallbackParam config

The query parameter that will be used to transmit the JSONP callback to the
server is now specified via the `jsonpCallbackParam` config value, instead of
using the `JSON_CALLBACK` placeholder.

* Any use of `JSON_CALLBACK` in a JSONP request URL will cause an error.
* Any request that provides a parameter with the same name as that given
by the `jsonpCallbackParam` config property will cause an error.

This is to prevent malicious attack via the response from an app inadvertently
allowing untrusted data to be used to generate the callback parameter.

BREAKING CHANGE

You can no longer use the `JSON_CALLBACK` placeholder in your JSONP requests.
Instead you must provide the name of the query parameter that will pass the
callback via the `jsonpCallbackParam` property of the config object, or app-wide via
the `$http.defaults.jsonpCallbackParam` property, which is `"callback"` by default.

Before this change:

```
$http.json('trusted/url?callback=JSON_CALLBACK');
$http.json('other/trusted/url', {params:cb:'JSON_CALLBACK'});
```

After this change:

```
$http.json('trusted/url');
$http.json('other/trusted/url', {callbackParam:'cb'});
```

Author: petebacondarwin

docs/content/error/$http/badjsonp.ngdoc

@@ -0,0 +1,20 @@
+@ngdoc error
+@name $http:badjsonp
+@fullName Bad JSONP Request Configuration
+@description
+
+This error occurs when the URL generated from the configuration object contains a parameter with the
+same name as the configured `jsonpCallbackParam` property; or when it contains a parameter whose
+value is `JSON_CALLBACK`.
+
+`$http` JSONP requests need to attach a callback query parameter to the URL. The name of this
+parameter is specified in the configuration object (or in the defaults) via the `jsonpCallbackParam`
+property. You must not provide your own parameter with this name in the configuratio of the request.
+
+In previous versions of Angular, you specified where to add the callback parameter value via the
+`JSON_CALLBACK` placeholder. This is no longer allowed.
+
+To resolve this error, remove any parameters that have the same name as the `jsonpCallbackParam`;
+and/or remove any parameters that have a value of `JSON_CALLBACK`.
+
+For more information, see the {@link ng.$http#jsonp `$http.jsonp()`} method API documentation.

docs/content/guide/concepts.ngdoc

@@ -326,7 +326,7 @@ The following example shows how this is done with Angular:
         var YAHOO_FINANCE_URL_PATTERN =
               '//query.yahooapis.com/v1/public/yql?q=select * from ' +
               'yahoo.finance.xchange where pair in ("PAIRS")&format=json&' +
-              'env=store://datatables.org/alltableswithkeys&callback=JSON_CALLBACK';
+              'env=store://datatables.org/alltableswithkeys';
         var currencies = ['USD', 'EUR', 'CNY'];
         var usdToForeignRates = {};
 

src/ng/http.js

@@ -286,6 +286,10 @@ function $HttpProvider() {
    *  If specified as string, it is interpreted as a function registered with the {@link auto.$injector $injector}.
    *  Defaults to {@link ng.$httpParamSerializer $httpParamSerializer}.
    *
+   * - **`defaults.jsonpCallbackParam`** - `{string} - the name of the query parameter that passes the name of the
+   * callback in a JSONP request. The value of this parameter will be replaced with the expression generated by the
+   * {@link $jsonpCallbacks} service. Defaults to `'callback'`.
+   *
    **/
   var defaults = this.defaults = {
     // transform incoming response data
@@ -309,7 +313,9 @@ function $HttpProvider() {
     xsrfCookieName: 'XSRF-TOKEN',
     xsrfHeaderName: 'X-XSRF-TOKEN',
 
-    paramSerializer: '$httpParamSerializer'
+    paramSerializer: '$httpParamSerializer',
+
+    jsonpCallbackParam: 'callback'
   };
 
   var useApplyAsync = false;
@@ -869,11 +875,11 @@ function $HttpProvider() {
     <button id="samplegetbtn" ng-click="updateModel('GET', 'http-hello.html')">Sample GET</button>
     <button id="samplejsonpbtn"
       ng-click="updateModel('JSONP',
-                    'https://angularjs.org/greet.php?callback=JSON_CALLBACK&name=Super%20Hero')">
+                    'https://angularjs.org/greet.php?name=Super%20Hero')">
       Sample JSONP
     </button>
     <button id="invalidjsonpbtn"
-      ng-click="updateModel('JSONP', 'https://angularjs.org/doesntexist&callback=JSON_CALLBACK')">
+      ng-click="updateModel('JSONP', 'https://angularjs.org/doesntexist')">
         Invalid JSONP
       </button>
     <pre>http status code: {{status}}</pre>
@@ -964,7 +970,8 @@ function $HttpProvider() {
         method: 'get',
         transformRequest: defaults.transformRequest,
         transformResponse: defaults.transformResponse,
-        paramSerializer: defaults.paramSerializer
+        paramSerializer: defaults.paramSerializer,
+        jsonpCallbackParam: defaults.jsonpCallbackParam
       }, requestConfig);
 
       config.headers = mergeHeaders(requestConfig);
@@ -1158,11 +1165,27 @@ function $HttpProvider() {
      * @description
      * Shortcut method to perform `JSONP` request.
      *
-     * Note that, since JSONP requests are sensitive because the response is given full acces to the browser,
+     * Note that, since JSONP requests are sensitive because the response is given full access to the browser,
      * the url must be declared, via {@link $sce} as a trusted resource URL.
      * You can trust a URL by adding it to the whitelist via
      * {@link $sceDelegateProvider#resourceUrlWhitelist  `$sceDelegateProvider.resourceUrlWhitelist`} or
-     * by explicitly trusted the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}.
+     * by explicitly trusting the URL via {@link $sce#trustAsResourceUrl `$sce.trustAsResourceUrl(url)`}.
+     *
+     * JSONP requests must specify a callback to be used in the response from the server. This callback
+     * is passed as a query parameter in the request. You must specify the name of this parameter by
+     * setting the `jsonpCallbackParam` property on the request config object.
+     *
+     * ```
+     * $http.jsonp('some/trusted/url', {jsonpCallbackParam: 'callback'})
+     * ```
+     *
+     * You can also specify a default callback parameter name in `$http.defaults.jsonpCallbackParam`.
+     * Initially this is set to `'callback'`.
+     *
+     * <div class="alert alert-danger">
+     * You can no longer use the `JSON_CALLBACK` string as a placeholder for specifying where the callback
+     * parameter value should go.
+     * </div>
      *
      * If you would like to customise where and how the callbacks are stored then try overriding
      * or decorating the {@link $jsonpCallbacks} service.
@@ -1267,9 +1290,10 @@ function $HttpProvider() {
           cache,
           cachedResp,
           reqHeaders = config.headers,
+          isJsonp = lowercase(config.method) === 'jsonp',
           url = config.url;
 
-      if (lowercase(config.method) === 'jsonp') {
+      if (isJsonp) {
         // JSONP is a pretty sensitive operation where we're allowing a script to have full access to
         // our DOM and JS space.  So we require that the URL satisfies SCE.RESOURCE_URL.
         url = $sce.getTrustedResourceUrl(url);
@@ -1280,6 +1304,11 @@ function $HttpProvider() {
 
       url = buildUrl(url, config.paramSerializer(config.params));
 
+      if (isJsonp) {
+        // Check the url and add the JSONP callback placeholder
+        url = sanitizeJsonpCallbackParam(url, config.jsonpCallbackParam);
+      }
+
       $http.pendingRequests.push(config);
       promise.then(removePendingReq, removePendingReq);
 
@@ -1414,5 +1443,23 @@ function $HttpProvider() {
       }
       return url;
     }
+
+    function sanitizeJsonpCallbackParam(url, key) {
+      if (/[&?][^=]+=JSON_CALLBACK/.test(url)) {
+        // Throw if the url already contains a reference to JSON_CALLBACK
+        throw $httpMinErr('badjsonp', 'Illegal use of JSON_CALLBACK in url, "{0}"', url);
+      }
+
+      var callbackParamRegex = new RegExp('[&?]' + key + '=');
+      if (callbackParamRegex.test(url)) {
+        // Throw if the callback param was already provided
+        throw $httpMinErr('badjsonp', 'Illegal use of callback param, "{0}", in url, "{1}"', key, url);
+      }
+
+      // Add in the JSON_CALLBACK callback param value
+      url += ((url.indexOf('?') === -1) ? '?' : '&') + key + '=JSON_CALLBACK';
+
+      return url;
+    }
   }];
 }

test/ng/httpSpec.js

@@ -326,10 +326,8 @@ describe('$http', function() {
     });
 
     it('should accept a $sce trusted object for the request configuration url', function() {
-      expect(function() {
-        $httpBackend.expect('GET', '/url').respond('');
-        $http({url: $sce.trustAsResourceUrl('/url')});
-      }).not.toThrowMinErr('$http','badreq', 'Http request configuration url must be a string.  Received: false');
+      $httpBackend.expect('GET', '/url').respond('');
+      $http({url: $sce.trustAsResourceUrl('/url')});
     });
 
     it('should send GET requests if no method specified', function() {
@@ -622,7 +620,7 @@ describe('$http', function() {
           expect(r.headers()).toEqual(Object.create(null));
         });
 
-        $httpBackend.expect('JSONP', '/some').respond(200);
+        $httpBackend.expect('JSONP', '/some?callback=JSON_CALLBACK').respond(200);
         $http({url: $sce.trustAsResourceUrl('/some'), method: 'JSONP'}).then(callback);
         $httpBackend.flush();
         expect(callback).toHaveBeenCalledOnce();
@@ -1030,39 +1028,80 @@ describe('$http', function() {
       });
 
       it('should have jsonp()', function() {
-        $httpBackend.expect('JSONP', '/url').respond('');
+        $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK').respond('');
         $http.jsonp($sce.trustAsResourceUrl('/url'));
       });
 
 
       it('jsonp() should allow config param', function() {
-        $httpBackend.expect('JSONP', '/url', undefined, checkHeader('Custom', 'Header')).respond('');
+        $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK', undefined, checkHeader('Custom', 'Header')).respond('');
         $http.jsonp($sce.trustAsResourceUrl('/url'), {headers: {'Custom': 'Header'}});
       });
     });
 
     describe('jsonp trust', function() {
       it('should throw error if the url is not a trusted resource', function() {
         var success, error;
-        $http({method: 'JSONP', url: 'http://example.org/path?cb=JSON_CALLBACK'}).catch(
-          function(e) { error = e; }
-        );
+        $http({method: 'JSONP', url: 'http://example.org/path'})
+              .catch(function(e) { error = e; });
         $rootScope.$digest();
         expect(error.message).toContain('[$sce:insecurl]');
       });
 
       it('should accept an explicitly trusted resource url', function() {
-        $httpBackend.expect('JSONP', 'http://example.org/path?cb=JSON_CALLBACK').respond('');
-        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?cb=JSON_CALLBACK')});
+        $httpBackend.expect('JSONP', 'http://example.org/path?callback=JSON_CALLBACK').respond('');
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path')});
       });
 
       it('jsonp() should accept explictly trusted urls', function() {
-        $httpBackend.expect('JSONP', '/url').respond('');
+        $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK').respond('');
         $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url')});
 
-        $httpBackend.expect('JSONP', '/url?a=b').respond('');
+        $httpBackend.expect('JSONP', '/url?a=b&callback=JSON_CALLBACK').respond('');
         $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), params: {a: 'b'}});
       });
+
+      it('should error if the URL contains a JSON_CALLBACK parameter', function() {
+        var error;
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?callback=JSON_CALLBACK')})
+            .catch(function(e) { error = e; });
+        $rootScope.$digest();
+        expect(error.message).toContain('[$http:badjsonp]');
+
+        error = undefined;
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path?other=JSON_CALLBACK')})
+            .catch(function(e) { error = e; });
+        $rootScope.$digest();
+        expect(error.message).toContain('[$http:badjsonp]');
+      });
+
+      it('should error if a param contains a JSON_CALLBACK value', function() {
+        var error;
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {callback: 'JSON_CALLBACK'}})
+            .catch(function(e) { error = e; });
+        $rootScope.$digest();
+        expect(error.message).toContain('[$http:badjsonp]');
+
+        error = undefined;
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {other: 'JSON_CALLBACK'}})
+            .catch(function(e) { error = e; });
+        $rootScope.$digest();
+        expect(error.message).toContain('[$http:badjsonp]');
+      });
+
+      it('should error if there is already a param matching the jsonpCallbackParam key', function() {
+        var error;
+        $http({ method: 'JSONP', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {callback: 'evilThing'}})
+            .catch(function(e) { error = e; });
+        $rootScope.$digest();
+        expect(error.message).toContain('[$http:badjsonp]');
+
+        error = undefined;
+        $http({ method: 'JSONP', jsonpCallbackParam: 'cb', url: $sce.trustAsResourceUrl('http://example.org/path'), params: {cb: 'evilThing'}})
+            .catch(function(e) { error = e; });
+        $rootScope.$digest();
+        expect(error.message).toContain('[$http:badjsonp]');
+      });
     });
 
     describe('callbacks', function() {
@@ -1524,11 +1563,11 @@ describe('$http', function() {
       }));
 
       it('should cache JSONP request when cache is provided', inject(function($rootScope) {
-        $httpBackend.expect('JSONP', '/url?cb=JSON_CALLBACK').respond('content');
-        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache});
+        $httpBackend.expect('JSONP', '/url?callback=JSON_CALLBACK').respond('content');
+        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), cache: cache});
         $httpBackend.flush();
 
-        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url?cb=JSON_CALLBACK'), cache: cache}).success(callback);
+        $http({method: 'JSONP', url: $sce.trustAsResourceUrl('/url'), cache: cache}).success(callback);
         $rootScope.$digest();
 
         expect(callback).toHaveBeenCalledOnce();