diff --git a/CHANGELOG.md b/CHANGELOG.md index 7081f01cd..3bea6c343 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The following emojis are used to highlight certain changes: ### Removed ### Fixed += `unixfs/hamt` Log error instead of panic if both link and shard are nil [#393](https://github.com/ipfs/boxo/pull/393) ### Security diff --git a/ipld/unixfs/hamt/hamt.go b/ipld/unixfs/hamt/hamt.go index a57ddad41..455d070c6 100644 --- a/ipld/unixfs/hamt/hamt.go +++ b/ipld/unixfs/hamt/hamt.go @@ -29,8 +29,6 @@ import ( "os" "sync" - "golang.org/x/sync/errgroup" - format "github.com/ipfs/boxo/ipld/unixfs" "github.com/ipfs/boxo/ipld/unixfs/internal" @@ -38,8 +36,12 @@ import ( bitfield "github.com/ipfs/go-bitfield" cid "github.com/ipfs/go-cid" ipld "github.com/ipfs/go-ipld-format" + logging "github.com/ipfs/go-log/v2" + "golang.org/x/sync/errgroup" ) +var log = logging.Logger("unixfs-hamt") + const ( // HashMurmur3 is the multiformats identifier for Murmur3 HashMurmur3 uint64 = 0x22 @@ -430,8 +432,13 @@ type listCidsAndShards struct { func (ds *Shard) walkChildren(processLinkValues func(formattedLink *ipld.Link) error) (*listCidsAndShards, error) { res := &listCidsAndShards{} - for idx, lnk := range ds.childer.links { - if nextShard := ds.childer.children[idx]; nextShard == nil { + for i, nextShard := range ds.childer.children { + if nextShard == nil { + lnk := ds.childer.link(i) + if lnk == nil { + log.Warnf("internal HAMT error: both link and shard nil at pos %d, dumping shard: %+v", i, *ds) + return nil, fmt.Errorf("internal HAMT error: both link and shard nil, check log") + } lnkLinkType, err := ds.childLinkType(lnk) if err != nil { return nil, err @@ -454,7 +461,6 @@ func (ds *Shard) walkChildren(processLinkValues func(formattedLink *ipld.Link) e default: return nil, errors.New("unsupported shard link type") } - } else { if nextShard.val != nil { formattedLink := &ipld.Link{ diff --git a/ipld/unixfs/hamt/hamt_test.go b/ipld/unixfs/hamt/hamt_test.go index 1946bbd7c..16b325773 100644 --- a/ipld/unixfs/hamt/hamt_test.go +++ b/ipld/unixfs/hamt/hamt_test.go @@ -749,3 +749,24 @@ func TestHamtBadSize(t *testing.T) { } } } + +func TestHamtNilLinkAndShard(t *testing.T) { + shard, err := NewShard(nil, 1024) + if err != nil { + t.Fatal(err) + } + shard.childer = shard.childer.makeChilder(nil, []*ipld.Link{nil}) + nextShard, err := shard.walkChildren(func(_ *ipld.Link) error { + t.Fatal("processLinkValues function should not have been called") + return nil + }) + if err == nil { + t.Fatal("expected error") + } + if err.Error() != "internal HAMT error: both link and shard nil, check log" { + t.Fatal("did not get expected error") + } + if nextShard != nil { + t.Fatal("nextShard should be nil") + } +}