From 628b0f6f68e5028d48ef458e0b7416d6b72d4cea Mon Sep 17 00:00:00 2001
From: Jorropo <jorropo.pgm@gmail.com>
Date: Tue, 24 Sep 2024 19:15:25 +0200
Subject: [PATCH] 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 <schomatis@gmail.com>
Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com>
---
 CHANGELOG.md                  |  1 +
 ipld/unixfs/hamt/hamt.go      | 16 +++++++++++-----
 ipld/unixfs/hamt/hamt_test.go | 21 +++++++++++++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

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")
+	}
+}