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

Attempt to lookup an empty peer ID #557

Open
Stebalien opened this issue Apr 6, 2020 · 2 comments
Open

Attempt to lookup an empty peer ID #557

Stebalien opened this issue Apr 6, 2020 · 2 comments
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@Stebalien
Copy link
Member

=== FAIL: core/coreapi/test TestIface/Name/TestBasicPublishResolveKey (14.84s)
panic: runtime error: index out of range [-1]

goroutine 82879 [running]:
github.com/libp2p/go-libp2p-peerstore/pstoremem.(*addrSegments).get(...)
	pkg/mod/github.com/libp2p/go-libp2p-peerstore@v0.2.2/pstoremem/addr_book.go:50
github.com/libp2p/go-libp2p-peerstore/pstoremem.(*memoryAddrBook).Addrs(0xc0036da000, 0x0, 0x0, 0x0, 0x0, 0x0)
	pkg/mod/github.com/libp2p/go-libp2p-peerstore@v0.2.2/pstoremem/addr_book.go:313 +0x168
github.com/libp2p/go-libp2p-peerstore/pstoremem.(*pstoremem).PeerInfo(0xc0036ed560, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	pkg/mod/github.com/libp2p/go-libp2p-peerstore@v0.2.2/pstoremem/peerstore.go:71 +0x66
github.com/libp2p/go-libp2p-peerstore.PeerInfos(0x1bf85e0, 0xc0036ed560, 0xc005825a40, 0x4, 0x4, 0xc005825a40, 0x3, 0x4)
	pkg/mod/github.com/libp2p/go-libp2p-peerstore@v0.2.2/peerstore.go:82 +0xbc
github.com/libp2p/go-libp2p-kad-dht.(*IpfsDHT).handleFindPeer(0xc0036af500, 0x1bde300, 0xc0061e76e0, 0xc000e6ec30, 0x22, 0xc005880d00, 0x1690ba0, 0x175e201, 0xc0065ba0b0)
	pkg/mod/github.com/libp2p/go-libp2p-kad-dht@v0.6.1/handlers.go:289 +0x1f6
github.com/libp2p/go-libp2p-kad-dht.(*IpfsDHT).handleNewMessage(0xc0036af500, 0x1bf14e0, 0xc004eedaa0, 0xa22c00)
	pkg/mod/github.com/libp2p/go-libp2p-kad-dht@v0.6.1/dht_net.go:151 +0x82a
github.com/libp2p/go-libp2p-kad-dht.(*IpfsDHT).handleNewStream(0xc0036af500, 0x1bf14e0, 0xc004eedaa0)
	pkg/mod/github.com/libp2p/go-libp2p-kad-dht@v0.6.1/dht_net.go:66 +0x83
github.com/libp2p/go-libp2p/p2p/host/basic.(*BasicHost).SetStreamHandler.func1(0xc001139200, 0xf, 0x7effdc6b9030, 0xc004eedaa0, 0x1, 0x0)
	pkg/mod/github.com/libp2p/go-libp2p@v0.7.4/p2p/host/basic/basic_host.go:417 +0x9d
created by github.com/libp2p/go-libp2p/p2p/host/basic.(*BasicHost).newStreamHandler
	pkg/mod/github.com/libp2p/go-libp2p@v0.7.4/p2p/host/basic/basic_host.go:301 +0x636

FAIL	github.com/ipfs/go-ipfs/core/coreapi/test	14.840s

This error happens when we try to lookup an empty ("") peer ID. See: libp2p/go-libp2p-peerstore#88

https://app.circleci.com/pipelines/github/ipfs/go-ipfs/2340/workflows/be5e6996-eb05-49c2-9b1b-7e26c3719c06/jobs/28662

@Stebalien Stebalien added the kind/bug A bug in existing code (including security flaws) label Apr 6, 2020
@Stebalien
Copy link
Member Author

We should probably just validate that all messages actually have a key in the main handelNewMessage functoin. I'm also not sure why this is showing up in tests now. Are we forgetting to set the key in one of our requests?

@aschmahmann
Copy link
Contributor

aschmahmann commented Apr 6, 2020

@Stebalien my guess is that the change that exposed this error was that we now search the peerstore for targetPid and return it, where as previously we only returned targetPid if we were Connected to it.

if !found {
closest = append(closest, targetPid)
}
}
}
if closest == nil {
return resp, nil
}
// TODO: pstore.PeerInfos should move to core (=> peerstore.AddrInfos).
closestinfos := pstore.PeerInfos(dht.peerstore, closest)
.

We should investigate why this is happening at all though.

Edit: After some spelunking I think the reason that there's a peer.ID of "" being propagated around is because https://github.com/ipfs/go-ipfs-provider/blob/3a98ef95f2e412a39a7b591cc2ce3ed5affb18c3/simple/provider.go#L88-L89 does not check if the provider dequeing channel is closed before calling doProvide(c cid.Cid) this causes an empty cid to be provided when the test ends, which causes an empty key to be looked for in GetClosestPeers which because of our weirdness in #556 looks like we're actually searching for an empty peerID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

2 participants