p2p: support block request redirects for p2p addresses

rpcs/blockService.go

@@ -378,7 +378,6 @@ func (bs *BlockService) handleCatchupReq(ctx context.Context, reqMsg network.Inc
 		return
 	}
 	respTopics, n = topicBlockBytes(bs.log, bs.ledger, basics.Round(round), string(requestType))
-	return
 }
 
 // redirectRequest redirects the request to the next round robin fallback endpoint if available
@@ -389,18 +388,24 @@ func (bs *BlockService) redirectRequest(round uint64, response http.ResponseWrit
 		return false
 	}
 
-	parsedURL, err := network.ParseHostOrURL(peerAddress)
-	if err != nil {
-		bs.log.Debugf("redirectRequest: %s", err.Error())
-		return false
+	var redirectURL string
+	if network.IsMultiaddr(peerAddress) {
+		redirectURL = strings.Replace(FormatBlockQuery(round, "", bs.net), "{genesisID}", bs.genesisID, 1)
+	} else {
+		parsedURL, err := network.ParseHostOrURL(peerAddress)
+		if err != nil {
+			bs.log.Debugf("redirectRequest: %s", err.Error())
+			return false
+		}
+		parsedURL.Path = strings.Replace(FormatBlockQuery(round, parsedURL.Path, bs.net), "{genesisID}", bs.genesisID, 1)
+		redirectURL = parsedURL.String()
 	}
-	parsedURL.Path = strings.Replace(FormatBlockQuery(round, parsedURL.Path, bs.net), "{genesisID}", bs.genesisID, 1)
-	http.Redirect(response, request, parsedURL.String(), http.StatusTemporaryRedirect)
-	bs.log.Debugf("redirectRequest: redirected block request to %s", parsedURL.String())
+	http.Redirect(response, request, redirectURL, http.StatusTemporaryRedirect)
+	bs.log.Debugf("redirectRequest: redirected block request to %s", redirectURL)
 	return true
 }
 
-// getNextCustomFallbackEndpoint returns the next custorm fallback endpoint in RR ordering
+// getNextCustomFallbackEndpoint returns the next custom fallback endpoint in RR ordering
 func (bs *BlockService) getNextCustomFallbackEndpoint() (endpointAddress string) {
 	if len(bs.fallbackEndpoints.endpoints) == 0 {
 		return
@@ -493,12 +498,16 @@ func makeFallbackEndpoints(log logging.Logger, customFallbackEndpoints string) (
 	}
 	endpoints := strings.Split(customFallbackEndpoints, ",")
 	for _, ep := range endpoints {
-		parsed, err := network.ParseHostOrURL(ep)
-		if err != nil {
-			log.Warnf("makeFallbackEndpoints: error parsing %s %s", ep, err.Error())
-			continue
+		if network.IsMultiaddr(ep) {
+			fe.endpoints = append(fe.endpoints, ep)
+		} else {
+			parsed, err := network.ParseHostOrURL(ep)
+			if err != nil {
+				log.Warnf("makeFallbackEndpoints: error parsing %s %s", ep, err.Error())
+				continue
+			}
+			fe.endpoints = append(fe.endpoints, parsed.String())
 		}
-		fe.endpoints = append(fe.endpoints, parsed.String())
 	}
 	return
 }

rpcs/blockService_test.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"sync"
 	"testing"
@@ -500,6 +501,7 @@ func TestRedirectExceptions(t *testing.T) {
 	parsedURLNodeB.Path = FormatBlockQuery(uint64(4), parsedURLNodeB.Path, net2)
 	blockURLNodeB := parsedURLNodeB.String()
 	requestNodeB, err := http.NewRequest("GET", blockURLNodeB, nil)
+	require.NoError(t, err)
 	_, err = client.Do(requestNodeB)
 
 	require.Error(t, err)
@@ -558,8 +560,45 @@ func addBlock(t *testing.T, ledger *data.Ledger) (timestamp int64) {
 
 func TestErrMemoryAtCapacity(t *testing.T) {
 	partitiontest.PartitionTest(t)
+	t.Parallel()
 
 	macError := errMemoryAtCapacity{capacity: uint64(100), used: uint64(110)}
 	errStr := macError.Error()
 	require.Equal(t, "block service memory over capacity: 110 / 100", errStr)
 }
+
+func TestBlockServiceRedirect(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	log := logging.TestingLog(t)
+
+	ep1 := "http://localhost:1234"
+	ep2 := "/ip4/127.0.0.1/tcp/2345/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN"
+	endpoints := strings.Join([]string{ep1, ep2}, ",")
+	fb := makeFallbackEndpoints(log, endpoints)
+	require.Len(t, fb.endpoints, 2)
+	require.Equal(t, ep1, fb.endpoints[0])
+	require.Equal(t, ep2, fb.endpoints[1])
+
+	bs := BlockService{
+		net:               &httpTestPeerSource{},
+		fallbackEndpoints: fb,
+		log:               log,
+	}
+
+	r := httptest.NewRequest("GET", "/", strings.NewReader(""))
+	w := httptest.NewRecorder()
+	ok := bs.redirectRequest(10, w, r)
+	require.True(t, ok)
+	expectedPath := ep1 + FormatBlockQuery(10, "/", bs.net)
+	require.Equal(t, expectedPath, w.Result().Header.Get("Location"))
+
+	r = httptest.NewRequest("GET", "/", strings.NewReader(""))
+	w = httptest.NewRecorder()
+	ok = bs.redirectRequest(11, w, r)
+	require.True(t, ok)
+	// for p2p nodes the url is actually a peer address in p2p network and not part of HTTP path
+	expectedPath = FormatBlockQuery(11, "", bs.net)
+	require.Equal(t, expectedPath, w.Result().Header.Get("Location"))
+}