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

TcpBuilder::connect should use SocketAddr instead of ToSocketAddrs. #45

Closed
ghost opened this issue Sep 30, 2016 · 4 comments
Closed

TcpBuilder::connect should use SocketAddr instead of ToSocketAddrs. #45

ghost opened this issue Sep 30, 2016 · 4 comments

Comments

@ghost
Copy link

ghost commented Sep 30, 2016

If ToSocketAddrs are resolved to multiple addresses, then in case of failure
connect will be used multiple times with the same socket. This is incorrect.
After first failed connect the state of socket is unspecified and socket should
not be reused.

Additionally, if socket is in non-blocking mode, then the connect can return
EINPROGRESS. Connection will not be aborted, and further calls to connect will
fail with EALREADY, because connection request is already in progress.

@alexcrichton
Copy link
Contributor

Hm yeah with TcpBuilder the story here isn't great, but this is also part of the state transition from TcpBuilder -> TcpStream, so the error would also need to carry that payload unfortunately :(

@ghost
Copy link
Author

ghost commented Oct 4, 2016

Currently the internal socket is consumed and converted to a TcpStream only
if connect succeeds. So you could actually check for EINPROGRESS and still call
to_tcp_stream after receiving an error from connect. Not saying this is the
best way to proceed, but it is currently possible.

I was mostly concerned with different thing, i.e., exposing an API that uses
connect syscall multiple times with the same socket, all because of
ToSocketAddrs used as an argument to TcpBuilder.

@alexcrichton
Copy link
Contributor

I suppose if we could go back I'd want to rethink a good portion of this crate. I feel like TcpBuilder never ended up pulling its own weight and I'd prefer to keep all the methods as low-level as possible (e.g. remove ToSocketAddrs).

@pfmooney
Copy link
Member

pfmooney commented Dec 9, 2020

net2 is deprecated and under sustaining maintenance now, accepting only bugfixes and patches to enable new rust platforms.

@pfmooney pfmooney closed this as completed Dec 9, 2020
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

No branches or pull requests

2 participants