Skip to content

Commit

Permalink
ipld/unixfs/hamt: catch panic in walkChildren (#393)
Browse files Browse the repository at this point in the history
* ipld/unixfs/hamt: catch panic in walkChildren

* Add test for nil link and shard

* rename test

* Update CHANGELOG.md

---------

Co-authored-by: Lucas Molas <schomatis@gmail.com>
Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com>
  • Loading branch information
3 people authored Sep 24, 2024
1 parent 171b0b7 commit 628b0f6
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 11 additions & 5 deletions ipld/unixfs/hamt/hamt.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,19 @@ import (
"os"
"sync"

"golang.org/x/sync/errgroup"

format "github.com/ipfs/boxo/ipld/unixfs"
"github.com/ipfs/boxo/ipld/unixfs/internal"

dag "github.com/ipfs/boxo/ipld/merkledag"
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
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand Down
21 changes: 21 additions & 0 deletions ipld/unixfs/hamt/hamt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

0 comments on commit 628b0f6

Please sign in to comment.