Skip to content

Commit

Permalink
refactor(enginenetx): make https-dialer API private (#1310)
Browse files Browse the repository at this point in the history
What needs to be public is the network API, while all the rest can and
should instead be private.

Part of ooni/probe#2531
  • Loading branch information
bassosimone authored Sep 26, 2023
1 parent 4384343 commit 51bf872
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 154 deletions.
16 changes: 8 additions & 8 deletions internal/enginenetx/beacons.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import (
// The zero value is invalid; please, init MANDATORY fields.
type beaconsPolicy struct {
// Fallback is the MANDATORY fallback policy.
Fallback HTTPSDialerPolicy
Fallback httpsDialerPolicy
}

var _ HTTPSDialerPolicy = &beaconsPolicy{}
var _ httpsDialerPolicy = &beaconsPolicy{}

// LookupTactics implements HTTPSDialerPolicy.
func (p *beaconsPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *HTTPSDialerTactic {
out := make(chan *HTTPSDialerTactic)
// LookupTactics implements httpsDialerPolicy.
func (p *beaconsPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
out := make(chan *httpsDialerTactic)

go func() {
defer close(out)
Expand All @@ -51,8 +51,8 @@ func (p *beaconsPolicy) LookupTactics(ctx context.Context, domain, port string)
return out
}

func (p *beaconsPolicy) tacticsForDomain(domain, port string) <-chan *HTTPSDialerTactic {
out := make(chan *HTTPSDialerTactic)
func (p *beaconsPolicy) tacticsForDomain(domain, port string) <-chan *httpsDialerTactic {
out := make(chan *httpsDialerTactic)

go func() {
defer close(out)
Expand All @@ -72,7 +72,7 @@ func (p *beaconsPolicy) tacticsForDomain(domain, port string) <-chan *HTTPSDiale

for _, ipAddr := range ipAddrs {
for _, sni := range snis {
out <- &HTTPSDialerTactic{
out <- &httpsDialerTactic{
Address: ipAddr,
InitialDelay: 0,
Port: port,
Expand Down
12 changes: 6 additions & 6 deletions internal/enginenetx/dnspolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ type dnsPolicy struct {
Resolver model.Resolver
}

var _ HTTPSDialerPolicy = &dnsPolicy{}
var _ httpsDialerPolicy = &dnsPolicy{}

// LookupTactics implements HTTPSDialerPolicy.
// LookupTactics implements httpsDialerPolicy.
func (p *dnsPolicy) LookupTactics(
ctx context.Context, domain, port string) <-chan *HTTPSDialerTactic {
out := make(chan *HTTPSDialerTactic)
ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
out := make(chan *httpsDialerTactic)

go func() {
// make sure we close the output channel when done
Expand All @@ -40,7 +40,7 @@ func (p *dnsPolicy) LookupTactics(
// Do not even start the DNS lookup if the context has already been canceled, which
// happens if some policy running before us had successfully connected
if err := ctx.Err(); err != nil {
p.Logger.Debugf("HTTPSDialerNullPolicy: LookupTactics: %s", err.Error())
p.Logger.Debugf("dnsPolicy: LookupTactics: %s", err.Error())
return
}

Expand All @@ -55,7 +55,7 @@ func (p *dnsPolicy) LookupTactics(
}

for idx, addr := range addrs {
tactic := &HTTPSDialerTactic{
tactic := &httpsDialerTactic{
Address: addr,
InitialDelay: happyEyeballsDelay(idx),
Port: port,
Expand Down
2 changes: 1 addition & 1 deletion internal/enginenetx/dnspolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/ooni/probe-cli/v3/internal/model"
)

func TestHTTPSDialerNullPolicy(t *testing.T) {
func TestDNSPolicy(t *testing.T) {
t.Run("LookupTactics with canceled context", func(t *testing.T) {
var called int

Expand Down
86 changes: 44 additions & 42 deletions internal/enginenetx/httpsdialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

// HTTPSDialerTactic is a tactic to establish a TLS connection.
type HTTPSDialerTactic struct {
// httpsDialerTactic is a tactic to establish a TLS connection.
type httpsDialerTactic struct {
// Address is the IPv4/IPv6 address for dialing.
Address string

Expand All @@ -37,11 +37,11 @@ type HTTPSDialerTactic struct {
VerifyHostname string
}

var _ fmt.Stringer = &HTTPSDialerTactic{}
var _ fmt.Stringer = &httpsDialerTactic{}

// Clone makes a deep copy of this [HTTPSDialerTactic].
func (dt *HTTPSDialerTactic) Clone() *HTTPSDialerTactic {
return &HTTPSDialerTactic{
// Clone makes a deep copy of this [httpsDialerTactic].
func (dt *httpsDialerTactic) Clone() *httpsDialerTactic {
return &httpsDialerTactic{
Address: dt.Address,
InitialDelay: dt.InitialDelay,
Port: dt.Port,
Expand All @@ -51,39 +51,41 @@ func (dt *HTTPSDialerTactic) Clone() *HTTPSDialerTactic {
}

// String implements fmt.Stringer.
func (dt *HTTPSDialerTactic) String() string {
func (dt *httpsDialerTactic) String() string {
return string(runtimex.Try1(json.Marshal(dt)))
}

// Summary returns a string summarizing this [HTTPSDialerTactic] for the
// specific purpose of inserting the struct into a map.
// tacticSummaryKey returns a string summarizing this [httpsDialerTactic] for
// the specific purpose of inserting the struct into a map.
//
// The fields used to compute the summary are:
//
// - IPAddr
//
// - Port
//
// - SNI
//
// - VerifyHostname
//
// The returned string contains the above fields separated by space.
func (dt *HTTPSDialerTactic) Summary() string {
func (dt *httpsDialerTactic) tacticSummaryKey() string {
return fmt.Sprintf("%v sni=%v verify=%v", net.JoinHostPort(dt.Address, dt.Port), dt.SNI, dt.VerifyHostname)
}

// domainEndpointKey returns the domain's endpoint string key for storing into a map.
func (dt *HTTPSDialerTactic) domainEndpointKey() string {
func (dt *httpsDialerTactic) domainEndpointKey() string {
return net.JoinHostPort(dt.VerifyHostname, dt.Port)
}

// HTTPSDialerPolicy describes the policy used by the [*HTTPSDialer].
type HTTPSDialerPolicy interface {
// httpsDialerPolicy describes the policy used by the [*httpsDialer].
type httpsDialerPolicy interface {
// LookupTactics returns zero or more tactics for the given host and port.
LookupTactics(ctx context.Context, domain, port string) <-chan *HTTPSDialerTactic
LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic
}

// HTTPSDialerStatsTracker tracks what happens while dialing TLS connections.
type HTTPSDialerStatsTracker interface {
// httpsDialerStatsTracker tracks what happens while dialing TLS connections.
type httpsDialerStatsTracker 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.
Expand All @@ -93,20 +95,20 @@ type HTTPSDialerStatsTracker interface {
// 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(tactic *HTTPSDialerTactic)
OnTCPConnectError(ctx context.Context, tactic *HTTPSDialerTactic, err error)
OnTLSHandshakeError(ctx context.Context, tactic *HTTPSDialerTactic, err error)
OnTLSVerifyError(tactic *HTTPSDialerTactic, err error)
OnSuccess(tactic *HTTPSDialerTactic)
OnStarting(tactic *httpsDialerTactic)
OnTCPConnectError(ctx context.Context, tactic *httpsDialerTactic, err error)
OnTLSHandshakeError(ctx context.Context, tactic *httpsDialerTactic, err error)
OnTLSVerifyError(tactic *httpsDialerTactic, err error)
OnSuccess(tactic *httpsDialerTactic)
}

// HTTPSDialer is the [model.TLSDialer] used by the engine to dial HTTPS connections.
// httpsDialer is the [model.TLSDialer] used by the engine to dial HTTPS connections.
//
// The zero value of this struct is invalid; construct using [NewHTTPSDialer].
// The zero value of this struct is invalid; construct using [newHTTPSDialer].
//
// This dialer MAY use an happy-eyeballs-like policy where we may try several IP addresses,
// including IPv4 and IPv6, and dialing tactics in parallel.
type HTTPSDialer struct {
type httpsDialer struct {
// idGenerator is the ID generator.
idGenerator *atomic.Int64

Expand All @@ -117,16 +119,16 @@ type HTTPSDialer struct {
netx *netxlite.Netx

// policy defines the dialing policy to use.
policy HTTPSDialerPolicy
policy httpsDialerPolicy

// rootCAs contains the root certificate pool we should use.
rootCAs *x509.CertPool

// stats tracks what happens while dialing.
stats HTTPSDialerStatsTracker
stats httpsDialerStatsTracker
}

// NewHTTPSDialer constructs a new [*HTTPSDialer] instance.
// newHTTPSDialer constructs a new [*httpsDialer] instance.
//
// Arguments:
//
Expand All @@ -138,18 +140,18 @@ type HTTPSDialer struct {
//
// - stats tracks what happens while we're dialing.
//
// The returned [*HTTPSDialer] would use the underlying network's
// The returned [*httpsDialer] would use the underlying network's
// DefaultCertPool to create and cache the cert pool to use.
func NewHTTPSDialer(
func newHTTPSDialer(
logger model.Logger,
netx *netxlite.Netx,
policy HTTPSDialerPolicy,
stats HTTPSDialerStatsTracker,
) *HTTPSDialer {
return &HTTPSDialer{
policy httpsDialerPolicy,
stats httpsDialerStatsTracker,
) *httpsDialer {
return &httpsDialer{
idGenerator: &atomic.Int64{},
logger: &logx.PrefixLogger{
Prefix: "HTTPSDialer: ",
Prefix: "httpsDialer: ",
Logger: logger,
},
netx: netx,
Expand All @@ -159,10 +161,10 @@ func NewHTTPSDialer(
}
}

var _ model.TLSDialer = &HTTPSDialer{}
var _ model.TLSDialer = &httpsDialer{}

// CloseIdleConnections implements model.TLSDialer.
func (hd *HTTPSDialer) CloseIdleConnections() {
func (hd *httpsDialer) CloseIdleConnections() {
// nothing
}

Expand All @@ -183,7 +185,7 @@ var errDNSNoAnswer = netxlite.NewErrWrapper(
)

// DialTLSContext implements model.TLSDialer.
func (hd *HTTPSDialer) DialTLSContext(ctx context.Context, network string, endpoint string) (net.Conn, error) {
func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpoint string) (net.Conn, error) {
hostname, port, err := net.SplitHostPort(endpoint)
if err != nil {
return nil, err
Expand Down Expand Up @@ -252,10 +254,10 @@ func httpsDialerReduceResult(connv []model.TLSConn, errorv []error) (model.TLSCo

// worker attempts to establish a TLS connection using and emits a single
// [*httpsDialerErrorOrConn] for each tactic.
func (hd *HTTPSDialer) worker(
func (hd *httpsDialer) worker(
ctx context.Context,
joiner chan<- any,
reader <-chan *HTTPSDialerTactic,
reader <-chan *httpsDialerTactic,
t0 time.Time,
writer chan<- *httpsDialerErrorOrConn,
) {
Expand All @@ -275,11 +277,11 @@ func (hd *HTTPSDialer) worker(
}

// dialTLS performs the actual TLS dial.
func (hd *HTTPSDialer) dialTLS(
func (hd *httpsDialer) dialTLS(
ctx context.Context,
logger model.Logger,
t0 time.Time,
tactic *HTTPSDialerTactic,
tactic *httpsDialerTactic,
) (model.TLSConn, error) {
// wait for the tactic to be ready to run
if err := httpsDialerTacticWaitReady(ctx, t0, tactic); err != nil {
Expand Down Expand Up @@ -353,7 +355,7 @@ func (hd *HTTPSDialer) dialTLS(
func httpsDialerTacticWaitReady(
ctx context.Context,
t0 time.Time,
tactic *HTTPSDialerTactic,
tactic *httpsDialerTactic,
) error {
deadline := t0.Add(tactic.InitialDelay)
delta := time.Until(deadline)
Expand Down
Loading

0 comments on commit 51bf872

Please sign in to comment.