-
Notifications
You must be signed in to change notification settings - Fork 18
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
Allow read_to_end with ChunkedEncoding #61
Conversation
if let HttpConnection::Tls(ref mut tls) = self.stream { | ||
return tls.fill_buf().await.map_err(|e| e.kind()); | ||
// The matches/if let dance is to fix lifetime of the borrowed inner connection. | ||
if self.stream.try_fill_buf().await.is_some() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rmja This got handy here, too :)
Do we want to return BufferTooSmall errors in the "outer" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed this. Lgtm!
Now this PR comes with a fix to an absolutely embarassingly dumb mistake :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Add failing test case Introduce TryBufRead, clean up
I had to do a bunch of refactoring, because I had to create an abstraction for
maybe-BufRead
types (and I had to implement that for the tokio adapter stuff). This is because ChunkedEncoding's new reader function works via BufRead.A downside of this PR is that
read_to_end
is only available forTryBufRead
types, which we only define forHttpConnection
in the library, and the tokio adapter in the tests.The PR can most likely be simplified, I was just aiming to get it working and it took long enough.