Skip to content

Read/Write impl rework for rustls#592

Merged
brson merged 4 commits intorust-lang:masterfrom
inejge:tls-hang-fix
Jul 29, 2016
Merged

Read/Write impl rework for rustls#592
brson merged 4 commits intorust-lang:masterfrom
inejge:tls-hang-fix

Conversation

@inejge
Copy link
Copy Markdown
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
Copy Markdown
Contributor

brson commented Jul 19, 2016

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

inejge added 2 commits July 25, 2016 09:48
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
Copy Markdown
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