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: store IPv4 returned from cgo resolver as 4-byte slice net.IP #53638

Closed
wants to merge 1 commit into from

Conversation

ZekeLu
Copy link
Contributor

@ZekeLu ZekeLu commented Jul 1, 2022

net.IP states that a 16-byte slice can still be an IPv4 address.
But after netip.Addr is introduced, it requires extra care to keep
it as an IPv4 address when converting it to a netip.Addr using
netip.AddrFromSlice.

To address this issue, let's change the cgo resolver to return
4-byte net.IP for IPv4. The change will save us 12 bytes too.

Please note that the go resolver already return IPv4 as 4-byte
slice.

The test TestResolverLookupIP has been modified to cover this
behavior. So no new test is added.

Fixes #53554.

@gopherbot
Copy link
Contributor

This PR (HEAD: 414bf0b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/415580 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 1: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1: TryBot-Result-1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

net.IP states that a 16-byte slice can still be an IPv4 address.
But after netip.Addr is introduced, it requires extra care to keep
it as an IPv4 address when converting it to a netip.Addr using
netip.AddrFromSlice.

To address this issue, let's change the cgo resolver to return
4-byte net.IP for IPv4. The change will save us 12 bytes too.

Please note that the go resolver already return IPv4 as 4-byte
slice.

The test TestResolverLookupIP has been modified to cover this
behavior. So no new test is added.
@gopherbot
Copy link
Contributor

This PR (HEAD: bd7bb2f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/415580 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 2: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 2: TryBot-Result+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 2: Auto-Submit+1 Code-Review+2

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/415580.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Nov 2, 2022
net.IP states that a 16-byte slice can still be an IPv4 address.
But after netip.Addr is introduced, it requires extra care to keep
it as an IPv4 address when converting it to a netip.Addr using
netip.AddrFromSlice.

To address this issue, let's change the cgo resolver to return
4-byte net.IP for IPv4. The change will save us 12 bytes too.

Please note that the go resolver already return IPv4 as 4-byte
slice.

The test TestResolverLookupIP has been modified to cover this
behavior. So no new test is added.

Fixes #53554.

Change-Id: I0dc2a59ad785c0c67a7bc22433105529f055997f
GitHub-Last-Rev: bd7bb2f
GitHub-Pull-Request: #53638
Reviewed-on: https://go-review.googlesource.com/c/go/+/415580
Auto-Submit: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/415580 has been merged.

@gopherbot gopherbot closed this Nov 2, 2022
romaindoumenc pushed a commit to TroutSoftware/go that referenced this pull request Nov 3, 2022
net.IP states that a 16-byte slice can still be an IPv4 address.
But after netip.Addr is introduced, it requires extra care to keep
it as an IPv4 address when converting it to a netip.Addr using
netip.AddrFromSlice.

To address this issue, let's change the cgo resolver to return
4-byte net.IP for IPv4. The change will save us 12 bytes too.

Please note that the go resolver already return IPv4 as 4-byte
slice.

The test TestResolverLookupIP has been modified to cover this
behavior. So no new test is added.

Fixes golang#53554.

Change-Id: I0dc2a59ad785c0c67a7bc22433105529f055997f
GitHub-Last-Rev: bd7bb2f
GitHub-Pull-Request: golang#53638
Reviewed-on: https://go-review.googlesource.com/c/go/+/415580
Auto-Submit: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
@ZekeLu ZekeLu deleted the ip branch November 4, 2022 16:16
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.

net: LookupNetIP returns IPv4 in IPv6 instead of IPv4
2 participants