Add new(), connect(), accept() and handshake() to SslStream#153
Add new(), connect(), accept() and handshake() to SslStream#153nox merged 1 commit intocloudflare:masterfrom
Conversation
These APIs allow more SslStream to be used more flexibly
|
Just wondering, why do we need more flexibility around those functions? I would rather we reduce the number of ways to do the same thing in that crate, than increase it. What edge does |
|
Btw, have you seen this other PR that touches the handshake API entrypoints? #142 |
|
My intention is to bring in the APIs openssl-rs provides. With these, it is easier for openssl-rs users to migrate to boring-rs (and vise versa). Also I think this PR might might help simplify the async implementation too, as claimed by rust-openssl/rust-openssl#1397. See more: tokio-rs/tokio-openssl@56f6618 |
|
I really don't like how tokio-openssl lets you mix up I think #142 is overall better than rust-openssl/rust-openssl#1397 but I'll let other people decide about that, as I can see value in API parity with the openssl stack, even if it is worse. |
|
Thanks.
enum MySslStream<S> {
PreConnect((Ssl, S))
MidHandshake(MidHandshakeSslStream<S>)
Connected<Ssltream<S>>
} |
I don't understand this. Why do you need |
|
This just an artificial example, say, I want my custom stream struct to log the TCP handshake time. MyStream<S> {
tcp_handshaket_time: Duration,
MySslStream<S>
}
enum MySslStream<S> {
PreConnect((Ssl, S))
MidHandshake(MidHandshakeSslStream<S>)
Connected<Ssltream<S>>
}vs MyStream<S> {
tcp_handshaket_time: Duration,
SslStream<S>
}It is just very annoying that I cannot have a single object to store the ssl stream before and after TLS handshake. The point I try to make is that the current interface is unnecessarily complicated for some use case, which can be greatly simplified with the APIs in this PR. |
|
Wouldn't you store the TCP handshake time in the inner |
|
Maybe that is just a bad example. Maybe I have something that doesn't belong to |
These APIs allow more SslStream to be used more flexibly