From 0ddafe603a3ae788f22d933687018fe13f1d903f Mon Sep 17 00:00:00 2001 From: jbenet Date: Mon, 16 May 2016 22:39:39 -0700 Subject: [PATCH] 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 --- blocks/blockstore/blockstore.go | 4 ++++ blocks/blockstore/blockstore_test.go | 8 ++++++++ blockservice/blockservice.go | 5 +++++ blockservice/test/blocks_test.go | 5 +++++ exchange/bitswap/bitswap.go | 3 +++ exchange/bitswap/bitswap_test.go | 13 +++++++++++++ merkledag/merkledag.go | 3 +++ merkledag/merkledag_test.go | 15 +++++++++++++++ 8 files changed, 56 insertions(+) diff --git a/blocks/blockstore/blockstore.go b/blocks/blockstore/blockstore.go index 671ae2c25..d3a9b1aa1 100644 --- a/blocks/blockstore/blockstore.go +++ b/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 diff --git a/blocks/blockstore/blockstore_test.go b/blocks/blockstore/blockstore_test.go index 446d4b776..8b0609f1f 100644 --- a/blocks/blockstore/blockstore_test.go +++ b/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")) diff --git a/blockservice/blockservice.go b/blockservice/blockservice.go index 78838757a..945f60ae6 100644 --- a/blockservice/blockservice.go +++ b/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 { diff --git a/blockservice/test/blocks_test.go b/blockservice/test/blocks_test.go index 584505b21..ed61dad59 100644 --- a/blockservice/test/blocks_test.go +++ b/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) { diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index 9f5c92d04..0afed265e 100644 --- a/exchange/bitswap/bitswap.go +++ b/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 diff --git a/exchange/bitswap/bitswap_test.go b/exchange/bitswap/bitswap_test.go index 024de3389..136fa85d2 100644 --- a/exchange/bitswap/bitswap_test.go +++ b/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) diff --git a/merkledag/merkledag.go b/merkledag/merkledag.go index 938b71308..b98fdafe6 100644 --- a/merkledag/merkledag.go +++ b/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") } diff --git a/merkledag/merkledag_test.go b/merkledag/merkledag_test.go index e475fa680..2739e2195 100644 --- a/merkledag/merkledag_test.go +++ b/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")}