-
Notifications
You must be signed in to change notification settings - Fork 111
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
move ResolveUnspecifiedAddress(es) and FilterAddrs here from libp2p/go-addr-util #168
Merged
Merged
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1269a58
move ResolveUnspecifiedAddress(es) here from libp2p/go-addr-util
marten-seemann 18266e9
unexport InterfaceAddresses
marten-seemann 334b79e
move FilterAddrs here from go-addr-util
marten-seemann 6792670
optimize FilterAddrs implementation
marten-seemann 6a0950f
rename AddrOverNonLocalIP to IsIpv6LinkLocal
marten-seemann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| package manet | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| ma "github.com/multiformats/go-multiaddr" | ||
| ) | ||
|
|
||
| // ResolveUnspecifiedAddress expands an unspecified ip addresses (/ip4/0.0.0.0, /ip6/::) to | ||
| // use the known local interfaces. If ifaceAddr is nil, we request interface addresses | ||
| // from the network stack. (this is so you can provide a cached value if resolving many addrs) | ||
| func ResolveUnspecifiedAddress(resolve ma.Multiaddr, ifaceAddrs []ma.Multiaddr) ([]ma.Multiaddr, error) { | ||
| // split address into its components | ||
| split := ma.Split(resolve) | ||
|
|
||
| // if first component (ip) is not unspecified, use it as is. | ||
| if !IsIPUnspecified(split[0]) { | ||
| return []ma.Multiaddr{resolve}, nil | ||
| } | ||
|
|
||
| out := make([]ma.Multiaddr, 0, len(ifaceAddrs)) | ||
| for _, ia := range ifaceAddrs { | ||
| // must match the first protocol to be resolve. | ||
| if ia.Protocols()[0].Code != resolve.Protocols()[0].Code { | ||
| continue | ||
| } | ||
|
|
||
| split[0] = ia | ||
| joined := ma.Join(split...) | ||
| out = append(out, joined) | ||
| } | ||
| if len(out) < 1 { | ||
| return nil, fmt.Errorf("failed to resolve: %s", resolve) | ||
| } | ||
| return out, nil | ||
| } | ||
|
|
||
| // ResolveUnspecifiedAddresses expands unspecified ip addresses (/ip4/0.0.0.0, /ip6/::) to | ||
| // use the known local interfaces. | ||
| func ResolveUnspecifiedAddresses(unspecAddrs, ifaceAddrs []ma.Multiaddr) ([]ma.Multiaddr, error) { | ||
| // todo optimize: only fetch these if we have a "any" addr. | ||
| if len(ifaceAddrs) < 1 { | ||
| var err error | ||
| ifaceAddrs, err = interfaceAddresses() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| var outputAddrs []ma.Multiaddr | ||
| for _, a := range unspecAddrs { | ||
| // unspecified? | ||
| resolved, err := ResolveUnspecifiedAddress(a, ifaceAddrs) | ||
| if err != nil { | ||
| continue // optimistic. if we can't resolve anything, we'll know at the bottom. | ||
| } | ||
| outputAddrs = append(outputAddrs, resolved...) | ||
| } | ||
|
|
||
| if len(outputAddrs) < 1 { | ||
| return nil, fmt.Errorf("failed to specify addrs: %s", unspecAddrs) | ||
| } | ||
| return outputAddrs, nil | ||
| } | ||
|
|
||
| // interfaceAddresses returns a list of addresses associated with local machine | ||
| // Note: we do not return link local addresses. IP loopback is ok, because we | ||
| // may be connecting to other nodes in the same machine. | ||
| func interfaceAddresses() ([]ma.Multiaddr, error) { | ||
| maddrs, err := InterfaceMultiaddrs() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var out []ma.Multiaddr | ||
| for _, a := range maddrs { | ||
| if !AddrOverNonLocalIP(a) { | ||
| continue | ||
| } | ||
| out = append(out, a) | ||
| } | ||
| return out, nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package manet | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| ma "github.com/multiformats/go-multiaddr" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestResolvingAddrs(t *testing.T) { | ||
| unspec := []ma.Multiaddr{ | ||
| newMultiaddr(t, "/ip4/0.0.0.0/tcp/1234"), | ||
| newMultiaddr(t, "/ip4/1.2.3.4/tcp/1234"), | ||
| newMultiaddr(t, "/ip6/::/tcp/1234"), | ||
| newMultiaddr(t, "/ip6/::100/tcp/1234"), | ||
| } | ||
|
|
||
| iface := []ma.Multiaddr{ | ||
| newMultiaddr(t, "/ip4/127.0.0.1"), | ||
| newMultiaddr(t, "/ip4/10.20.30.40"), | ||
| newMultiaddr(t, "/ip6/::1"), | ||
| newMultiaddr(t, "/ip6/::f"), | ||
| } | ||
|
|
||
| spec := []ma.Multiaddr{ | ||
| newMultiaddr(t, "/ip4/127.0.0.1/tcp/1234"), | ||
| newMultiaddr(t, "/ip4/10.20.30.40/tcp/1234"), | ||
| newMultiaddr(t, "/ip4/1.2.3.4/tcp/1234"), | ||
| newMultiaddr(t, "/ip6/::1/tcp/1234"), | ||
| newMultiaddr(t, "/ip6/::f/tcp/1234"), | ||
| newMultiaddr(t, "/ip6/::100/tcp/1234"), | ||
| } | ||
|
|
||
| actual, err := ResolveUnspecifiedAddresses(unspec, iface) | ||
| require.NoError(t, err) | ||
| require.Equal(t, actual, spec) | ||
|
|
||
| ip4u := []ma.Multiaddr{newMultiaddr(t, "/ip4/0.0.0.0")} | ||
| ip4i := []ma.Multiaddr{newMultiaddr(t, "/ip4/1.2.3.4")} | ||
|
|
||
| ip6u := []ma.Multiaddr{newMultiaddr(t, "/ip6/::")} | ||
| ip6i := []ma.Multiaddr{newMultiaddr(t, "/ip6/::1")} | ||
|
|
||
| if _, err := ResolveUnspecifiedAddress(ip4u[0], ip6i); err == nil { | ||
| t.Fatal("should have failed") | ||
| } | ||
| if _, err := ResolveUnspecifiedAddress(ip6u[0], ip4i); err == nil { | ||
| t.Fatal("should have failed") | ||
| } | ||
|
|
||
| if _, err := ResolveUnspecifiedAddresses(ip6u, ip4i); err == nil { | ||
| t.Fatal("should have failed") | ||
| } | ||
| if _, err := ResolveUnspecifiedAddresses(ip4u, ip6i); err == nil { | ||
| t.Fatal("should have failed") | ||
| } | ||
| } | ||
|
|
||
| func TestAddrOverNonLocalIP(t *testing.T) { | ||
| bad := []ma.Multiaddr{ | ||
| newMultiaddr(t, "/ip6/fe80::1/tcp/1234"), // link local | ||
| newMultiaddr(t, "/ip6/fe80::100/tcp/1234"), // link local | ||
| } | ||
| good := []ma.Multiaddr{ | ||
| newMultiaddr(t, "/ip4/127.0.0.1/tcp/1234"), | ||
| newMultiaddr(t, "/ip6/::1/tcp/1234"), | ||
| newMultiaddr(t, "/ip4/1.2.3.4/udp/1234/utp"), | ||
| } | ||
| for _, addr := range bad { | ||
| require.Falsef(t, AddrOverNonLocalIP(addr), "%s is a link local addr", addr) | ||
| } | ||
| for _, addr := range good { | ||
| require.Truef(t, AddrOverNonLocalIP(addr), "%s is not a link local addr", addr) | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a strange function. What about IPv4 localhost?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where we use it: https://github.com/libp2p/go-libp2p-swarm/blob/69f15a70a0caa2ad4f94bfc603f63181b13a40fb/swarm_dial.go#L636-L661
Is there any reason we don't want to dial IPv6 link-local addresses, but are ok with dialing IPv4 localhost?
At least the code here is consistent with the comment in the swarm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because they're almost always useless while localhost can actually be useful (and is usually only one address). NOTE: I don't think we advertise link-local addresses anymore.
Really, this isn't the right place to do things like this, but it's the only place till we get a real address "scope" system.
I'd change it to
IsIpv6LinkLocal, or something like that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I renamed the function. All (the few) users of that functions will need to adjust the path manually anyway, so this doesn't create too much extra hassle.