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

Fix readpartial loop #115

Merged
merged 1 commit into from Mar 28, 2014
Merged

Fix readpartial loop #115

merged 1 commit into from Mar 28, 2014

Conversation

ixti
Copy link
Member

@ixti ixti commented Mar 28, 2014

This closes outstanding bits with socket not being correctly closed and cases when parser was received body data in one chunk.

Things to keep in mind

@socket should belong to Connection...

... and Connection should belong to `Response (see #72). This will allow client re-use without having any issues:

client = HTTP::Client.new

res_1 = client.get "http://example.com/chunked-1"
res_2 = client.get "http://example.com/chunked-2"

With current implementation, the use-case above will be defected (res1 will never be able to read it's response body - it will be body of second request).

There's no need to keep track of Content-Length...

... and/or Transfer-Encoding. This is done by HTTP::Parser already. Thus we can safely rely on HTTP::Response::Parser#finished? which is true as soon as underlying HTTP::Parser reports message end.

Related issues

//cc @sferik @tarcieri

@ixti
Copy link
Member Author

ixti commented Mar 28, 2014

@tarcieri @sferik this is the last bit that was holding 0.6 release.

sferik added a commit that referenced this pull request Mar 28, 2014
@sferik sferik merged commit baa16bf into master Mar 28, 2014
@sferik sferik deleted the fix/client branch March 28, 2014 12:31
@sferik
Copy link
Contributor

sferik commented Mar 28, 2014

Awesome! I think we’re ready to go on v0.6.

@tarcieri: You’re the only one with gem push access…

@ixti ixti mentioned this pull request Mar 28, 2014
@ghost
Copy link

ghost commented Mar 28, 2014

🍰

@tarcieri
Copy link
Member

🎉 🎉 🎉

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

Successfully merging this pull request may close these issues.

None yet

3 participants