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/core/commands/object/patch.go b/core/commands/object/patch.go index a0a635d20..f100f1862 100644 --- a/core/commands/object/patch.go +++ b/core/commands/object/patch.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "strings" - key "github.com/ipfs/go-ipfs/blocks/key" cmds "github.com/ipfs/go-ipfs/commands" core "github.com/ipfs/go-ipfs/core" dag "github.com/ipfs/go-ipfs/merkledag" @@ -273,8 +272,12 @@ a file containing 'bar', and returns the hash of the new object. return } - path := req.Arguments()[1] - childk := key.B58KeyDecode(req.Arguments()[2]) + npath := req.Arguments()[1] + childp, err := path.ParsePath(req.Arguments()[2]) + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } create, _, err := req.Option("create").Bool() if err != nil { @@ -291,13 +294,13 @@ a file containing 'bar', and returns the hash of the new object. e := dagutils.NewDagEditor(root, nd.DAG) - childnd, err := nd.DAG.Get(req.Context(), childk) + childnd, err := core.Resolve(req.Context(), nd, childp) if err != nil { res.SetError(err, cmds.ErrNormal) return } - err = e.InsertNodeAtPath(req.Context(), path, childnd, createfunc) + err = e.InsertNodeAtPath(req.Context(), npath, childnd, createfunc) if err != nil { res.SetError(err, cmds.ErrNormal) return diff --git a/exchange/bitswap/bitswap.go b/exchange/bitswap/bitswap.go index d5eb882b8..3e060a76c 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 7ef2a0d96..31d4f74ae 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..430e31f7a 100644 --- a/merkledag/merkledag_test.go +++ b/merkledag/merkledag_test.go @@ -245,6 +245,14 @@ func assertCanGet(t *testing.T, ds DAGService, n *Node) { } } +func TestEmptyKey(t *testing.T) { + ds := dstest.Mock() + _, 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")} diff --git a/test/sharness/t0051-object.sh b/test/sharness/t0051-object.sh index 0506235b2..c47bf90f2 100755 --- a/test/sharness/t0051-object.sh +++ b/test/sharness/t0051-object.sh @@ -180,6 +180,22 @@ test_object_cmd() { ipfs object stat $OUTPUT ' + test_expect_success "'ipfs object patch add-link' should work with paths" ' + EMPTY_DIR=$(ipfs object new unixfs-dir) && + N1=$(ipfs object patch $EMPTY_DIR add-link baz $EMPTY_DIR) && + N2=$(ipfs object patch $EMPTY_DIR add-link bar $N1) && + N3=$(ipfs object patch $EMPTY_DIR add-link foo /ipfs/$N2/bar) && + ipfs object stat /ipfs/$N3 > /dev/null && + ipfs object stat $N3/foo > /dev/null && + ipfs object stat /ipfs/$N3/foo/baz > /dev/null + ' + + test_expect_success "object patch creation looks right" ' + echo "QmPc73aWK9dgFBXe86P4PvQizHo9e5Qt7n7DAMXWuigFuG" > hash_exp && + echo $N3 > hash_actual && + test_cmp hash_exp hash_actual + ' + test_expect_success "multilayer ipfs patch works" ' echo "hello world" > hwfile && FILE=$(ipfs add -q hwfile) &&