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

Read/Write impl rework for rustls #592

Merged
merged 4 commits into from
Jul 29, 2016
Merged

Conversation

inejge
Copy link
Contributor

@inejge inejge commented Jul 18, 2016

Further tweaks for #568: rustls-enabled rustup can now actually download content. I think that merging should wait, though, since I don't think I thoroughly understand the consequences of the modifications I've had to make both to rustup and rustls in order to make it happen (which is why I've switched it to a private fork for the time being.)

rustls really, really wants to be driven asynchronously, and adapting it to synchronous-style hyper involved manually inserting calls to write_tls() after processing input packets in the Read impl (without which the protocol negotiation would hang.) Also, wants_read() in rustls used to return true all the time; I modified it to track the size of its plaintext buffer, so that further read_tls() calls are triggered only when that buffer is empty. It seems to work, but I'm not sure what happens when the other side sends an alert, or forcibly closes the connection.

cc @ctz

@brson
Copy link
Contributor

brson commented Jul 19, 2016

OK, holding off on merging for further word. Thanks @inejge.

Methods like read_tls(), formerly implemented directly on ClientSession,
are now accessible via the Session trait, so we need it in scope.
@inejge
Copy link
Contributor Author

inejge commented Jul 25, 2016

After discussing my changes in rustls/rustls#12, I think that they can be merged. I've updated rustls to the latest master, which now supports the usage in this PR, and has a number of other fixes.

@brson brson merged commit d632726 into rust-lang:master Jul 29, 2016
@inejge inejge deleted the tls-hang-fix branch July 31, 2016 19:38
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.

2 participants