hasBody() should return false if Content-Length: 0 #815

Merged
merged 1 commit into from Jun 5, 2013

Projects

None yet

2 participants

@darobin

Some clients will set Content-Length: 0 when there is no content (and shouldn't be, e.g. on a GET). This can cause other parts of connect to be confused. For instance, if the client also sets Content-Type: application/json, the json middleware will try to parse an empty string and blow up.

Of course, such clients are unhelpful at best, but connect should not blow up.

@darobin darobin hasBody() should return false if Content-Length: 0
Some clients will set Content-Length: 0 when there is no content (and shouldn't be, e.g. on a GET). This can cause other parts of connect to be confused. For instance, if the client also sets Content-Type: application/json, the json middleware will try to parse an empty string and blow up.

Of course, such clients are unhelpful at best, but connect should not blow up.
4f5941d
@darobin

Hmmm, the newline changes were introduced by GitHub.

@tj
Sencha Labs member
tj commented Jun 5, 2013

hahaha whyyyy do all http clients suck, is this jquery again?

@darobin

Hehe, no, this time it's request (this one: https://github.com/mikeal/request). If you set json to true because you want it to advertise that you accept it and parse it for you, it sends GET requests with Content-Type: application/json and Content-Length: 0. That's worthy of a fix there as well, but I figured connect ought to survive this case anyway.

@tj
Sencha Labs member
tj commented Jun 5, 2013

damn, yeah, surely someone else is doing it too

@tj tj merged commit 092341c into senchalabs:master Jun 5, 2013

1 check passed

Details default The Travis CI build passed
@tj
Sencha Labs member
tj commented Jun 5, 2013

a test would be great too if you have a minute

@darobin

Sure thing, here goes: #821

Will trade you for a review of pillarjs/send#19 ;-)

@wavded wavded pushed a commit to wavded/connect that referenced this pull request Nov 1, 2013
@darobin darobin add a test to check that connect properly survives content-length=0 a…
…nd content-type=application/json (see senchalabs#815)
66d0c7f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment