Skip to content

Commit

Permalink
bitswap/httpnet: probe for HEAD support
Browse files Browse the repository at this point in the history
  • Loading branch information
hsanjuan committed Feb 11, 2025
1 parent d3055a8 commit 612f23a
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 41 deletions.
75 changes: 43 additions & 32 deletions bitswap/network/httpnet/httpnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ var (
DefaultIdleConnTimeout = 30 * time.Second
DefaultResponseHeaderTimeout = 10 * time.Second
DefaultMaxIdleConns = 50
DefaultSupportsHave = true
DefaultInsecureSkipVerify = false
DefaultMaxBackoff = time.Minute
DefaultMaxHTTPAddressesPerPeer = 10
Expand All @@ -54,6 +53,8 @@ var pingCid = "bafkqaaa" // identity CID

const http2proto = "HTTP/2.0"

const peerstoreSupportsHeadKey = "http-retrieval-head-support"

// Option allows to configure the Network.
type Option func(net *Network)

Expand Down Expand Up @@ -103,15 +104,6 @@ func WithMaxIdleConns(n int) Option {
}

Check warning on line 104 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L101-L104

Added lines #L101 - L104 were not covered by tests
}

// WithSupportsHave specifies whether want to expose that we can handle Have
// messages (i.e. to the MessageQueue). Have messages trigger HEAD HTTP
// requests. Not all HTTP-endpoints may know how to handle a HEAD request.
func WithSupportsHave(b bool) Option {
return func(net *Network) {
net.supportsHave = b
}
}

// WithInsecureSkipVerify allows making HTTPS connections to test servers.
// Use for testing.
func WithInsecureSkipVerify(b bool) Option {
Expand Down Expand Up @@ -169,7 +161,6 @@ type Network struct {
idleConnTimeout time.Duration
responseHeaderTimeout time.Duration
maxIdleConns int
supportsHave bool
insecureSkipVerify bool
maxHTTPAddressesPerPeer int
httpWorkers int
Expand Down Expand Up @@ -203,7 +194,6 @@ func New(host host.Host, opts ...Option) network.BitSwapNetwork {
idleConnTimeout: DefaultIdleConnTimeout,
responseHeaderTimeout: DefaultResponseHeaderTimeout,
maxIdleConns: DefaultMaxIdleConns,
supportsHave: DefaultSupportsHave,
insecureSkipVerify: DefaultInsecureSkipVerify,
maxHTTPAddressesPerPeer: DefaultMaxHTTPAddressesPerPeer,
httpWorkers: DefaultHTTPWorkers,
Expand Down Expand Up @@ -408,42 +398,37 @@ func (ht *Network) Connect(ctx context.Context, p peer.AddrInfo) error {
// time with the client. We call peer.Connected()
// on success.
var workingAddrs []multiaddr.Multiaddr
supportsHead := true
for i, u := range urls {
req, err := buildRequest(ctx, u, "GET", pingCid, ht.userAgent)
if err != nil {
log.Debug(err)
return err
}

log.Debugf("connect request to %s %q", p.ID, req.URL)
resp, err := ht.client.Do(req)
err := ht.connect(ctx, p.ID, u, "GET")
if err != nil {
log.Debugf("connect error %s", err)
// abort if context cancelled
if ctxErr := ctx.Err(); ctxErr != nil {
// abort when context cancelled
return ctxErr
}
continue

Check warning on line 409 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L400-L409

Added lines #L400 - L409 were not covered by tests
}

if resp.Proto != http2proto {
log.Warnf("%s://%q is not using HTTP/2 (%s)", req.URL.Scheme, req.URL.Host, resp.Proto)
// GET works. Does HEAD work though?
err = ht.connect(ctx, p.ID, u, "HEAD")
if err != nil {
// abort if context cancelled
if ctxErr := ctx.Err(); ctxErr != nil {
return ctxErr
}
supportsHead = false

Check warning on line 418 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L412-L418

Added lines #L412 - L418 were not covered by tests
}

if resp.StatusCode >= 500 { // 5xx
log.Debugf("connect error %d <- %s %q", resp.StatusCode, p.ID, req.URL)
// We made a proper request and got a 5xx back.
// We cannot consider this a working connection.
continue
}
workingAddrs = append(workingAddrs, htaddrs.Addrs[i])

Check warning on line 421 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L421

Added line #L421 was not covered by tests
}

if len(workingAddrs) > 0 {
ht.host.Peerstore().AddAddrs(p.ID, workingAddrs, peerstore.PermanentAddrTTL)
// ignoring error
_ = ht.host.Peerstore().Put(p.ID, peerstoreSupportsHeadKey, supportsHead)

ht.connEvtMgr.Connected(p.ID)
ht.pinger.startPinging(p.ID)
log.Debugf("connect success to %s", p.ID)
log.Debugf("connect success to %s (supports HEAD: %t)", p.ID, supportsHead)
// We "connected"
return nil
}

Check warning on line 434 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L424-L434

Added lines #L424 - L434 were not covered by tests
Expand All @@ -453,6 +438,32 @@ func (ht *Network) Connect(ctx context.Context, p peer.AddrInfo) error {
return err

Check warning on line 438 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L436-L438

Added lines #L436 - L438 were not covered by tests
}

func (ht *Network) connect(ctx context.Context, p peer.ID, u network.ParsedURL, method string) error {
req, err := buildRequest(ctx, u, method, pingCid, ht.userAgent)
if err != nil {
log.Debug(err)
return err
}

Check warning on line 446 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L441-L446

Added lines #L441 - L446 were not covered by tests

log.Debugf("connect request to %s %s %q", p, method, req.URL)
resp, err := ht.client.Do(req)
if err != nil {
return err
}

Check warning on line 452 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L448-L452

Added lines #L448 - L452 were not covered by tests

if resp.Proto != http2proto {
log.Warnf("%s://%q is not using HTTP/2 (%s)", req.URL.Scheme, req.URL.Host, resp.Proto)
}

Check warning on line 456 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L454-L456

Added lines #L454 - L456 were not covered by tests

if resp.StatusCode >= 500 { // 5xx
log.Debugf("connect error: %d <- %q (%s)", resp.StatusCode, req.URL, p)
// We made a proper request and got a 5xx back.
// We cannot consider this a working connection.
return err
}
return nil

Check warning on line 464 in bitswap/network/httpnet/httpnet.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/httpnet.go#L458-L464

Added lines #L458 - L464 were not covered by tests
}

// DisconnectFrom marks this peer as Disconnected in the connection event
// manager, stops pinging for latency measurements and removes it from the
// peerstore.
Expand Down
13 changes: 7 additions & 6 deletions bitswap/network/httpnet/httpnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
return
}

if cidstr == pingCid {
rw.WriteHeader(http.StatusOK)
return
}

if c.Equals(errorCid) {
rw.WriteHeader(http.StatusInternalServerError)
return
Expand Down Expand Up @@ -451,9 +456,7 @@ func TestSendMessageWithPartialResponse(t *testing.T) {
func TestSendMessageSendHavesAndDontHaves(t *testing.T) {
ctx := context.Background()
recv := mockReceiver(t)
htnet, mn := mockNetwork(t, recv,
WithSupportsHave(true),
)
htnet, mn := mockNetwork(t, recv)
peer, err := mn.GenPeer()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -487,9 +490,7 @@ func TestSendMessageSendHavesAndDontHaves(t *testing.T) {
func TestSendCancels(t *testing.T) {
ctx := context.Background()
recv := mockReceiver(t)
htnet, mn := mockNetwork(t, recv,
WithSupportsHave(true),
)
htnet, mn := mockNetwork(t, recv)
peer, err := mn.GenPeer()
if err != nil {
t.Fatal(err)
Expand Down
11 changes: 8 additions & 3 deletions bitswap/network/httpnet/msg_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,15 @@ func (sender *httpMsgSender) Reset() error {
return nil

Check warning on line 542 in bitswap/network/httpnet/msg_sender.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/msg_sender.go#L538-L542

Added lines #L538 - L542 were not covered by tests
}

// SupportsHave can advertise whether we would like to support HAVE entries
// (they trigger HEAD requests).
// SupportsHave indicates whether the peer answers to HEAD requests.
// This has been probed during Connect().
func (sender *httpMsgSender) SupportsHave() bool {
return sender.ht.supportsHave
v, err := sender.ht.host.Peerstore().Get(sender.peer, peerstoreSupportsHeadKey)
if err != nil {
return false
}
b, ok := v.(bool)
return ok && b

Check warning on line 553 in bitswap/network/httpnet/msg_sender.go

View check run for this annotation

Codecov / codecov/patch

bitswap/network/httpnet/msg_sender.go#L547-L553

Added lines #L547 - L553 were not covered by tests
}

// parseRetryAfter returns how many seconds the Retry-After header header
Expand Down

0 comments on commit 612f23a

Please sign in to comment.