Skip to content

Conversation

@asayers
Copy link

@asayers asayers commented Mar 3, 2017

I'd like to upgrade the openssl dependency to the latest, to allow rust-websocket to be used alongside other crates which also depend on openssl. The upgrade was mostly straightforward, but I did hit one snag: SslStream no longer implements try_clone, which makes implementing it for WebSocketStream problematic. Apparently rustls' ClientSession also doesn't provide a try_clone.

This PR breaks secure websockets; I'm not suggesting you merge it as-is - just asking for suggestions.

WebSocketStream::Tcp(ref inner) => WebSocketStream::Tcp(try!(inner.try_clone())),
WebSocketStream::Ssl(ref inner) => WebSocketStream::Ssl(try!(inner.try_clone())),
// FIXME: This is **terrible**! But SslStream no longer has try_clone...
WebSocketStream::Ssl(ref inner) => WebSocketStream::Tcp(try!(inner.get_ref().try_clone())),
Copy link
Author

Choose a reason for hiding this comment

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

This is the problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I know that problem exactly. 😢
I've been thinking about how to fix it and I think just to disallow separating for SSL

@illegalprime
Copy link
Collaborator

illegalprime commented Mar 28, 2017

@asayers I have this fixed in #80 , watch out for that to land

@asayers
Copy link
Author

asayers commented Mar 29, 2017

Great! I guess I can close this PR now then.

@asayers asayers closed this Mar 29, 2017
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