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

Satisfy IO#readpartial API #618

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

singpolyma-shopify
Copy link

IO#readpartial allows for a second "outbuf" param which some streaming usages
expect, so support it here to allow using response bodies anywhere IO can be.

lib/http/connection.rb Show resolved Hide resolved
Comment on lines 70 to 71
buffer << outbuf while connection.readpartial(3, outbuf)
expect(buffer).to eq "1234567890"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the problem above, if you will change size to 1 or 2 you will get corrupted data: 1224447790 or 12345457890 respectively.

@ixti
Copy link
Member

ixti commented Sep 22, 2020

@singpolyma-shopify thank you for the PR. I really like the proposed changes, but we need to make sure they work as expected when parser's buffer contains more data than requested size :D

IO#readpartial allows for a second "outbuf" param which some streaming usages
expect, so support it here to allow using response bodies anywhere IO can be.
@singpolyma-shopify
Copy link
Author

@ixti fixed

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

2 participants