diff --git a/server.go b/server.go index 330cc18..518974b 100644 --- a/server.go +++ b/server.go @@ -194,9 +194,18 @@ func start(ctx context.Context, cfg *config) error { } // Combine HTTP routers with DHT and additional routers - crRouters := combineRouters(dhtRouting, cachedAddrBook, providerHTTPRouters, blockProviderRouters) - prRouters := combineRouters(dhtRouting, cachedAddrBook, peerHTTPRouters, nil) - ipnsRouters := combineRouters(dhtRouting, cachedAddrBook, ipnsHTTPRouters, nil) + crRouters := combineRouters(h, dhtRouting, cachedAddrBook, providerHTTPRouters, blockProviderRouters) + prRouters := combineRouters(h, dhtRouting, cachedAddrBook, peerHTTPRouters, nil) + ipnsRouters := combineRouters(h, dhtRouting, cachedAddrBook, ipnsHTTPRouters, nil) + + // Create DHT router for GetClosestPeers endpoint + var dhtRouters router + if cachedAddrBook != nil && dhtRouting != nil { + cachedRouter := NewCachedRouter(libp2pRouter{host: h, routing: dhtRouting}, cachedAddrBook) + dhtRouters = sanitizeRouter{cachedRouter} + } else if dhtRouting != nil { + dhtRouters = sanitizeRouter{libp2pRouter{host: h, routing: dhtRouting}} + } _, port, err := net.SplitHostPort(cfg.listenAddress) if err != nil { @@ -220,6 +229,7 @@ func start(ctx context.Context, cfg *config) error { providers: crRouters, peers: prRouters, ipns: ipnsRouters, + dht: dhtRouters, }, handlerOpts...) // Add CORS. @@ -325,14 +335,14 @@ func newHost(cfg *config) (host.Host, error) { // combineRouters combines delegated HTTP routers with DHT and additional routers. // It no longer creates HTTP clients (that's done in createDelegatedHTTPRouters). -func combineRouters(dht routing.Routing, cachedAddrBook *cachedAddrBook, delegatedRouters, additionalRouters []router) router { +func combineRouters(h host.Host, dht routing.Routing, cachedAddrBook *cachedAddrBook, delegatedRouters, additionalRouters []router) router { var dhtRouter router if cachedAddrBook != nil { - cachedRouter := NewCachedRouter(libp2pRouter{routing: dht}, cachedAddrBook) + cachedRouter := NewCachedRouter(libp2pRouter{host: h, routing: dht}, cachedAddrBook) dhtRouter = sanitizeRouter{cachedRouter} } else if dht != nil { - dhtRouter = sanitizeRouter{libp2pRouter{routing: dht}} + dhtRouter = sanitizeRouter{libp2pRouter{host: h, routing: dht}} } if len(delegatedRouters) == 0 && len(additionalRouters) == 0 { diff --git a/server_dht_test.go b/server_dht_test.go index 3d20452..12b8981 100644 --- a/server_dht_test.go +++ b/server_dht_test.go @@ -351,6 +351,32 @@ func TestGetClosestPeersEndpoint(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode) require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type")) }) + + t.Run("GET /routing/v1/dht/closest/peers/{cid} returns 500 when DHT is disabled", func(t *testing.T) { + t.Parallel() + + _, pid := makeEd25519PeerID(t) + key := peer.ToCid(pid) + + // Pass nil router to simulate DHT disabled via --dht=disabled + handler := server.Handler(&composableRouter{dht: nil}) + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + + urlStr := fmt.Sprintf("http://%s/routing/v1/dht/closest/peers/%s", srv.Listener.Addr().String(), key.String()) + req, err := http.NewRequest(http.MethodGet, urlStr, nil) + require.NoError(t, err) + req.Header.Set("Accept", mediaTypeJSON) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + + // Returns 500 Internal Server Error instead of misleading 200 with empty results + require.Equal(t, http.StatusInternalServerError, resp.StatusCode) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Contains(t, string(body), "not supported") + }) } // mockDHTRouter implements the router interface for testing diff --git a/server_routers.go b/server_routers.go index c919af5..ce7806f 100644 --- a/server_routers.go +++ b/server_routers.go @@ -71,7 +71,9 @@ func (r composableRouter) FindPeers(ctx context.Context, pid peer.ID, limit int) func (r composableRouter) GetClosestPeers(ctx context.Context, key cid.Cid) (iter.ResultIter[*types.PeerRecord], error) { if r.dht == nil { - return iter.ToResultIter(iter.FromSlice([]*types.PeerRecord{})), nil + // Return ErrNotSupported when no DHT is available (e.g., disabled via --dht=disabled CLI param). + // This returns HTTP 501 Not Implemented instead of misleading HTTP 200 with empty results. + return nil, routing.ErrNotSupported } return r.dht.GetClosestPeers(ctx, key) } diff --git a/server_test.go b/server_test.go index e4ffbb9..ee5a986 100644 --- a/server_test.go +++ b/server_test.go @@ -13,14 +13,14 @@ func TestCombineRouters(t *testing.T) { mockRouter := composableRouter{} // Check that combineRouters with DHT only returns sanitizeRouter - v := combineRouters(&bundledDHT{}, nil, nil, nil) + v := combineRouters(nil, &bundledDHT{}, nil, nil, nil) require.IsType(t, sanitizeRouter{}, v) // Check that combineRouters with delegated routers only returns parallelRouter - v = combineRouters(nil, nil, []router{mockRouter}, nil) + v = combineRouters(nil, nil, nil, []router{mockRouter}, nil) require.IsType(t, parallelRouter{}, v) // Check that combineRouters with both DHT and delegated routers returns parallelRouter - v = combineRouters(&bundledDHT{}, nil, []router{mockRouter}, nil) + v = combineRouters(nil, &bundledDHT{}, nil, []router{mockRouter}, nil) require.IsType(t, parallelRouter{}, v) }