From 555a0b2ead32da70eb9cc94531ccfb8f682f052a Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 7 Feb 2020 13:48:34 -0800 Subject: [PATCH] fix: fix a panic when deleting If we try deleting a bunch of files _before_ reading them, we'll panic because they haven't been loaded. This works around that by using the _link_ directly when the child hasn't been loaded. fixes https://github.com/ipfs/go-ipfs/issues/6860 This commit was moved from ipfs/go-unixfs@af8709db62ebcce86b400fb3e1cd75223ef31a15 --- unixfs/hamt/hamt.go | 32 +++++++++++++++++++++++++++++--- unixfs/hamt/hamt_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/unixfs/hamt/hamt.go b/unixfs/hamt/hamt.go index 6b89b47c7..374f47df2 100644 --- a/unixfs/hamt/hamt.go +++ b/unixfs/hamt/hamt.go @@ -489,10 +489,28 @@ func (ds *Shard) modifyValue(ctx context.Context, hv *hashBits, key string, val // structures, this will help to normalize them. return ds.childer.rm(idx) case 1: - nchild := child.childer.children[0] - if nchild.isValueNode() { + // The single child _should_ be a value by + // induction. However, we allow for it to be a + // shard in case an implementation is broken. + + // Have we loaded the child? Prefer that. + schild := child.childer.child(0) + if schild != nil { + if schild.isValueNode() { + ds.childer.set(schild, i) + } + return nil + } + + // Otherwise, work with the link. + slnk := child.childer.link(0) + lnkType, err := child.childer.sd.childLinkType(slnk) + if err != nil { + return err + } + if lnkType == shardValueLink { // sub-shard with a single value element, collapse it - ds.childer.set(nchild, i) + ds.childer.setLink(slnk, i) } return nil } @@ -517,6 +535,8 @@ type childer struct { sd *Shard dserv ipld.DAGService bitfield bitfield.Bitfield + + // Only one of links/children will be non-nil for every child/link. links []*ipld.Link children []*Shard } @@ -572,6 +592,12 @@ func (s *childer) insert(key string, lnk *ipld.Link, idx int) error { func (s *childer) set(sd *Shard, i int) { s.children[i] = sd + s.links[i] = nil +} + +func (s *childer) setLink(lnk *ipld.Link, i int) { + s.children[i] = nil + s.links[i] = lnk } func (s *childer) rm(childIndex int) error { diff --git a/unixfs/hamt/hamt_test.go b/unixfs/hamt/hamt_test.go index 4025bfb37..4d2f64b22 100644 --- a/unixfs/hamt/hamt_test.go +++ b/unixfs/hamt/hamt_test.go @@ -268,6 +268,45 @@ func TestRemoveElems(t *testing.T) { } } +func TestRemoveAfterMarshal(t *testing.T) { + ds := mdtest.Mock() + dirs, s, err := makeDir(ds, 500) + if err != nil { + t.Fatal(err) + } + nd, err := s.Node() + if err != nil { + t.Fatal(err) + } + + s, err = NewHamtFromDag(ds, nd) + + ctx := context.Background() + + shuffle(time.Now().UnixNano(), dirs) + + for i, d := range dirs { + err := s.Remove(ctx, d) + if err != nil { + t.Fatalf("%d/%d: %s", i, len(dirs), err) + } + } + + nd, err = s.Node() + if err != nil { + t.Fatal(err) + } + + if len(nd.Links()) > 0 { + t.Fatal("shouldnt have any links here") + } + + err = s.Remove(ctx, "doesnt exist") + if err != os.ErrNotExist { + t.Fatal("expected error does not exist") + } +} + func TestSetAfterMarshal(t *testing.T) { ds := mdtest.Mock() _, s, err := makeDir(ds, 300)