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

Empty response body when Content-Length and Transfer-Encoding headers are omitted #475

Closed
jknipp opened this issue Feb 14, 2018 · 18 comments · Fixed by #496
Closed

Empty response body when Content-Length and Transfer-Encoding headers are omitted #475

jknipp opened this issue Feb 14, 2018 · 18 comments · Fixed by #496
Assignees
Labels

Comments

@jknipp
Copy link

jknipp commented Feb 14, 2018

Beginning with v1.11.0, if the response from the server omits both the Content-Length and Transfer-Encoding response headers, the body of the response is not parsed and returned.

v1.11.0 Response Body

screen_shot_2018-02-14_at_10_40_38_am

v1.10.1 Response Body

screen_shot_2018-02-14_at_10_45_46_am

I believe the bug was introduced with the change in #450 to handle empty/invalid content-length.

5b51fb8#diff-03059c69a9df088e2476724af69f06bdL334

@jknipp jknipp changed the title No response body when Content-Length and Transfer-Encoding headers are omitted Empty response body when Content-Length and Transfer-Encoding headers are omitted Feb 14, 2018
@intentionally-left-nil
Copy link

This is a breaking bug for us, we've had to pin our hackney at an old version to mitigate it.

@jknipp jknipp changed the title Empty response body when Content-Length and Transfer-Encoding headers are omitted Empty response body when Content-Length and Transfer-Encoding headers are omitted Feb 16, 2018
@ConnorRigby
Copy link

I just ran into this issue also with Httpoison

@benoitc
Copy link
Owner

benoitc commented Feb 26, 2018

@jknipp good catch!

Is the server sending an HTTP 1.0 response ? 1.1 normally expects to have a content length. Can you check it?

I propose indeed to fix it by checking if the client receive an http 1.0 response and then read until a max. Thoughts?

@benoitc benoitc self-assigned this Feb 26, 2018
@benoitc benoitc added the bug label Feb 26, 2018
@intentionally-left-nil
Copy link

intentionally-left-nil commented Feb 27, 2018

@benoitc: Unfortunately, a lot of the internet doesn't follow specifications (HTTP 1.1 or otherwise). It would be beneficial for a client library to be as flexible as possible in this regard.

The spec actually doesn't require Content-length to be set. Instead, if the header is not sent, then the client must close the connection after it is completed. See the spec: https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4

5.By the server closing the connection. (Closing the connection cannot be used to indicate the end of a request body, since that would leave no possibility for the server to send back a response.)

@benoitc
Copy link
Owner

benoitc commented Feb 27, 2018 via email

@ericmj
Copy link
Contributor

ericmj commented Feb 27, 2018

The server is following the spec, as @AnilRedshift said a missing content-length is valid and closing the connection will signal the end of the response body.

@benoitc
Copy link
Owner

benoitc commented Feb 27, 2018

well the server should do its best to send a content-length. I will fix it anyway.

benoitc added a commit that referenced this issue Feb 27, 2018
this change handle the case where no content-length and
transfer-encoding headers are given in the response. The spec says we
should try to read it.

fix #475
@benoitc
Copy link
Owner

benoitc commented Feb 27, 2018

can you test #481 ? It shoudl fix your issue. Let me know :)

@PatNowak
Copy link

PatNowak commented Feb 28, 2018

Hi,

I just tested. Value in response is correct, but the whole response is returned as an "error response". I use email decryptor service, so body returned is an email. My tests are in Elixir IEx.
Please take a look:

Response from hackney 1.10.0 via HTTPoison (default use case)

{:ok,
	%HTTPoison.Response{
	body: "[\"empty11@o3.pl\"]", 
	headers: headers,
	request_url: "URL",
	status_code: 200
}}

Response from hackney 1.11.0 (master) via HTTPoison - reason for this issue

{:ok,
	%HTTPoison.Response{
	body: "",
	headers: headers,
	request_url: "URL",
	status_code: 200
}}

Response from hackney from branch fix/gh-475 via HTTPoison

{:error, %HTTPoison.Error{id: nil, reason: {:closed, "[\"empty11@o3.pl\"]"}}}

Response from hackney from fix branch

{:ok, 200, _list, ref} = :hackney.request(:get, url, headers) # that's fine
 :hackney.body(ref)
{:error, {:closed, "[\"empty11@o3.pl\"]"}}

As you may see, the value is correct, but returns in error response.

@benoitc
Copy link
Owner

benoitc commented Feb 28, 2018

@PatNowak do you have a link I could test by chance? Can be done in private if you want to

@PatNowak
Copy link

PatNowak commented Feb 28, 2018

It was url from private network, so it will not work for you. I can try to find an example that will be available publicly :)

Edit: Sorry, I didn't find an example in public network :/

@benoitc
Copy link
Owner

benoitc commented Feb 28, 2018

@PatNowak I pushed ce32d21 in the PR that should fix your issue. Let me know :)

@benoitc
Copy link
Owner

benoitc commented Mar 1, 2018

@PatNowak ?

@benoitc
Copy link
Owner

benoitc commented Mar 8, 2018

bump.

@filmor
Copy link

filmor commented Mar 16, 2018

@benoitc, in the meantime, would you please retract the package from Hex?

@benoitc
Copy link
Owner

benoitc commented Mar 16, 2018

@filmor Only mix has this feature. Anyway did you test the PR, it would help to have some feedback...

@filmor
Copy link

filmor commented Mar 16, 2018

I think you can ping the hex admins (support@hex.pm), I'll test the PR on Monday.

@benoitc
Copy link
Owner

benoitc commented Apr 2, 2018

@filmor ping?

benoitc added a commit that referenced this issue Apr 2, 2018
benoitc added a commit that referenced this issue Apr 2, 2018
when the connection is closed but buffer not empty, return and OK
result.

fix #475
benoitc added a commit that referenced this issue Apr 2, 2018
when the connection is closed but buffer not empty, return and OK
result.

fix #475
benoitc added a commit that referenced this issue Apr 2, 2018
fix return for empty content length

when the connection is closed but buffer not empty, return and OK
result.

fix #475
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants