feat(routing/http/server): improve ux of /ipns

CHANGELOG.md

@@ -17,6 +17,7 @@ The following emojis are used to highlight certain changes:
 ### Added
 
 * `routing/http/server` now adds `Cache-Control` HTTP header to GET requests: 15 seconds for empty responses, or 5 minutes for responses with providers.
+* `routing/http/server` the `/ipns` endpoint is more friendly to users opening URL in web browsers: returns `Content-Disposition` header and defaults to `application/vnd.ipfs.ipns-record` response when `Accept` is missing.
 
 ### Changed
 

routing/http/server/server.go

@@ -22,6 +22,7 @@
 	"github.com/ipfs/go-cid"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/multiformats/go-multiaddr"
+	"github.com/multiformats/go-multibase"
 
 	logging "github.com/ipfs/go-log/v2"
 )
@@ -370,8 +371,14 @@
 }
 
 func (s *server) GetIPNS(w http.ResponseWriter, r *http.Request) {
-	if !strings.Contains(r.Header.Get("Accept"), mediaTypeIPNSRecord) {
-		writeErr(w, "GetIPNS", http.StatusNotAcceptable, errors.New("content type in 'Accept' header is missing or not supported"))
+	acceptHdrValue := r.Header.Get("Accept")
+	// When 'Accept' header is missing, default to 'application/vnd.ipfs.ipns-record'
+	// (improved UX, similar to how we default to JSON response for /providers and /peers)
+	if len(acceptHdrValue) == 0 || strings.Contains(acceptHdrValue, mediaTypeWildcard) {
+		acceptHdrValue = mediaTypeIPNSRecord
+	}
+	if !strings.Contains(acceptHdrValue, mediaTypeIPNSRecord) {
+		writeErr(w, "GetIPNS", http.StatusNotAcceptable, errors.New("content type in 'Accept' header is not supported, retry with 'application/vnd.ipfs.ipns-record'"))
 		return
 	}
 
@@ -420,6 +427,11 @@
 
 	w.Header().Set("Etag", fmt.Sprintf(`"%x"`, xxhash.Sum64(rawRecord)))
 	w.Header().Set("Content-Type", mediaTypeIPNSRecord)
+
+	// Content-Disposition is not required, but improves UX by assigning a meaningful filename when opening URL in a web browser
+	if filename, err := cid.StringOfBase(multibase.Base36); err == nil {
+		w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"%s.ipns-record\"", filename))
+	}
 	w.Header().Add("Vary", "Accept")
 	w.Write(rawRecord)
 }

routing/http/server/server_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http/httptest"
 	"regexp"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
@@ -128,7 +129,14 @@ func TestProviders(t *testing.T) {
 
 		req, err := http.NewRequest(http.MethodGet, urlStr, nil)
 		require.NoError(t, err)
-		req.Header.Set("Accept", contentType)
+
+		if contentType == "" || strings.Contains(contentType, mediaTypeWildcard) {
+			// When no Accept header is provided with request
+			// we default expected response to  JSON
+			contentType = mediaTypeJSON
+		} else {
+			req.Header.Set("Accept", contentType)
+		}
 
 		resp, err := http.DefaultClient.Do(req)
 		require.NoError(t, err)
@@ -158,6 +166,16 @@ func TestProviders(t *testing.T) {
 		runTest(t, mediaTypeJSON, true, false, `{"Providers":null}`)
 	})
 
+	t.Run("Wildcard Accept header defaults to JSON Response", func(t *testing.T) {
+		accept := "text/html,*/*"
+		runTest(t, accept, true, false, `{"Providers":null}`)
+	})
+
+	t.Run("Missing Accept header defaults to JSON Response", func(t *testing.T) {
+		accept := ""
+		runTest(t, accept, true, false, `{"Providers":null}`)
+	})
+
 	t.Run("NDJSON Response", func(t *testing.T) {
 		runTest(t, mediaTypeNDJSON, false, true, `{"Addrs":[],"ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vn","Protocols":["transport-bitswap"],"Schema":"peer"}`+"\n"+`{"Schema":"bitswap","Protocol":"transport-bitswap","ID":"12D3KooWM8sovaEGU1bmiWGWAzvs47DEcXKZZTuJnpQyVTkRs2Vz"}`+"\n")
 	})
@@ -173,7 +191,9 @@ func TestPeers(t *testing.T) {
 		t.Cleanup(server.Close)
 		req, err := http.NewRequest(http.MethodGet, "http://"+server.Listener.Addr().String()+"/routing/v1/peers/"+arg, nil)
 		require.NoError(t, err)
-		req.Header.Set("Accept", contentType)
+		if contentType != "" {
+			req.Header.Set("Accept", contentType)
+		}
 		resp, err := http.DefaultClient.Do(req)
 		require.NoError(t, err)
 		return resp
@@ -187,7 +207,7 @@ func TestPeers(t *testing.T) {
 		require.Equal(t, 400, resp.StatusCode)
 	})
 
-	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (No Results, JSON)", func(t *testing.T) {
+	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (No Results, explicit JSON)", func(t *testing.T) {
 		t.Parallel()
 
 		_, pid := makePeerID(t)
@@ -206,6 +226,42 @@ func TestPeers(t *testing.T) {
 		requireCloseToNow(t, resp.Header.Get("Last-Modified"))
 	})
 
+	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (No Results, implicit JSON, wildcard Accept header)", func(t *testing.T) {
+		t.Parallel()
+
+		_, pid := makePeerID(t)
+		results := iter.FromSlice([]iter.Result[*types.PeerRecord]{})
+
+		router := &mockContentRouter{}
+		router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
+
+		// Simulate request with Accept header that includes wildcard match
+		resp := makeRequest(t, router, "text/html,*/*", peer.ToCid(pid).String())
+
+		// Expect response to default to application/json
+		require.Equal(t, 200, resp.StatusCode)
+		require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
+
+	})
+
+	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (No Results, implicit JSON, no Accept header)", func(t *testing.T) {
+		t.Parallel()
+
+		_, pid := makePeerID(t)
+		results := iter.FromSlice([]iter.Result[*types.PeerRecord]{})
+
+		router := &mockContentRouter{}
+		router.On("FindPeers", mock.Anything, pid, 20).Return(results, nil)
+
+		// Simulate request without Accept header
+		resp := makeRequest(t, router, "", peer.ToCid(pid).String())
+
+		// Expect response to default to application/json
+		require.Equal(t, 200, resp.StatusCode)
+		require.Equal(t, mediaTypeJSON, resp.Header.Get("Content-Type"))
+
+	})
+
 	t.Run("GET /routing/v1/peers/{cid-libp2p-key-peer-id} returns 200 with correct body and headers (JSON)", func(t *testing.T) {
 		t.Parallel()
 
@@ -398,14 +454,16 @@ func TestIPNS(t *testing.T) {
 	cid1, err := cid.Decode("bafkreifjjcie6lypi6ny7amxnfftagclbuxndqonfipmb64f2km2devei4")
 	require.NoError(t, err)
 
-	makeRequest := func(t *testing.T, router *mockContentRouter, path string) *http.Response {
+	makeRequest := func(t *testing.T, router *mockContentRouter, path string, accept string) *http.Response {
 		server := httptest.NewServer(Handler(router))
 		t.Cleanup(server.Close)
 		serverAddr := "http://" + server.Listener.Addr().String()
 		urlStr := serverAddr + path
 		req, err := http.NewRequest(http.MethodGet, urlStr, nil)
 		require.NoError(t, err)
-		req.Header.Set("Accept", mediaTypeIPNSRecord)
+		if accept != "" {
+			req.Header.Set("Accept", accept)
+		}
 		resp, err := http.DefaultClient.Do(req)
 		require.NoError(t, err)
 		return resp
@@ -428,7 +486,7 @@ func TestIPNS(t *testing.T) {
 
 		_, name2 := makeName(t)
 
-		t.Run("GET /routing/v1/ipns/{cid-peer-id} returns 200", func(t *testing.T) {
+		t.Run("GET /routing/v1/ipns/{cid-peer-id} returns 200 (explicit Accept header)", func(t *testing.T) {
 			t.Parallel()
 
 			rec, err := ipns.UnmarshalRecord(rawRecord1)
@@ -437,14 +495,16 @@ func TestIPNS(t *testing.T) {
 			router := &mockContentRouter{}
 			router.On("GetIPNS", mock.Anything, name1).Return(rec, nil)
 
-			resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.String())
+			resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.String(), mediaTypeIPNSRecord)
 			require.Equal(t, 200, resp.StatusCode)
 			require.Equal(t, mediaTypeIPNSRecord, resp.Header.Get("Content-Type"))
 			require.Equal(t, "Accept", resp.Header.Get("Vary"))
 			require.NotEmpty(t, resp.Header.Get("Etag"))
 
 			requireCloseToNow(t, resp.Header.Get("Last-Modified"))
 
+			require.Contains(t, resp.Header.Get("Content-Disposition"), `attachment; filename="`+name1.String()+`.ipns-record"`)
+
 			require.Contains(t, resp.Header.Get("Cache-Control"), "public, max-age=42")
 
 			// expected "stale" values are int(time.Until(eol).Seconds())
@@ -465,19 +525,65 @@ func TestIPNS(t *testing.T) {
 			require.Equal(t, body, rawRecord1)
 		})
 
+		t.Run("GET /routing/v1/ipns/{cid-peer-id} returns 200 (Accept header missing)", func(t *testing.T) {
+			t.Parallel()
+
+			rec, err := ipns.UnmarshalRecord(rawRecord1)
+			require.NoError(t, err)
+
+			router := &mockContentRouter{}
+			router.On("GetIPNS", mock.Anything, name1).Return(rec, nil)
+
+			// Simulate request without explicit Accept header
+			noAccept := ""
+			resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.String(), noAccept)
+
+			// Expect application/vnd.ipfs.ipns-record in response
+			require.Equal(t, 200, resp.StatusCode)
+			require.Equal(t, mediaTypeIPNSRecord, resp.Header.Get("Content-Type"))
+
+			// Confirm body matches  expected bytes
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.Equal(t, body, rawRecord1)
+		})
+
+		t.Run("GET /routing/v1/ipns/{cid-peer-id} returns 200 (Accept header with wildcard)", func(t *testing.T) {
+			t.Parallel()
+
+			rec, err := ipns.UnmarshalRecord(rawRecord1)
+			require.NoError(t, err)
+
+			router := &mockContentRouter{}
+			router.On("GetIPNS", mock.Anything, name1).Return(rec, nil)
+
+			// Simulate request with wildcard Accept header
+			wcAccept := "text/html,*/*"
+			resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.String(), wcAccept)
+
+			// Expect application/vnd.ipfs.ipns-record in response
+			require.Equal(t, 200, resp.StatusCode)
+			require.Equal(t, mediaTypeIPNSRecord, resp.Header.Get("Content-Type"))
+
+			// Confirm body matches  expected bytes
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.Equal(t, body, rawRecord1)
+		})
+
 		t.Run("GET /routing/v1/ipns/{non-peer-cid} returns 400", func(t *testing.T) {
 			t.Parallel()
 
 			router := &mockContentRouter{}
-			resp := makeRequest(t, router, "/routing/v1/ipns/"+cid1.String())
+			resp := makeRequest(t, router, "/routing/v1/ipns/"+cid1.String(), mediaTypeIPNSRecord)
 			require.Equal(t, 400, resp.StatusCode)
 		})
 
 		t.Run("GET /routing/v1/ipns/{peer-id} returns 400", func(t *testing.T) {
 			t.Parallel()
 
 			router := &mockContentRouter{}
-			resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.Peer().String())
+			resp := makeRequest(t, router, "/routing/v1/ipns/"+name1.Peer().String(), mediaTypeIPNSRecord)
 			require.Equal(t, 400, resp.StatusCode)
 		})