fix(commands): prevent panic from typed nil DHTClient interface

Fixes panic when ipfsNode.DHTClient is a non-nil interface containing a nil pointer value (typed nil). This happened when Routing.Type=delegated or when using HTTP-only routing without DHT. The panic occurred because: - Go interfaces can be non-nil while containing nil pointer values - Simple `if DHTClient == nil` checks pass, but calling methods panics - Example: `(*ddht.DHT)(nil)` stored in interface passes nil check Solution: - Add HasActiveDHTClient() method to check both interface and concrete value - Update all 7 call sites to use proper check before DHT operations - Rename provideRoot → provideCIDSync for clarity - Add structured logging with "fast-provide" prefix for easier filtering - Add tests covering nil cases and valid DHT configurations Fixes: https://github.com/ipfs/kubo/pull/11046#issuecomment-3525313349
2026-02-21 10:27:46 +08:00 · 2025-11-13 21:22:39 +01:00 · 2025-11-13 21:22:39 +01:00 · 0f720a070c
commit 0f720a070c
parent caab4c7951
8 changed files with 227 additions and 18 deletions
--- a/core/commands/add.go
+++ b/core/commands/add.go
@ -214,7 +214,7 @@ https://github.com/ipfs/kubo/blob/master/docs/config.md#import
 		cmds.UintOption(modeOptionName, "Custom POSIX file mode to store in created UnixFS entries. WARNING: experimental, forces dag-pb for root block, disables raw-leaves"),
 		cmds.Int64Option(mtimeOptionName, "Custom POSIX modification time to store in created UnixFS entries (seconds before or after the Unix Epoch). WARNING: experimental, forces dag-pb for root block, disables raw-leaves"),
 		cmds.UintOption(mtimeNsecsOptionName, "Custom POSIX modification time (optional time fraction in nanoseconds)"),
-		cmds.BoolOption(fastProvideOptionName, "Apply fast-provide function to the root CID after add completes").WithDefault(true), // TODO: default could be Provide.DHT.SweepEnabled
+		cmds.BoolOption(fastProvideOptionName, "Immediately announce root CID to DHT for fast content discovery. Optimized for accelerated/sweep/optimistic DHT. Skipped if DHT unavailable.").WithDefault(true),
 	},
 	PreRun: func(req *cmds.Request, env cmds.Environment) error {
 		quiet, _ := req.Options[quietOptionName].(bool)
@ -569,10 +569,18 @@ https://github.com/ipfs/kubo/blob/master/docs/config.md#import

 		// Apply fast-provide if the flag is enabled
 		if fastProvide && (lastRootCid != path.ImmutablePath{}) {
-			// Blocks until root CID is provided to the DHT.
-			// TODO: consider logging that fast-provide is in progress for user
-			if err := provideRoot(req.Context, ipfsNode.DHTClient, lastRootCid.RootCid()); err != nil {
-				log.Warnf("fast-provide failed for root CID %s: %s", lastRootCid.String(), err)
+			// Check if DHT is available before attempting to provide
+			if ipfsNode.HasActiveDHTClient() {
+				// Blocks until root CID is provided to the DHT
+				rootCid := lastRootCid.RootCid()
+				log.Debugw("fast-provide: announcing root CID", "cid", rootCid)
+				if err := provideCIDSync(req.Context, ipfsNode.DHTClient, rootCid); err != nil {
+					log.Warnw("fast-provide: failed", "cid", rootCid, "error", err)
+				} else {
+					log.Debugw("fast-provide: completed", "cid", rootCid)
+				}
+			} else {
+				log.Debugw("fast-provide: skipped", "reason", "DHT not available")
 			}
 		}

--- a/core/commands/dht.go
+++ b/core/commands/dht.go
@ -56,7 +56,7 @@ var queryDhtCmd = &cmds.Command{
 			return err
 		}

-		if nd.DHTClient == nil {
+		if !nd.HasActiveDHTClient() {
 			return ErrNotDHT
 		}

@ -70,7 +70,7 @@ var queryDhtCmd = &cmds.Command{
 		ctx, events := routing.RegisterForQueryEvents(ctx)

 		client := nd.DHTClient
-		if client == nd.DHT {
+		if nd.DHT != nil && client == nd.DHT {
 			client = nd.DHT.WAN
 			if !nd.DHT.WANActive() {
 				client = nd.DHT.LAN
--- a/core/commands/provide.go
+++ b/core/commands/provide.go
@ -579,17 +579,18 @@ func humanFull(val float64, decimals int) string {
 	return humanize.CommafWithDigits(val, decimals)
 }

-// provideRoot performs a provide operation on the supplied DHT client for the
-// given CID.
+// provideCIDSync performs a synchronous/blocking provide operation to announce
+// the given CID to the DHT.
 //
 //   - If the accelerated DHT client is used, a DHT lookup isn't needed, we
 //     directly allocate provider records to closest peers.
 //   - If Provide.DHT.SweepEnabled=true or OptimisticProvide=true, we make an
 //     optimistic provide call.
 //   - Else we make a standard provide call (much slower).
-func provideRoot(ctx context.Context, router routing.Routing, c cid.Cid) error {
-	if router == nil {
-		return nil
-	}
+//
+// IMPORTANT: The caller MUST verify DHT availability using HasActiveDHTClient()
+// before calling this function. Calling with a nil or invalid router will cause
+// a panic - this is the caller's responsibility to prevent.
+func provideCIDSync(ctx context.Context, router routing.Routing, c cid.Cid) error {
 	return router.Provide(ctx, c, true)
 }
--- a/core/commands/routing.go
+++ b/core/commands/routing.go
@ -230,12 +230,12 @@ var provideRefRoutingCmd = &cmds.Command{
 			}
 		}()

-		if dhtClient := nd.DHTClient; dhtClient != nil {
+		if nd.HasActiveDHTClient() {
 			// If node has a DHT client, provide immediately the supplied cids before
 			// returning.
 			for _, c := range cids {
-				if err = provideRoot(req.Context, dhtClient, c); err != nil {
-					return fmt.Errorf("error providing root cid: %w", err)
+				if err = provideCIDSync(req.Context, nd.DHTClient, c); err != nil {
+					return fmt.Errorf("error providing cid: %w", err)
 				}
 			}
 		}
--- a/core/commands/stat_dht.go
+++ b/core/commands/stat_dht.go
@ -75,7 +75,8 @@ This interface is not stable and may change from release to release.
 			var dht *dht.IpfsDHT

 			var separateClient bool
-			if nd.DHTClient != nd.DHT {
+			// Check if using separate DHT client (e.g., accelerated DHT)
+			if nd.HasActiveDHTClient() && nd.DHTClient != nd.DHT {
 				separateClient = true
 			}

--- a/core/commands/version.go
+++ b/core/commands/version.go
@ -255,7 +255,7 @@ func DetectNewKuboVersion(nd *core.IpfsNode, minPercent int64) (VersionCheckOutp
 	}

 	// Amino DHT client keeps information about previously seen peers
-	if nd.DHTClient != nd.DHT && nd.DHTClient != nil {
+	if nd.HasActiveDHTClient() && nd.DHTClient != nd.DHT {
 		client, ok := nd.DHTClient.(*fullrt.FullRT)
 		if !ok {
 			return VersionCheckOutput{}, errors.New("could not perform version check due to missing or incompatible DHT configuration")
--- a/core/core.go
+++ b/core/core.go
@ -30,9 +30,11 @@ import (
 	ipld "github.com/ipfs/go-ipld-format"
 	logging "github.com/ipfs/go-log/v2"
 	ddht "github.com/libp2p/go-libp2p-kad-dht/dual"
+	"github.com/libp2p/go-libp2p-kad-dht/fullrt"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
 	psrouter "github.com/libp2p/go-libp2p-pubsub-router"
 	record "github.com/libp2p/go-libp2p-record"
+	routinghelpers "github.com/libp2p/go-libp2p-routing-helpers"
 	connmgr "github.com/libp2p/go-libp2p/core/connmgr"
 	ic "github.com/libp2p/go-libp2p/core/crypto"
 	p2phost "github.com/libp2p/go-libp2p/core/host"
@ -143,6 +145,42 @@ func (n *IpfsNode) Close() error {
 	return n.stop()
 }

+// HasActiveDHTClient checks if the node's DHT client is active and usable for DHT operations.
+//
+// Returns false for:
+//   - nil DHTClient
+//   - typed nil pointers (e.g., (*ddht.DHT)(nil))
+//   - no-op routers (routinghelpers.Null)
+//
+// Note: This method only checks for known DHT client types (ddht.DHT, fullrt.FullRT).
+// Custom routing.Routing implementations are not explicitly validated.
+//
+// This method prevents the "typed nil interface" bug where an interface contains
+// a nil pointer of a concrete type, which passes nil checks but panics when methods
+// are called.
+func (n *IpfsNode) HasActiveDHTClient() bool {
+	if n.DHTClient == nil {
+		return false
+	}
+
+	// Check for no-op router (Routing.Type=none)
+	if _, ok := n.DHTClient.(routinghelpers.Null); ok {
+		return false
+	}
+
+	// Check for typed nil *ddht.DHT (common when Routing.Type=delegated or HTTP-only)
+	if d, ok := n.DHTClient.(*ddht.DHT); ok && d == nil {
+		return false
+	}
+
+	// Check for typed nil *fullrt.FullRT (accelerated DHT client)
+	if f, ok := n.DHTClient.(*fullrt.FullRT); ok && f == nil {
+		return false
+	}
+
+	return true
+}
+
 // Context returns the IpfsNode context
 func (n *IpfsNode) Context() context.Context {
 	if n.ctx == nil {
--- a/core/core_test.go
+++ b/core/core_test.go
@ -1,15 +1,28 @@
 package core

 import (
+	"os"
+	"path/filepath"
 	"testing"

 	context "context"

 	"github.com/ipfs/kubo/repo"

+	"github.com/ipfs/boxo/filestore"
+	"github.com/ipfs/boxo/keystore"
 	datastore "github.com/ipfs/go-datastore"
 	syncds "github.com/ipfs/go-datastore/sync"
 	config "github.com/ipfs/kubo/config"
+	"github.com/ipfs/kubo/core/node/libp2p"
+	golib "github.com/libp2p/go-libp2p"
+	ddht "github.com/libp2p/go-libp2p-kad-dht/dual"
+	"github.com/libp2p/go-libp2p-kad-dht/fullrt"
+	routinghelpers "github.com/libp2p/go-libp2p-routing-helpers"
+	"github.com/libp2p/go-libp2p/core/host"
+	"github.com/libp2p/go-libp2p/core/peer"
+	pstore "github.com/libp2p/go-libp2p/core/peerstore"
+	mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
 )

 func TestInitialization(t *testing.T) {
@ -65,3 +78,151 @@ var testIdentity = config.Identity{
 	PeerID:  "QmNgdzLieYi8tgfo2WfTUzNVH5hQK9oAYGVf6dxN12NrHt",
 	PrivKey: "CAASrRIwggkpAgEAAoICAQCwt67GTUQ8nlJhks6CgbLKOx7F5tl1r9zF4m3TUrG3Pe8h64vi+ILDRFd7QJxaJ/n8ux9RUDoxLjzftL4uTdtv5UXl2vaufCc/C0bhCRvDhuWPhVsD75/DZPbwLsepxocwVWTyq7/ZHsCfuWdoh/KNczfy+Gn33gVQbHCnip/uhTVxT7ARTiv8Qa3d7qmmxsR+1zdL/IRO0mic/iojcb3Oc/PRnYBTiAZFbZdUEit/99tnfSjMDg02wRayZaT5ikxa6gBTMZ16Yvienq7RwSELzMQq2jFA4i/TdiGhS9uKywltiN2LrNDBcQJSN02pK12DKoiIy+wuOCRgs2NTQEhU2sXCk091v7giTTOpFX2ij9ghmiRfoSiBFPJA5RGwiH6ansCHtWKY1K8BS5UORM0o3dYk87mTnKbCsdz4bYnGtOWafujYwzueGx8r+IWiys80IPQKDeehnLW6RgoyjszKgL/2XTyP54xMLSW+Qb3BPgDcPaPO0hmop1hW9upStxKsefW2A2d46Ds4HEpJEry7PkS5M4gKL/zCKHuxuXVk14+fZQ1rstMuvKjrekpAC2aVIKMI9VRA3awtnje8HImQMdj+r+bPmv0N8rTTr3eS4J8Yl7k12i95LLfK+fWnmUh22oTNzkRlaiERQrUDyE4XNCtJc0xs1oe1yXGqazCIAQIDAQABAoICAQCk1N/ftahlRmOfAXk//8wNl7FvdJD3le6+YSKBj0uWmN1ZbUSQk64chr12iGCOM2WY180xYjy1LOS44PTXaeW5bEiTSnb3b3SH+HPHaWCNM2EiSogHltYVQjKW+3tfH39vlOdQ9uQ+l9Gh6iTLOqsCRyszpYPqIBwi1NMLY2Ej8PpVU7ftnFWouHZ9YKS7nAEiMoowhTu/7cCIVwZlAy3AySTuKxPMVj9LORqC32PVvBHZaMPJ+X1Xyijqg6aq39WyoztkXg3+Xxx5j5eOrK6vO/Lp6ZUxaQilHDXoJkKEJjgIBDZpluss08UPfOgiWAGkW+L4fgUxY0qDLDAEMhyEBAn6KOKVL1JhGTX6GjhWziI94bddSpHKYOEIDzUy4H8BXnKhtnyQV6ELS65C2hj9D0IMBTj7edCF1poJy0QfdK0cuXgMvxHLeUO5uc2YWfbNosvKxqygB9rToy4b22YvNwsZUXsTY6Jt+p9V2OgXSKfB5VPeRbjTJL6xqvvUJpQytmII/C9JmSDUtCbYceHj6X9jgigLk20VV6nWHqCTj3utXD6NPAjoycVpLKDlnWEgfVELDIk0gobxUqqSm3jTPEKRPJgxkgPxbwxYumtw++1UY2y35w3WRDc2xYPaWKBCQeZy+mL6ByXp9bWlNvxS3Knb6oZp36/ovGnf2pGvdQKCAQEAyKpipz2lIUySDyE0avVWAmQb2tWGKXALPohzj7AwkcfEg2GuwoC6GyVE2sTJD1HRazIjOKn3yQORg2uOPeG7sx7EKHxSxCKDrbPawkvLCq8JYSy9TLvhqKUVVGYPqMBzu2POSLEA81QXas+aYjKOFWA2Zrjq26zV9ey3+6Lc6WULePgRQybU8+RHJc6fdjUCCfUxgOrUO2IQOuTJ+FsDpVnrMUGlokmWn23OjL4qTL9wGDnWGUs2pjSzNbj3qA0d8iqaiMUyHX/D/VS0wpeT1osNBSm8suvSibYBn+7wbIApbwXUxZaxMv2OHGz3empae4ckvNZs7r8wsI9UwFt8mwKCAQEA4XK6gZkv9t+3YCcSPw2ensLvL/xU7i2bkC9tfTGdjnQfzZXIf5KNdVuj/SerOl2S1s45NMs3ysJbADwRb4ahElD/V71nGzV8fpFTitC20ro9fuX4J0+twmBolHqeH9pmeGTjAeL1rvt6vxs4FkeG/yNft7GdXpXTtEGaObn8Mt0tPY+aB3UnKrnCQoQAlPyGHFrVRX0UEcp6wyyNGhJCNKeNOvqCHTFObhbhO+KWpWSN0MkVHnqaIBnIn1Te8FtvP/iTwXGnKc0YXJUG6+LM6LmOguW6tg8ZqiQeYyyR+e9eCFH4csLzkrTl1GxCxwEsoSLIMm7UDcjttW6tYEghkwKCAQEAmeCO5lCPYImnN5Lu71ZTLmI2OgmjaANTnBBnDbi+hgv61gUCToUIMejSdDCTPfwv61P3TmyIZs0luPGxkiKYHTNqmOE9Vspgz8Mr7fLRMNApESuNvloVIY32XVImj/GEzh4rAfM6F15U1sN8T/EUo6+0B/Glp+9R49QzAfRSE2g48/rGwgf1JVHYfVWFUtAzUA+GdqWdOixo5cCsYJbqpNHfWVZN/bUQnBFIYwUwysnC29D+LUdQEQQ4qOm+gFAOtrWU62zMkXJ4iLt8Ify6kbrvsRXgbhQIzzGS7WH9XDarj0eZciuslr15TLMC1Azadf+cXHLR9gMHA13mT9vYIQKCAQA/DjGv8cKCkAvf7s2hqROGYAs6Jp8yhrsN1tYOwAPLRhtnCs+rLrg17M2vDptLlcRuI/vIElamdTmylRpjUQpX7yObzLO73nfVhpwRJVMdGU394iBIDncQ+JoHfUwgqJskbUM40dvZdyjbrqc/Q/4z+hbZb+oN/GXb8sVKBATPzSDMKQ/xqgisYIw+wmDPStnPsHAaIWOtni47zIgilJzD0WEk78/YjmPbUrboYvWziK5JiRRJFA1rkQqV1c0M+OXixIm+/yS8AksgCeaHr0WUieGcJtjT9uE8vyFop5ykhRiNxy9wGaq6i7IEecsrkd6DqxDHWkwhFuO1bSE83q/VAoIBAEA+RX1i/SUi08p71ggUi9WFMqXmzELp1L3hiEjOc2AklHk2rPxsaTh9+G95BvjhP7fRa/Yga+yDtYuyjO99nedStdNNSg03aPXILl9gs3r2dPiQKUEXZJ3FrH6tkils/8BlpOIRfbkszrdZIKTO9GCdLWQ30dQITDACs8zV/1GFGrHFrqnnMe/NpIFHWNZJ0/WZMi8wgWO6Ik8jHEpQtVXRiXLqy7U6hk170pa4GHOzvftfPElOZZjy9qn7KjdAQqy6spIrAE94OEL+fBgbHQZGLpuTlj6w6YGbMtPU8uo7sXKoc6WOCb68JWft3tejGLDa1946HAWqVM9B/UcneNc=",
 }
+
+// mockHostOption creates a HostOption that uses the provided mocknet.
+// Inlined to avoid import cycle with core/mock package.
+func mockHostOption(mn mocknet.Mocknet) libp2p.HostOption {
+	return func(id peer.ID, ps pstore.Peerstore, opts ...golib.Option) (host.Host, error) {
+		var cfg golib.Config
+		if err := cfg.Apply(opts...); err != nil {
+			return nil, err
+		}
+
+		// The mocknet does not use the provided libp2p.Option. This options include
+		// the listening addresses we want our peer listening on. Therefore, we have
+		// to manually parse the configuration and add them here.
+		ps.AddAddrs(id, cfg.ListenAddrs, pstore.PermanentAddrTTL)
+		return mn.AddPeerWithPeerstore(id, ps)
+	}
+}
+
+func TestHasActiveDHTClient(t *testing.T) {
+	// Test 1: nil DHTClient
+	t.Run("nil DHTClient", func(t *testing.T) {
+		node := &IpfsNode{
+			DHTClient: nil,
+		}
+		if node.HasActiveDHTClient() {
+			t.Error("Expected false for nil DHTClient")
+		}
+	})
+
+	// Test 2: Typed nil *ddht.DHT (common case when Routing.Type=delegated)
+	t.Run("typed nil ddht.DHT", func(t *testing.T) {
+		node := &IpfsNode{
+			DHTClient: (*ddht.DHT)(nil),
+		}
+		if node.HasActiveDHTClient() {
+			t.Error("Expected false for typed nil *ddht.DHT")
+		}
+	})
+
+	// Test 3: Typed nil *fullrt.FullRT (accelerated DHT client)
+	t.Run("typed nil fullrt.FullRT", func(t *testing.T) {
+		node := &IpfsNode{
+			DHTClient: (*fullrt.FullRT)(nil),
+		}
+		if node.HasActiveDHTClient() {
+			t.Error("Expected false for typed nil *fullrt.FullRT")
+		}
+	})
+
+	// Test 4: routinghelpers.Null no-op router (Routing.Type=none)
+	t.Run("routinghelpers.Null", func(t *testing.T) {
+		node := &IpfsNode{
+			DHTClient: routinghelpers.Null{},
+		}
+		if node.HasActiveDHTClient() {
+			t.Error("Expected false for routinghelpers.Null")
+		}
+	})
+
+	// Test 5: Valid standard dual DHT (Routing.Type=auto/dht/dhtclient)
+	t.Run("valid standard dual DHT", func(t *testing.T) {
+		ctx := context.Background()
+		mn := mocknet.New()
+		defer mn.Close()
+
+		ds := syncds.MutexWrap(datastore.NewMapDatastore())
+		c := config.Config{}
+		c.Identity = testIdentity
+		c.Addresses.Swarm = []string{"/ip4/0.0.0.0/tcp/4001"}
+
+		r := &repo.Mock{
+			C: c,
+			D: ds,
+			K: keystore.NewMemKeystore(),
+			F: filestore.NewFileManager(ds, filepath.Dir(os.TempDir())),
+		}
+
+		node, err := NewNode(ctx, &BuildCfg{
+			Routing: libp2p.DHTServerOption,
+			Repo:    r,
+			Host:    mockHostOption(mn),
+			Online:  true,
+		})
+		if err != nil {
+			t.Fatalf("Failed to create node with DHT: %v", err)
+		}
+		defer node.Close()
+
+		// First verify test setup created the expected DHT type
+		if node.DHTClient == nil {
+			t.Fatalf("Test setup failed: DHTClient is nil")
+		}
+
+		if _, ok := node.DHTClient.(*ddht.DHT); !ok {
+			t.Fatalf("Test setup failed: expected DHTClient to be *ddht.DHT, got %T", node.DHTClient)
+		}
+
+		// Now verify HasActiveDHTClient() correctly identifies it as active
+		if !node.HasActiveDHTClient() {
+			t.Error("Expected true for valid dual DHT client")
+		}
+	})
+
+	// Test 6: Valid accelerated DHT client (Routing.Type=autoclient)
+	t.Run("valid accelerated DHT client", func(t *testing.T) {
+		ctx := context.Background()
+		mn := mocknet.New()
+		defer mn.Close()
+
+		ds := syncds.MutexWrap(datastore.NewMapDatastore())
+		c := config.Config{}
+		c.Identity = testIdentity
+		c.Addresses.Swarm = []string{"/ip4/0.0.0.0/tcp/4001"}
+		c.Routing.AcceleratedDHTClient = config.True
+
+		r := &repo.Mock{
+			C: c,
+			D: ds,
+			K: keystore.NewMemKeystore(),
+			F: filestore.NewFileManager(ds, filepath.Dir(os.TempDir())),
+		}
+
+		node, err := NewNode(ctx, &BuildCfg{
+			Routing: libp2p.DHTOption,
+			Repo:    r,
+			Host:    mockHostOption(mn),
+			Online:  true,
+		})
+		if err != nil {
+			t.Fatalf("Failed to create node with accelerated DHT: %v", err)
+		}
+		defer node.Close()
+
+		// First verify test setup created the expected accelerated DHT type
+		if node.DHTClient == nil {
+			t.Fatalf("Test setup failed: DHTClient is nil")
+		}
+
+		if _, ok := node.DHTClient.(*fullrt.FullRT); !ok {
+			t.Fatalf("Test setup failed: expected DHTClient to be *fullrt.FullRT, got %T", node.DHTClient)
+		}
+
+		// Now verify HasActiveDHTClient() correctly identifies it as active
+		if !node.HasActiveDHTClient() {
+			t.Error("Expected true for valid accelerated DHT client")
+		}
+	})
+}