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

fix($http): won't parse single space response #9907

Closed
wants to merge 2 commits into from

Conversation

gampleman
Copy link
Contributor

According to this question, Rails in order to patch a bug in Safari, returns a single space when calling head :ok, which is the standard for POST requests.

Angular since 9ba24c54d checks if the body is empty before calling JSON parse, but this still breaks in this case, since the body isn't empty, but contains a single space.

We are hitting this issue when trying to upgrade from Angular 1.2.10. This PR should help in that case.

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@@ -782,7 +782,7 @@ function $HttpProvider() {
function transformResponse(response) {
// make a copy since the response must be cacheable
var resp = extend({}, response);
if (!response.data) {
if (!response.data || response.data === '') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't actually make sense, since '' is still falsy

Rails sends a single space response instead of empty headers, this
change will not attempt to parse the response
@pkozlowski-opensource
Copy link
Member

@gampleman thnx for this PR but I don't think we can merge it as-is - see my comments in the code.

@pkozlowski-opensource
Copy link
Member

So while it kind of make sense that we trim strings before running it through toJson people could easily work-arround it already by adding a custom transformer so I guess it is not critically urgent that we get this fix in.

@pkozlowski-opensource pkozlowski-opensource added this to the Ice Box milestone Nov 11, 2014
@gampleman
Copy link
Contributor Author

OK, so I've changed it to check right before attempting to parse the JSON that the response isn't just blank.


Side note: For us it's a blocker on upgrading to Angular 1.3, since all our APIs have this behaviour. We don't want to change Rails and while adding a custom transformer everywhere is an option, it doesn't seem too attractive.

@pkozlowski-opensource pkozlowski-opensource modified the milestones: 1.3.4, Ice Box Nov 15, 2014
@pkozlowski-opensource
Copy link
Member

LGTM, merging

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

Successfully merging this pull request may close these issues.

4 participants