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

x/net/ipv4: NewPacketConn requires net.Conn but this is not documented #47491

Open
AudriusButkevicius opened this issue Aug 1, 2021 · 1 comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@AudriusButkevicius
Copy link
Contributor

AudriusButkevicius commented Aug 1, 2021

What version of Go are you using (go version)?

tip

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOOS=windows
GOARCH=amd64

What did you do?

Called ipv4.NewPacketConn with something that satisfies net.PacketConn but not net.Conn.

The code in NewPacketConn does a type assertion to net.Conn, without checking if the type satisfies it.
It's not documented that the parameter also needs to satisfy net.Conn.

See: https://github.com/golang/net/blob/master/ipv4/endpoint.go#L102-L110

Ideally NewPacketConn would return a conn, err, but that would be a breaking change, in which case the second best option would be for it to return a connection that fails the ok() checks of the genericOpt, dgramOpt and payloadHandler so that the connection returned would just return errInvalidConn for any operation.

Alternative would be to document that ipv4.NewPacketConn also needs to satisfy net.Conn.

What did you expect to see?

A connection returned, potentially one that returns an error on every operation.

What did you see instead?

A panic

@gopherbot gopherbot added this to the Unreleased milestone Aug 1, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 1, 2021
@seankhliao
Copy link
Member

cc @ianlancetaylor

@mdlayher mdlayher changed the title x/net/ipv4: NewPacketConn is unsafe to use x/net/ipv4: NewPacketConn requires net.Conn but this is not documented Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants