Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Body is blank when response has no Content-Length #45

Closed
bjeanes opened this issue Nov 4, 2013 · 10 comments
Closed

Body is blank when response has no Content-Length #45

bjeanes opened this issue Nov 4, 2013 · 10 comments
Milestone

Comments

@bjeanes
Copy link

bjeanes commented Nov 4, 2013

As per RFC 2616 section 4.4, Content-Length is not a required header in responses. I'm using The HTTP Gem with a server that doesn't always send a Content-Length header.

In these cases, the body content is "". Combined with accept(:json), this usually means A JSON text must at least contain two octets! (JSON::ParserError) exception.

See how Curl handles this:

curl -i 'https://api.honeybadger.io/v1/projects.json?auth_token=a-bad-key' -H 'Accept: application/json'
HTTP/1.1 401 Unauthorized
Server: nginx/1.4.3
Date: Mon, 04 Nov 2013 21:48:29 GMT
Content-Type: application/json; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Status: 401 Unauthorized
Strict-Transport-Security: max-age=31536000
X-UA-Compatible: IE=Edge,chrome=1
X-Request-Id: b48da36d18e3c259bfec15019f3393c4
X-Runtime: 0.039962
X-Rack-Cache: miss

{"error":"Invalid authentication token."}

With The HTTP Gem:

irb(main):001:0> require 'http'
=> true
irb(main):002:0> HTTP.get("https://api.honeybadger.io/v1/projects.json", params: {auth_token: "a-bad-key"})
=> ""

A gander at the code shows that this is because @body_remaining is derived entirely from the presence of a Content-Length header. In some (all?) conditions, if @body_remaining is nil, the body is not read.

@bjeanes
Copy link
Author

bjeanes commented Nov 4, 2013

It's worth noting here that the socket content after the request has been made is:

HTTP/1.1 401 Unauthorized\r\nServer: nginx/1.4.3\r\nDate: Mon, 04 Nov 2013 22:12:01 GMT\r\nContent-Type: application/json; charset=utf-8\r\nTransfer-Encoding: chunked\r\nConnection: keep-alive\r\nStatus: 401 Unauthorized\r\nStrict-Transport-Security: max-age=31536000\r\nX-UA-Compatible: IE=Edge,chrome=1\r\nX-Request-Id: 0404cee81486a95569ab4e55c85daeea\r\nX-Runtime: 0.012569\r\nX-Rack-Cache: miss\r\n\r\n29\r\n{\"error\":\"Invalid authentication token.\"}\r\n0\r\n\r\n

Note the chunked encoding. In chunked encoding, one can't read Content-Length bytes, one has to read each chunk until you reach the empty chunk (length 0).

@tarcieri
Copy link
Member

tarcieri commented Nov 4, 2013

You might take a look at some of the existing discussion of this issue on #28 and #39. I don't see a specific ticket for this problem still open for this issue though (besides those tickets, which are vicariously related).

tl;dr: yes, the logic is buggy, it's the result of not enough diligence in merging patches, and at this point I was hoping to address it via a rewrite I haven't had enough time to complete.

That said, an interim fix is probably in order.

@tarcieri
Copy link
Member

tarcieri commented Nov 4, 2013

@bjeanes I merged #28. HEAD should at least give you different behavior now, although it's still not correct.

@bjeanes
Copy link
Author

bjeanes commented Nov 4, 2013

I'll have to pull it down in test it, but it seems that this will just mean the body will be nil instead of empty string, no? Definitely better than an empty string semantically, but unfortunately doesn't help me here.

I'll have a look at those #39 too to see if there is anything I can do to help.

@tarcieri
Copy link
Member

tarcieri commented Nov 5, 2013

Yep, #28 is not a fix :(

@schmurfy
Copy link

I suppose this also affects chunked encoded response, am I wrong ?
When such response is received the body is blanked which is not that helpful...

@tarcieri
Copy link
Member

@schmurfy this should also be obsoleted by #39

@schmurfy
Copy link

so chunked response should work ? I will have a look again at my code tomorrow it may be on my side then.

@tarcieri
Copy link
Member

Yes, and you can stream the chunks using HTTP::ResponseBody#readpartial

@schmurfy
Copy link

ok I got it, the latest stable release is so far behind master that ResponseBody does not even exists in it xD
I will try master thanks for helping figure this out xD

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

No branches or pull requests

3 participants