diff --git a/bitswap/network/httpnet/httpnet.go b/bitswap/network/httpnet/httpnet.go index 939e4e4a5..cc7a53607 100644 --- a/bitswap/network/httpnet/httpnet.go +++ b/bitswap/network/httpnet/httpnet.go @@ -26,12 +26,13 @@ import ( "github.com/libp2p/go-libp2p/core/peerstore" "github.com/libp2p/go-libp2p/p2p/protocol/ping" "github.com/multiformats/go-multiaddr" + "go.uber.org/multierr" ) var log = logging.Logger("httpnet") var ErrNoHTTPAddresses = errors.New("AddrInfo does not contain any valid HTTP addresses") -var ErrNoSuccess = errors.New("none of the peer HTTP endpoints responded successfully to request") +var ErrNoSuccess = errors.New("connection failed to all HTTP endpoints") var ErrNotConnected = errors.New("no HTTP connection has been setup to this peer") var _ network.BitSwapNetwork = (*Network)(nil) @@ -411,6 +412,8 @@ func (ht *Network) Connect(ctx context.Context, pi peer.AddrInfo) error { urls := network.ExtractURLsFromPeer(pi) + errs := []error{ErrNoSuccess} + // Filter addresses based on allow and denylists var filteredURLs []network.ParsedURL for _, u := range urls { @@ -426,11 +429,13 @@ func (ht *Network) Connect(ctx context.Context, pi peer.AddrInfo) error { _, denied := ht.denylist[host] if allowed && !denied { filteredURLs = append(filteredURLs, u) + } else { + errs = append(errs, fmt.Errorf("%s: address not allowed per allow/denylist", u.Multiaddress.String())) } } urls = filteredURLs if len(urls) == 0 { - return ErrNoHTTPAddresses + return multierr.Combine(errs...) } if len(urls) > ht.maxHTTPAddressesPerPeer { urls = urls[0:ht.maxHTTPAddressesPerPeer] @@ -446,9 +451,10 @@ func (ht *Network) Connect(ctx context.Context, pi peer.AddrInfo) error { // If head works we assume GET works too. err := ht.connectToURL(ctx, pi.ID, u, "HEAD") if err != nil { + errs = append(errs, fmt.Errorf("%s: %s", u.Multiaddress.String(), err)) // abort if context cancelled if ctxErr := ctx.Err(); ctxErr != nil { - return ctxErr + return multierr.Combine(errs...) } } else { workingAddrs = append(workingAddrs, u.Multiaddress) @@ -460,9 +466,9 @@ func (ht *Network) Connect(ctx context.Context, pi peer.AddrInfo) error { err = ht.connectToURL(ctx, pi.ID, u, "GET") if err != nil { - // abort if context cancelled + errs = append(errs, fmt.Errorf("%s: %s", u.Multiaddress.String(), err)) if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return err + return multierr.Combine(errs...) } continue } @@ -471,7 +477,7 @@ func (ht *Network) Connect(ctx context.Context, pi peer.AddrInfo) error { // Bail out if no working urls found. if len(workingAddrs) == 0 { - return fmt.Errorf("connect failure to %s: %w", p, ErrNoSuccess) + return multierr.Combine(errs...) } // We have some working urls! diff --git a/bitswap/network/httpnet/httpnet_test.go b/bitswap/network/httpnet/httpnet_test.go index 8ed067814..e0a524e35 100644 --- a/bitswap/network/httpnet/httpnet_test.go +++ b/bitswap/network/httpnet/httpnet_test.go @@ -131,7 +131,9 @@ func mockNetwork(t *testing.T, recv network.Receiver, opts ...Option) (*Network, if err != nil { t.Fatal(err) } - opts = append(opts, WithInsecureSkipVerify(true)) + + // allow ovewrite of default options by prepending them + opts = append([]Option{WithInsecureSkipVerify(true)}, opts...) htnet := New(h, opts...) htnet.Start(recv) return htnet.(*Network), mn @@ -282,20 +284,25 @@ func srvMultiaddr(t *testing.T, srv *httptest.Server) multiaddr.Multiaddr { return maddr.Encapsulate(httpma) } -func connectToPeer(t *testing.T, ctx context.Context, htnet *Network, remote host.Host, srvs ...*httptest.Server) { +func connectToPeer(t *testing.T, ctx context.Context, htnet *Network, remote host.Host, srvs ...*httptest.Server) error { var addrs []multiaddr.Multiaddr for _, srv := range srvs { addrs = append(addrs, srvMultiaddr(t, srv)) } - err := htnet.Connect( + return htnet.Connect( ctx, peer.AddrInfo{ ID: remote.ID(), Addrs: addrs, }, ) - if err != nil { +} + +func mustConnectToPeer(t *testing.T, ctx context.Context, htnet *Network, remote host.Host, srvs ...*httptest.Server) { + t.Helper() + + if err := connectToPeer(t, ctx, htnet, remote, srvs...); err != nil { t.Fatal(err) } } @@ -308,7 +315,7 @@ func TestBestURL(t *testing.T) { t.Fatal(err) } msrv := makeServer(t, 0, 0) - connectToPeer(t, ctx, htnet, peer, msrv) + mustConnectToPeer(t, ctx, htnet, peer, msrv) nms, err := htnet.NewMessageSender( ctx, @@ -393,6 +400,48 @@ func TestBestURL(t *testing.T) { } +func TestConnectErrors(t *testing.T) { + ctx := context.Background() + recv := mockReceiver(t) + msrv := makeServer(t, 0, 0) + + htnet, mn := mockNetwork(t, recv, + WithInsecureSkipVerify(false), + ) + peer, err := mn.GenPeer() + if err != nil { + t.Fatal(err) + } + + err = connectToPeer(t, ctx, htnet, peer, msrv) + if err == nil { + t.Fatal("expected error") + } + t.Log(err) + if !strings.Contains(err.Error(), "failed to verify") { + t.Error("wrong error") + } + + htnet2, mn2 := mockNetwork(t, recv, + WithDenylist([]string{"127.0.0.1"}), + ) + + peer2, err := mn2.GenPeer() + if err != nil { + t.Fatal(err) + } + + err = connectToPeer(t, ctx, htnet2, peer2, msrv) + if err == nil { + t.Fatal("expected error") + } + t.Log(err) + if !strings.Contains(err.Error(), "denylist") { + t.Error("wrong error") + } + +} + func TestSendMessage(t *testing.T) { ctx := context.Background() recv := mockReceiver(t) @@ -402,7 +451,7 @@ func TestSendMessage(t *testing.T) { t.Fatal(err) } msrv := makeServer(t, 0, 10) - connectToPeer(t, ctx, htnet, peer, msrv) + mustConnectToPeer(t, ctx, htnet, peer, msrv) wl := makeCids(t, 0, 10) msg := makeWantsMessage(wl) @@ -431,7 +480,7 @@ func TestSendMessageWithFailingServer(t *testing.T) { } msrv := makeServer(t, 0, 0) msrv2 := makeServer(t, 0, 10) - connectToPeer(t, ctx, htnet, peer, msrv, msrv2) + mustConnectToPeer(t, ctx, htnet, peer, msrv, msrv2) wl := makeCids(t, 0, 10) msg := makeWantsMessage(wl) @@ -462,7 +511,7 @@ func TestSendMessageWithPartialResponse(t *testing.T) { t.Fatal(err) } msrv := makeServer(t, 5, 10) - connectToPeer(t, ctx, htnet, peer, msrv) + mustConnectToPeer(t, ctx, htnet, peer, msrv) wl := makeCids(t, 0, 10) msg := makeWantsMessage(wl) @@ -497,7 +546,7 @@ func TestSendMessageSendHavesAndDontHaves(t *testing.T) { t.Fatal(err) } msrv := makeServer(t, 0, 5) - connectToPeer(t, ctx, htnet, peer, msrv) + mustConnectToPeer(t, ctx, htnet, peer, msrv) wl := makeCids(t, 0, 10) msg := makeHavesMessage(wl) @@ -545,8 +594,8 @@ func TestBackOff(t *testing.T) { } msrv := makeServer(t, 0, 1) - connectToPeer(t, ctx, htnet, peer, msrv) - connectToPeer(t, ctx, htnet, peer2, msrv) + mustConnectToPeer(t, ctx, htnet, peer, msrv) + mustConnectToPeer(t, ctx, htnet, peer2, msrv) nms, err := htnet.NewMessageSender(ctx, peer.ID(), nil) if err != nil { @@ -598,7 +647,7 @@ func TestErrorTracking(t *testing.T) { } msrv := makeServer(t, 0, 0) - connectToPeer(t, ctx, htnet, peer, msrv) + mustConnectToPeer(t, ctx, htnet, peer, msrv) err = recv.waitConnected(1) if err != nil {