Skip to content

Commit

Permalink
fix(enginenetx): pass context to tactics callbacks (ooni#1286)
Browse files Browse the repository at this point in the history
This diff modifies the tactics callbacks to take in input a context.

We need the context to know whether the operation failed in itself or
was canceled externally through the context.

With this information, we can store better statistics about which
tactics are working and which tactics are not working.

While there, fix a couple of typos (thanks @robertodauria!)

Part of ooni/probe#2531
  • Loading branch information
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent 236338b commit 3c92f1b
Showing 1 changed file with 17 additions and 11 deletions.
28 changes: 17 additions & 11 deletions internal/enginenetx/httpsdialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ type HTTPSDialerPolicy interface {
// LookupTactics performs a DNS lookup for the given domain using the given resolver and
// returns either a list of tactics for dialing or an error.
//
// This functoion MUST NOT return an empty list and a nil error. If this happens the
// code inside [HTTPSDialer] will panic.
// This function MUST NOT return an empty list and a nil error. If this happens the
// code inside [HTTPSDialer] will PANIC.
LookupTactics(ctx context.Context, domain string, reso model.Resolver) ([]HTTPSDialerTactic, error)

// Parallelism returns the number of goroutines to create when TLS dialing. The
Expand All @@ -47,10 +47,16 @@ type HTTPSDialerTactic interface {
// These callbacks are invoked during the TLS handshake to inform this
// tactic about events that occurred. A tactic SHOULD keep track of which
// addresses, SNIs, etc. work and return them more frequently.
//
// Callbacks that take an error as argument also take a context as
// argument and MUST check whether the context has been canceled or
// its timeout has expired (i.e., using ctx.Err()) to determine
// whether the operation failed or was merely canceled. In the latter
// case, obviously, the policy MUST NOT consider the tactic failed.
OnStarting()
OnTCPConnectError(err error)
OnTLSHandshakeError(err error)
OnTLSVerifyError(err error)
OnTCPConnectError(ctx context.Context, err error)
OnTLSHandshakeError(ctx context.Context, err error)
OnTLSVerifyError(ctz context.Context, err error)
OnSuccess()

// SNI returns the SNI to send in the TLS Client Hello.
Expand Down Expand Up @@ -146,17 +152,17 @@ func (*httpsDialerNullTactic) OnSuccess() {
}

// OnTCPConnectError implements HTTPSDialerTactic.
func (*httpsDialerNullTactic) OnTCPConnectError(err error) {
func (*httpsDialerNullTactic) OnTCPConnectError(ctx context.Context, err error) {
// nothing
}

// OnTLSHandshakeError implements HTTPSDialerTactic.
func (*httpsDialerNullTactic) OnTLSHandshakeError(err error) {
func (*httpsDialerNullTactic) OnTLSHandshakeError(ctx context.Context, err error) {
// nothing
}

// OnTLSVerifyError implements HTTPSDialerTactic.
func (*httpsDialerNullTactic) OnTLSVerifyError(err error) {
func (*httpsDialerNullTactic) OnTLSVerifyError(ctx context.Context, err error) {
// nothing
}

Expand Down Expand Up @@ -413,7 +419,7 @@ func (hd *HTTPSDialer) dialTLS(ctx context.Context,

// handle a dialing error
if err != nil {
tactic.OnTCPConnectError(err)
tactic.OnTCPConnectError(ctx, err)
return nil, err
}

Expand All @@ -439,7 +445,7 @@ func (hd *HTTPSDialer) dialTLS(ctx context.Context,

// handle handshake error
if err != nil {
tactic.OnTLSHandshakeError(err)
tactic.OnTLSHandshakeError(ctx, err)
tcpConn.Close()
return nil, err
}
Expand All @@ -451,7 +457,7 @@ func (hd *HTTPSDialer) dialTLS(ctx context.Context,

// handle verification error
if err != nil {
tactic.OnTLSVerifyError(err)
tactic.OnTLSVerifyError(ctx, err)
tlsConn.Close()
return nil, err
}
Expand Down

0 comments on commit 3c92f1b

Please sign in to comment.