Skip to content

Commit b35990e

Browse files
lidellidel
committed
feat: implement IPIP-513 for routing v1 endpoints
return HTTP 200 with empty results instead of 404 when no records found, while maintaining backward compatibility in clients to handle both response codes Fixes: #1024 Spec: ipfs/specs#513
1 parent 03aa83e commit b35990e

File tree

4 files changed

+269
-44
lines changed

4 files changed

+269
-44
lines changed

‎routing/http/client/client.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
logging "github.com/ipfs/go-log/v2"
2828
"github.com/libp2p/go-libp2p/core/crypto"
2929
"github.com/libp2p/go-libp2p/core/peer"
30+
"github.com/libp2p/go-libp2p/core/routing"
3031
"github.com/multiformats/go-multiaddr"
3132
)
3233

@@ -279,6 +280,8 @@ func (c *Client) FindProviders(ctx context.Context, key cid.Cid) (providers iter
279280
}
280281

281282
m.statusCode = resp.StatusCode
283+
// Per IPIP-0513: Handle 404 as empty results for backward compatibility
284+
// New servers return 200 with empty results, old servers return 404
282285
if resp.StatusCode == http.StatusNotFound {
283286
resp.Body.Close()
284287
m.record(ctx)
@@ -462,6 +465,8 @@ func (c *Client) FindPeers(ctx context.Context, pid peer.ID) (peers iter.ResultI
462465
}
463466

464467
m.statusCode = resp.StatusCode
468+
// Per IPIP-0513: Handle 404 as empty results for backward compatibility
469+
// New servers return 200 with empty results, old servers return 404
465470
if resp.StatusCode == http.StatusNotFound {
466471
resp.Body.Close()
467472
m.record(ctx)
@@ -533,10 +538,25 @@ func (c *Client) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, err
533538
}
534539
defer resp.Body.Close()
535540

541+
// Per IPIP-0513: Handle 404 as "no record found" for backward compatibility
542+
// New servers return 200 with text/plain for no record, old servers return 404
543+
if resp.StatusCode == http.StatusNotFound {
544+
io.Copy(io.Discard, resp.Body) // Drain body for connection reuse
545+
return nil, routing.ErrNotFound
546+
}
547+
536548
if resp.StatusCode != http.StatusOK {
537549
return nil, httpError(resp.StatusCode, resp.Body)
538550
}
539551

552+
// Per IPIP-0513: Only Content-Type: application/vnd.ipfs.ipns-record indicates a valid record
553+
// Any other content type (e.g., text/plain) means no record found
554+
contentType := resp.Header.Get("Content-Type")
555+
if !strings.Contains(contentType, mediaTypeIPNSRecord) {
556+
io.Copy(io.Discard, resp.Body) // Drain body for connection reuse
557+
return nil, routing.ErrNotFound
558+
}
559+
540560
// Limit the reader to the maximum record size.
541561
rawRecord, err := io.ReadAll(io.LimitReader(resp.Body, int64(ipns.MaxRecordSize)))
542562
if err != nil {

‎routing/http/client/client_test.go‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/ipfs/go-cid"
2121
"github.com/libp2p/go-libp2p/core/crypto"
2222
"github.com/libp2p/go-libp2p/core/peer"
23+
"github.com/libp2p/go-libp2p/core/routing"
2324
"github.com/multiformats/go-multiaddr"
2425
"github.com/multiformats/go-multibase"
2526
"github.com/multiformats/go-multihash"
@@ -317,6 +318,19 @@ func TestClient_FindProviders(t *testing.T) {
317318
httpStatusCode: 404,
318319
expResult: nil,
319320
},
321+
{
322+
name: "returns no providers if the HTTP server returns 200 with empty JSON array",
323+
routerResult: []iter.Result[types.Record]{},
324+
expResult: nil,
325+
serverStreamingDisabled: true,
326+
expJSONResponse: true,
327+
},
328+
{
329+
name: "returns no providers if the HTTP server returns 200 with empty NDJSON",
330+
routerResult: []iter.Result[types.Record]{},
331+
expResult: nil,
332+
expStreamingResponse: true,
333+
},
320334
}
321335
for _, c := range cases {
322336
t.Run(c.name, func(t *testing.T) {
@@ -604,6 +618,19 @@ func TestClient_FindPeers(t *testing.T) {
604618
httpStatusCode: 404,
605619
expResult: nil,
606620
},
621+
{
622+
name: "returns no providers if the HTTP server returns 200 with empty JSON array",
623+
routerResult: []iter.Result[*types.PeerRecord]{},
624+
expResult: nil,
625+
serverStreamingDisabled: true,
626+
expJSONResponse: true,
627+
},
628+
{
629+
name: "returns no providers if the HTTP server returns 200 with empty NDJSON",
630+
routerResult: []iter.Result[*types.PeerRecord]{},
631+
expResult: nil,
632+
expStreamingResponse: true,
633+
},
607634
}
608635
for _, c := range cases {
609636
t.Run(c.name, func(t *testing.T) {
@@ -676,6 +703,139 @@ func TestClient_FindPeers(t *testing.T) {
676703
}
677704
}
678705

706+
func TestClient_EmptyResponses(t *testing.T) {
707+
// Test that client handles various empty response formats correctly
708+
cid := makeCID()
709+
pid := peer.ID("12D3KooWD3eckifWpRn9wQpMG9R9hX3sD158z7EqHWmweQAJU5SA")
710+
_, name := makeName(t)
711+
712+
// Helper to create test server with given response
713+
makeTestServer := func(handler http.HandlerFunc) *httptest.Server {
714+
return httptest.NewServer(handler)
715+
}
716+
717+
// Helper to verify empty results for providers
718+
verifyEmptyProviders := func(t *testing.T, serverURL string) {
719+
client, err := New(serverURL, WithHTTPClient((&http.Client{})))
720+
require.NoError(t, err)
721+
722+
resultsIter, err := client.FindProviders(context.Background(), cid)
723+
require.NoError(t, err)
724+
725+
results, err := iter.ReadAllResults(resultsIter)
726+
require.NoError(t, err)
727+
require.Empty(t, results)
728+
}
729+
730+
// Helper to verify empty results for peers
731+
verifyEmptyPeers := func(t *testing.T, serverURL string) {
732+
client, err := New(serverURL, WithHTTPClient((&http.Client{})))
733+
require.NoError(t, err)
734+
735+
resultsIter, err := client.FindPeers(context.Background(), pid)
736+
require.NoError(t, err)
737+
738+
results, err := iter.ReadAllResults(resultsIter)
739+
require.NoError(t, err)
740+
require.Empty(t, results)
741+
}
742+
743+
// Helper to verify IPNS not found
744+
verifyIPNSNotFound := func(t *testing.T, serverURL string) {
745+
client, err := New(serverURL, WithHTTPClient((&http.Client{})))
746+
require.NoError(t, err)
747+
748+
record, err := client.GetIPNS(context.Background(), name)
749+
require.ErrorIs(t, err, routing.ErrNotFound)
750+
require.Nil(t, record)
751+
}
752+
753+
testCases := []struct {
754+
name string
755+
handler http.HandlerFunc
756+
testFunc func(*testing.T, string)
757+
}{
758+
// FindProviders tests
759+
{
760+
name: "404 Not Found (legacy server)",
761+
handler: func(w http.ResponseWriter, r *http.Request) {
762+
w.WriteHeader(http.StatusNotFound)
763+
w.Write([]byte("404 page not found"))
764+
},
765+
testFunc: verifyEmptyProviders,
766+
},
767+
{
768+
name: "200 with null providers in JSON",
769+
handler: func(w http.ResponseWriter, r *http.Request) {
770+
w.Header().Set("Content-Type", "application/json")
771+
// Old server behavior - returns null instead of empty array
772+
w.Write([]byte(`{"Providers":null}`))
773+
},
774+
testFunc: verifyEmptyProviders,
775+
},
776+
{
777+
name: "200 with empty array in JSON",
778+
handler: func(w http.ResponseWriter, r *http.Request) {
779+
w.Header().Set("Content-Type", "application/json")
780+
// New server behavior - returns empty array
781+
w.Write([]byte(`{"Providers":[]}`))
782+
},
783+
testFunc: verifyEmptyProviders,
784+
},
785+
{
786+
name: "200 with empty NDJSON stream",
787+
handler: func(w http.ResponseWriter, r *http.Request) {
788+
w.Header().Set("Content-Type", "application/x-ndjson")
789+
// Empty body - no NDJSON lines
790+
},
791+
testFunc: verifyEmptyProviders,
792+
},
793+
// FindPeers tests
794+
{
795+
name: "FindPeers: 404 Not Found (legacy server)",
796+
handler: func(w http.ResponseWriter, r *http.Request) {
797+
w.WriteHeader(http.StatusNotFound)
798+
w.Write([]byte("404 page not found"))
799+
},
800+
testFunc: verifyEmptyPeers,
801+
},
802+
{
803+
name: "FindPeers: 200 with null peers in JSON",
804+
handler: func(w http.ResponseWriter, r *http.Request) {
805+
w.Header().Set("Content-Type", "application/json")
806+
// Old server behavior - returns null instead of empty array
807+
w.Write([]byte(`{"Peers":null}`))
808+
},
809+
testFunc: verifyEmptyPeers,
810+
},
811+
// IPNS tests
812+
{
813+
name: "IPNS: 404 Not Found (legacy server)",
814+
handler: func(w http.ResponseWriter, r *http.Request) {
815+
w.WriteHeader(http.StatusNotFound)
816+
w.Write([]byte("404 page not found"))
817+
},
818+
testFunc: verifyIPNSNotFound,
819+
},
820+
{
821+
name: "IPNS: 200 with text/plain (no record found)",
822+
handler: func(w http.ResponseWriter, r *http.Request) {
823+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
824+
w.Write([]byte("delegate error: routing: not found"))
825+
},
826+
testFunc: verifyIPNSNotFound,
827+
},
828+
}
829+
830+
for _, tc := range testCases {
831+
t.Run(tc.name, func(t *testing.T) {
832+
server := makeTestServer(tc.handler)
833+
defer server.Close()
834+
tc.testFunc(t, server.URL)
835+
})
836+
}
837+
}
838+
679839
func TestNormalizeBaseURL(t *testing.T) {
680840
cases := []struct {
681841
name string

‎routing/http/server/server.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,11 @@ func (s *server) findProvidersJSON(w http.ResponseWriter, provIter iter.ResultIt
295295
return
296296
}
297297

298+
// Ensure providers is an empty slice instead of nil for proper JSON marshaling
299+
if providers == nil {
300+
providers = []types.Record{}
301+
}
302+
298303
writeJSONResult(w, "FindProviders", jsontypes.ProvidersResponse{
299304
Providers: providers,
300305
})
@@ -451,6 +456,11 @@ func (s *server) findPeersJSON(w http.ResponseWriter, peersIter iter.ResultIter[
451456
return
452457
}
453458

459+
// Ensure peers is an empty slice instead of nil for proper JSON marshaling
460+
if peers == nil {
461+
peers = []*types.PeerRecord{}
462+
}
463+
454464
writeJSONResult(w, "FindPeers", jsontypes.PeersResponse{
455465
Peers: peers,
456466
})
@@ -503,7 +513,9 @@ func (s *server) GetIPNS(w http.ResponseWriter, r *http.Request) {
503513
record, err := s.svc.GetIPNS(ctx, name)
504514
if err != nil {
505515
if errors.Is(err, routing.ErrNotFound) {
506-
writeErr(w, "GetIPNS", http.StatusNotFound, fmt.Errorf("delegate error: %w", err))
516+
// Per IPIP-0513: Return 200 with text/plain to indicate no record found
517+
setCacheControl(w, maxAgeWithoutResults, maxStale)
518+
http.Error(w, fmt.Sprintf("delegate error: %s", err), http.StatusOK)
507519
return
508520
} else {
509521
writeErr(w, "GetIPNS", http.StatusInternalServerError, fmt.Errorf("delegate error: %w", err))
@@ -629,11 +641,8 @@ func writeJSONResult(w http.ResponseWriter, method string, val interface{ Length
629641
return
630642
}
631643

632-
if val.Length() > 0 {
633-
w.WriteHeader(http.StatusOK)
634-
} else {
635-
w.WriteHeader(http.StatusNotFound)
636-
}
644+
// Always return 200 OK per IPIP-0513
645+
w.WriteHeader(http.StatusOK)
637646

638647
_, err = io.Copy(w, bytes.NewBuffer(b))
639648
if err != nil {
@@ -708,8 +717,8 @@ func writeResultsIterNDJSON[T types.Record](w http.ResponseWriter, resultIter it
708717
}
709718

710719
if !hasResults {
711-
// There weren't results, cache for shorter and send 404
720+
// There weren't results, cache for shorter but still send 200 per IPIP-0513
712721
setCacheControl(w, maxAgeWithoutResults, maxStale)
713-
w.WriteHeader(http.StatusNotFound)
722+
w.WriteHeader(http.StatusOK)
714723
}
715724
}

0 commit comments

Comments
 (0)