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

Request interceptor headers #2770

Closed
wants to merge 2 commits into from
Closed

Request interceptor headers #2770

wants to merge 2 commits into from

Conversation

jasadams
Copy link

Currently it is possible to add a request interceptor and modify the config object for a http request before it is sent. I am able to add custom headers to the config object within an interceptor however these headers get ignored when the http request is constructed. I was able to fix this in my own fork by modifying the following line in the "serverRequest" function
from:

var reqData = transformData(config.data, headersGetter(headers), config.transformRequest);

to:

var reqData = transformData(config.data, headersGetter(extend(headers,config.headers)), config.transformRequest);

@IgorMinar
Copy link
Contributor

the issue you are fixing only occurs if you override the headers object in the interceptor instead of just mutating the existing object.

that we should either claim that the current behavior is desired or that when an interceptor overwrites the headers object config this new object should be used instead of trying to merge the new object with the previously used object.

here is my test:

      iit('should allow replacement of headers object', function() {
        module(function($httpProvider) {
          $httpProvider.interceptors.push(function() {
            return {
              request: function(config) {
                config.headers = {foo: 'intercepted'};
                return config;
              }
            };
          });
        });
        inject(function($http, $httpBackend, $rootScope) {
          $httpBackend.expect('GET', '/url', null, function (headers) {
            return angular.equals(headers, {foo: 'intercepted'});
          }).respond('');
          $http.get('/url');
          $rootScope.$apply();
        });
      });

and my fix:

diff --git a/src/ng/http.js b/src/ng/http.js
index d81b791..a44da3a 100644
--- a/src/ng/http.js
+++ b/src/ng/http.js
@@ -666,6 +666,7 @@ function $HttpProvider() {


       var serverRequest = function(config) {
+        headers = config.headers;
         var reqData = transformData(config.data, headersGetter(headers), config.transformRequest);

         // strip content-type if data is undefined

@vojtajina
Copy link
Contributor

This is just a bug imho. Igor's fix LGTM.

@jasadams
Copy link
Author

you guys know about how this should be more than me. As long as I can intercept requests and add items to the header, I am happy.

@IgorMinar
Copy link
Contributor

you can do that today as long as you don't overwrite the config.headers object.

so instead of:

config.headers = { foo: 'bar' };

do:

config.headers.foo ='bar'

after my fix, the first snippet will work, but will override all the default headers which is rarely desirable, but better than ignoring the new headers completely.

@jasadams
Copy link
Author

Got it. Thanks

On 13/07/2013, at 10:29, Igor Minar notifications@github.com wrote:

you can do that today as long as you don't overwrite the config.headers object.

so instead of:

config.headers = { foo: 'bar' };
do:

config.headers.foo ='bar'
after my fix, the first snippet will work, but will override all the default headers which is rarely desirable, but better than ignoring the new headers completely.


Reply to this email directly or view it on GitHub.

@IgorMinar IgorMinar closed this in 514dc0e Jul 13, 2013
ctrahey pushed a commit to ctrahey/angular.js that referenced this pull request Jul 22, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants