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

net.PacketConn #367

Closed
neilalexander opened this issue May 7, 2021 · 4 comments · Fixed by #558
Closed

net.PacketConn #367

neilalexander opened this issue May 7, 2021 · 4 comments · Fixed by #558
Assignees
Milestone

Comments

@neilalexander
Copy link

I'm a little surprised to find that a DTLS implementation, which is specifically datagram-oriented, seems to expect a stream-oriented net.Conn rather than a datagram-oriented net.PacketConn as an input to Client() and Server().

Curious if there is a reason for this? It means that anything net.PacketConn-based has to be wrapped.

@daenney
Copy link
Member

daenney commented May 7, 2021

@Sean-Der Do you know why this is? Looking at our code, the only thing we use from Conn that PacketConn doesn't have is the RemoteAddr, but since using ReadFrom on a PacketConn returns the info it doesn't seem necessary. Somewhat similarly we'd need to use WriteTo on a PacketConn, which in turn needs the address. But it feels like this is something we should just plop onto the ConnCtx.

I'm honestly a little confused myself now 😅

@igolaizola
Copy link
Member

The other day I was listening to Adam Woodbeck, author of Network Programming with Go, during a gotime podcast (https://changelog.com/gotime/176) and he recommends to use net.PacketConn on UDP implementations.

@Sean-Der
Copy link
Member

Sean-Der commented May 17, 2021

Just a design mistake! I am up for fixing this in the next major release. Maybe we could provide a nice helper to make net.Conn users migration easy.

This also would be a great chance to fix this as well. We shouldn't provide a Handshake function that is implicitly called on Read/Write. Right now we do it on conn creation which diverges from crypto/tls and causes issues.

@daenney daenney added this to the 3.0.0 milestone May 17, 2021
@hasheddan hasheddan self-assigned this Aug 2, 2023
@hasheddan
Copy link
Contributor

Assigned myself as this is being addressed as part of #558.

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 a pull request may close this issue.

5 participants