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

Don't send empty String in place of no body #4021

Closed
wants to merge 1 commit into from

Conversation

jroper
Copy link
Contributor

@jroper jroper commented Sep 17, 2013

XMLHttpRequest.send spec defines different semantics for an empty string as compared to null, any string, whether empty or not, should be sent with a Content-Type of text/plain, whereas null should have no Content-Type header set. Since the user hasn't passed in content to send, the latter should be used.

See http://www.w3.org/TR/XMLHttpRequest/#the-send()-method

Combined with this bug in WebKit based browsers:

https://code.google.com/p/chromium/issues/detail?id=172802

This means when no body is supplied, the Content-Type ends up being application/xml, which for an empty String, means the body is invalid according to the content type. This causes issues for web servers that automatically parse the body according to the declared content type, such as this issue in Play framework:

playframework/playframework#1676

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@jroper
Copy link
Contributor Author

jroper commented Sep 17, 2013

CLA signed and commit message updated according to conventions.

@benmccann
Copy link
Contributor

+1 to this. thanks james for finding this issue!

@petebacondarwin
Copy link
Contributor

@jroper - thanks for this. Can you provide a unit test that shows the wrong behaviour before your fix?

@jroper
Copy link
Contributor Author

jroper commented Sep 19, 2013

I'm happy to do this, but I followed the instructions here to get an angular development environment setup:

http://docs.angularjs.org/misc/contribute

In short, I already had Java, node and npm installed, so I ran these commands:

sudo npm install -g grunt-cli
sudo npm install -g bower
cd angular.js
npm install
bower install

and then I ran

grunt package

and got this:

$ grunt package

module.js:340
    throw err;
          ^
Error: Cannot find module 'coffee-script'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:362:17)
    at require (module.js:378:17)
    at Object.<anonymous> (/Users/jroper/src/angular.js/node_modules/grunt/lib/grunt.js:16:1)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:362:17)

I'm not a grunt, npm or bower user, I have no idea where to start to work out what's gone wrong here. If you can help me get my dev environment setup I'll be happy to write the tests.

The XMLHttpRequest.send spec defines different semantics for null as to
an empty String, an empty String should be sent with a Content-Type of
text/plain, whereas null should have no Content-Type header set.  Since
the user hasn't passed in content to send, the latter should be used.
@jroper
Copy link
Contributor Author

jroper commented Sep 19, 2013

I figured it out, I had an old version of npm, after updating npm, and doing git clean, I got everything working.

I've added a test, and I also ran it without my change to make sure it failed.

@btford
Copy link
Contributor

btford commented Oct 1, 2013

Landed as 0d0330a.

Thanks!

@btford btford closed this Oct 1, 2013
@jroper jroper deleted the patch-1 branch October 22, 2014 02:51
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.

5 participants