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

Fix to make default headers to be different objects breaks POST,PUT,DELETE #5997

Closed
JoakimBe opened this issue Jan 27, 2014 · 6 comments
Closed

Comments

@JoakimBe
Copy link

I updated my angular-lib from an early 1.2 release yesterday to 1.2.10 and all my POST,PUT and DELETE( CORS ) requests failed due to CSRF ( django backend ).

I started to go backwards and found and that the error started to occur between 1.2.8 and 1.2.9. To be specific, the following commit makes my requests fail:

cironunes@7812874

I did a quick test and changed the source-code in 1.2.10 back according to this commit and eveyrthing started working again.

Can anyone confirm this?

Hopefully I manage to get enough time tonight to setup a test-app that shows the problem if debugging needs to be done.

@ghost ghost assigned vojtajina Jan 28, 2014
@vojtajina
Copy link
Contributor

Can you check the actual headers (either in the web inspector - network panel or log them on the server) and compare the diff between 1.2.8 and 1.2.9?

@JoakimBe
Copy link
Author

Thats a good point. Basically the request headers are identical, except this:

X-CSRFToken:N251mZOjKpxRFHEtC0zlGsIeVTY5g21D

Which dont seem to be sent in 1.2.9+, but works fine in 1.2.8 and earlier.
Of course this is what makes the CSRF to fail.

To set the X-CSRFToken I do this:
$http.defaults.headers.post['X-CSRFToken'] = $cookies.csrftoken;

Question is why the the copy() command in the commit I mentioned prevents the ['X-CSRFToken'] to be sent?

@caitp
Copy link
Contributor

caitp commented Jan 28, 2014

@JoakimBe, the copying happens before your config function ever runs. I guess we should have realized this could be a breaking change, it might be nice to have a convenience method to set them for all methods.

@JoakimBe
Copy link
Author

@caitp , Okey, makes sense. So is it a bug or more of a behavioral change in the angular-lib? The conclusion of that question is, do we need to configure these header settings differently then we have in the past? Or will you create a fix for it in the coming versions?

@caitp
Copy link
Contributor

caitp commented Jan 28, 2014

You need to configure them more heavily... we could write a convenience method to simplify it, but yeah for now you should set the header for each of the things that need it

@JoakimBe
Copy link
Author

Ok, it worked like you said.

I'am closing this issue. Thank you @caitp

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants