ipld/unixfs/hamt: catch panic in walkChildren (#393)

* ipld/unixfs/hamt: catch panic in walkChildren * Add test for nil link and shard * rename test * Update CHANGELOG.md --------- Co-authored-by: Lucas Molas <[email protected]> Co-authored-by: Andrew Gillis <[email protected]>
ipfs · Sep 24, 2024 · 628b0f6 · 628b0f6
1 parent 171b0b7
commit 628b0f6
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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")
+	}
+}