add error checking for nil keys

Checks in:
- blockstore
- blockservice
- dagservice
- bitswap

Do not anger the pokemans #2715

License: MIT
Signed-off-by: Juan Benet <juan@benet.ai>

blocks/blockstore/blockstore.go

@@ -74,6 +74,10 @@ type blockstore struct {
 }
 
 func (bs *blockstore) Get(k key.Key) (blocks.Block, error) {
+	if k == "" {
+		return nil, ErrNotFound
+	}
+
 	maybeData, err := bs.datastore.Get(k.DsKey())
 	if err == ds.ErrNotFound {
 		return nil, ErrNotFound

blocks/blockstore/blockstore_test.go

@@ -27,6 +27,14 @@ func TestGetWhenKeyNotPresent(t *testing.T) {
 	t.Fail()
 }
 
+func TestGetWhenKeyIsEmptyString(t *testing.T) {
+	bs := NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore()))
+	_, err := bs.Get(key.Key(""))
+	if err != ErrNotFound {
+		t.Fail()
+	}
+}
+
 func TestPutThenGetBlock(t *testing.T) {
 	bs := NewBlockstore(ds_sync.MutexWrap(ds.NewMapDatastore()))
 	block := blocks.NewBlock([]byte("some data"))

blockservice/blockservice.go

@@ -72,6 +72,11 @@ func (s *BlockService) AddBlocks(bs []blocks.Block) ([]key.Key, error) {
 // GetBlock retrieves a particular block from the service,
 // Getting it from the datastore using the key (hash).
 func (s *BlockService) GetBlock(ctx context.Context, k key.Key) (blocks.Block, error) {
+	if k == "" {
+		log.Debug("BlockService GetBlock: Nil Key")
+		return nil, ErrNotFound
+	}
+
 	log.Debugf("BlockService GetBlock: '%s'", k)
 	block, err := s.Blockstore.Get(k)
 	if err == nil {

blockservice/test/blocks_test.go

@@ -22,6 +22,11 @@ func TestBlocks(t *testing.T) {
 	bs := New(bstore, offline.Exchange(bstore))
 	defer bs.Close()
 
+	_, err := bs.GetBlock(context.Background(), key.Key(""))
+	if err != ErrNotFound {
+		t.Error("Empty String Key should error", err)
+	}
+
 	b := blocks.NewBlock([]byte("beep boop"))
 	h := u.Hash([]byte("beep boop"))
 	if !bytes.Equal(b.Multihash(), h) {

exchange/bitswap/bitswap.go

@@ -155,6 +155,9 @@ type blockRequest struct {
 // GetBlock attempts to retrieve a particular block from peers within the
 // deadline enforced by the context.
 func (bs *Bitswap) GetBlock(parent context.Context, k key.Key) (blocks.Block, error) {
+	if k == "" {
+		return nil, blockstore.ErrNotFound
+	}
 
 	// Any async work initiated by this function must end when this function
 	// returns. To ensure this, derive a new context. Note that it is okay to

exchange/bitswap/bitswap_test.go

@@ -11,6 +11,7 @@ import (
 	context "gx/ipfs/QmZy2y8t9zQH2a1b8q2ZSLKp17ATuJoCNxxyMFG5qFExpt/go-net/context"
 
 	blocks "github.com/ipfs/go-ipfs/blocks"
+	blockstore "github.com/ipfs/go-ipfs/blocks/blockstore"
 	blocksutil "github.com/ipfs/go-ipfs/blocks/blocksutil"
 	key "github.com/ipfs/go-ipfs/blocks/key"
 	tn "github.com/ipfs/go-ipfs/exchange/bitswap/testnet"
@@ -278,6 +279,18 @@ func TestSendToWantingPeer(t *testing.T) {
 
 }
 
+func TestEmptyKey(t *testing.T) {
+	net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(kNetworkDelay))
+	sg := NewTestSessionGenerator(net)
+	defer sg.Close()
+	bs := sg.Instances(1)[0].Exchange
+
+	_, err := bs.GetBlock(context.Background(), key.Key(""))
+	if err != blockstore.ErrNotFound {
+		t.Error("empty str key should return ErrNotFound")
+	}
+}
+
 func TestBasicBitswap(t *testing.T) {
 	net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(kNetworkDelay))
 	sg := NewTestSessionGenerator(net)

merkledag/merkledag.go

@@ -68,6 +68,9 @@ func (n *dagService) Batch() *Batch {
 
 // Get retrieves a node from the dagService, fetching the block in the BlockService
 func (n *dagService) Get(ctx context.Context, k key.Key) (*Node, error) {
+	if k == "" {
+		return nil, ErrNotFound
+	}
 	if n == nil {
 		return nil, fmt.Errorf("dagService is nil")
 	}

merkledag/merkledag_test.go

@@ -32,6 +32,13 @@ type dagservAndPinner struct {
 	mp pin.Pinner
 }
 
+func getDagserv(t *testing.T) DAGService {
+	db := dssync.MutexWrap(ds.NewMapDatastore())
+	bs := bstore.NewBlockstore(db)
+	blockserv := bserv.New(bs, offline.Exchange(bs))
+	return NewDAGService(blockserv)
+}
+
 func getDagservAndPinner(t *testing.T) dagservAndPinner {
 	db := dssync.MutexWrap(ds.NewMapDatastore())
 	bs := bstore.NewBlockstore(db)
@@ -245,6 +252,14 @@ func assertCanGet(t *testing.T, ds DAGService, n *Node) {
 	}
 }
 
+func TestEmptyKey(t *testing.T) {
+	ds := getDagserv(t)
+	_, err := ds.Get(context.Background(), key.Key(""))
+	if err != ErrNotFound {
+		t.Error("dag service should error when key is nil", err)
+	}
+}
+
 func TestCantGet(t *testing.T) {
 	dsp := getDagservAndPinner(t)
 	a := &Node{Data: []byte("A")}