refactor(enginenetx): split generation and mixing

internal/enginenetx/bridgespolicy.go

@@ -12,6 +12,26 @@ import (
 	"time"
 )
 
+// bridgesPolicyV2 is a policy where we use bridges for communicating
+// with the OONI backend, i.e., api.ooni.io.
+//
+// A bridge is an IP address that can route traffic from and to
+// the OONI backend and accepts any SNI.
+//
+// The zero value is invalid; please, init MANDATORY fields.
+//
+// This is v2 of the bridgesPolicy because the previous implementation
+// incorporated mixing logic, while now the mixing happens outside
+// of this policy, thus giving us much more flexibility.
+type bridgesPolicyV2 struct{}
+
+var _ httpsDialerPolicy = &bridgesPolicyV2{}
+
+// LookupTactics implements httpsDialerPolicy.
+func (p *bridgesPolicyV2) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
+	return bridgesTacticsForDomain(domain, port)
+}
+
 // bridgesPolicy is a policy where we use bridges for communicating
 // with the OONI backend, i.e., api.ooni.io.
 //
@@ -31,7 +51,7 @@ func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string)
 	return mixSequentially(
 		// emit bridges related tactics first which are empty if there are
 		// no bridges for the givend domain and port
-		p.bridgesTacticsForDomain(domain, port),
+		bridgesTacticsForDomain(domain, port),
 
 		// now fallback to get more tactics (typically here the fallback
 		// uses the DNS and obtains some extra tactics)
@@ -42,14 +62,6 @@ func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string)
 	)
 }
 
-var bridgesPolicyTestHelpersDomains = []string{
-	"0.th.ooni.org",
-	"1.th.ooni.org",
-	"2.th.ooni.org",
-	"3.th.ooni.org",
-	"d33d1gs9kpq1c5.cloudfront.net",
-}
-
 func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
 	out := make(chan *httpsDialerTactic)
 
@@ -58,14 +70,14 @@ func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialer
 
 		for tactic := range input {
 			// When we're not connecting to a TH, pass the policy down the chain unmodified
-			if !slices.Contains(bridgesPolicyTestHelpersDomains, tactic.VerifyHostname) {
+			if !slices.Contains(testHelpersDomains, tactic.VerifyHostname) {
 				out <- tactic
 				continue
 			}
 
 			// This is the case where we're connecting to a test helper. Let's try
 			// to produce policies hiding the SNI to censoring middleboxes.
-			for _, sni := range p.bridgesDomainsInRandomOrder() {
+			for _, sni := range bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        tactic.Address,
 					InitialDelay:   0, // set when dialing
@@ -80,7 +92,7 @@ func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialer
 	return out
 }
 
-func (p *bridgesPolicy) bridgesTacticsForDomain(domain, port string) <-chan *httpsDialerTactic {
+func bridgesTacticsForDomain(domain, port string) <-chan *httpsDialerTactic {
 	out := make(chan *httpsDialerTactic)
 
 	go func() {
@@ -91,8 +103,8 @@ func (p *bridgesPolicy) bridgesTacticsForDomain(domain, port string) <-chan *htt
 			return
 		}
 
-		for _, ipAddr := range p.bridgesAddrs() {
-			for _, sni := range p.bridgesDomainsInRandomOrder() {
+		for _, ipAddr := range bridgesAddrs() {
+			for _, sni := range bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        ipAddr,
 					InitialDelay:   0, // set when dialing
@@ -107,23 +119,23 @@ func (p *bridgesPolicy) bridgesTacticsForDomain(domain, port string) <-chan *htt
 	return out
 }
 
-func (p *bridgesPolicy) bridgesDomainsInRandomOrder() (out []string) {
-	out = p.bridgesDomains()
+func bridgesDomainsInRandomOrder() (out []string) {
+	out = bridgesDomains()
 	r := rand.New(rand.NewSource(time.Now().UnixNano()))
 	r.Shuffle(len(out), func(i, j int) {
 		out[i], out[j] = out[j], out[i]
 	})
 	return
 }
 
-func (p *bridgesPolicy) bridgesAddrs() (out []string) {
+func bridgesAddrs() (out []string) {
 	return append(
 		out,
 		"162.55.247.208",
 	)
 }
 
-func (p *bridgesPolicy) bridgesDomains() (out []string) {
+func bridgesDomains() (out []string) {
 	// See https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40273
 	return append(
 		out,

internal/enginenetx/bridgespolicy_test.go

@@ -9,6 +9,61 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
+func TestBridgesPolicyV2(t *testing.T) {
+	t.Run("for domains for which we don't have bridges", func(t *testing.T) {
+		p := &bridgesPolicyV2{}
+
+		tactics := p.LookupTactics(context.Background(), "www.example.com", "443")
+
+		var count int
+		for range tactics {
+			count++
+		}
+
+		if count != 0 {
+			t.Fatal("expected to see zero tactics")
+		}
+	})
+
+	t.Run("for the api.ooni.io domain", func(t *testing.T) {
+		p := &bridgesPolicyV2{}
+
+		tactics := p.LookupTactics(context.Background(), "api.ooni.io", "443")
+
+		var count int
+		for tactic := range tactics {
+			count++
+
+			// for each generated tactic, make sure we're getting the
+			// expected value for each of the fields
+
+			if tactic.Port != "443" {
+				t.Fatal("the port should always be 443")
+			}
+
+			if tactic.Address != "162.55.247.208" {
+				t.Fatal("the host should always be 162.55.247.208")
+			}
+
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
+			if tactic.SNI == "api.ooni.io" {
+				t.Fatal("we should not see the `api.ooni.io` SNI on the wire")
+			}
+
+			if tactic.VerifyHostname != "api.ooni.io" {
+				t.Fatal("the VerifyHostname field should always be like `api.ooni.io`")
+			}
+		}
+
+		if count <= 0 {
+			t.Fatal("expected to see at least one tactic")
+		}
+	})
+}
+
 func TestBridgesPolicy(t *testing.T) {
 	t.Run("for domains for which we don't have bridges and DNS failure", func(t *testing.T) {
 		expected := errors.New("mocked error")
@@ -202,7 +257,7 @@ func TestBridgesPolicy(t *testing.T) {
 	})
 
 	t.Run("for test helper domains", func(t *testing.T) {
-		for _, domain := range bridgesPolicyTestHelpersDomains {
+		for _, domain := range testHelpersDomains {
 			t.Run(domain, func(t *testing.T) {
 				expectedAddrs := []string{"164.92.180.7"}
 

internal/enginenetx/dnspolicy.go

@@ -16,8 +16,6 @@ import (
 // given resolver and the domain as the SNI.
 //
 // The zero value is invalid; please, init all MANDATORY fields.
-//
-// This policy uses an Happy-Eyeballs-like algorithm.
 type dnsPolicy struct {
 	// Logger is the MANDATORY logger.
 	Logger model.Logger

internal/enginenetx/httpsdialer.go

@@ -202,6 +202,16 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 		return nil, err
 	}
 
+	// TODO(bassosimone): this code should be refactored using the same
+	// pattern used by `./internal/httpclientx` to perform attempts faster
+	// in case there is an initial early failure.
+
+	// TODO(bassosimone): the algorithm to filter and assign initial
+	// delays is broken because, if the DNS runs for more than one
+	// second, then several policies will immediately be due. We should
+	// probably use a better strategy that takes as the zero the time
+	// when the first dialing policy becomes available.
+
 	// We need a cancellable context to interrupt the tactics emitter early when we
 	// immediately get a valid response and we don't need to use other tactics.
 	ctx, cancel := context.WithCancel(ctx)
@@ -319,6 +329,9 @@ func (hd *httpsDialer) dialTLS(
 		return nil, err
 	}
 
+	// for debugging let the user know which tactic is ready
+	logger.Infof("tactic '%+v' is ready", tactic)
+
 	// tell the observer that we're starting
 	hd.stats.OnStarting(tactic)
 

internal/enginenetx/mixpolicy.go

@@ -0,0 +1,133 @@
+package enginenetx
+
+//
+// Mix policies - ability of mixing from a primary policy and a fallback policy
+// in a more flexible way than strictly falling back
+//
+
+import (
+	"context"
+
+	"github.com/ooni/probe-cli/v3/internal/optional"
+)
+
+// mixPolicyEitherOr reads from primary and only if primary does
+// not return any tactic, then it reads from fallback.
+type mixPolicyEitherOr struct {
+	// Primary is the primary policy.
+	Primary httpsDialerPolicy
+
+	// Fallback is the fallback policy.
+	Fallback httpsDialerPolicy
+}
+
+var _ httpsDialerPolicy = &mixPolicyEitherOr{}
+
+// LookupTactics implements httpsDialerPolicy.
+func (m *mixPolicyEitherOr) LookupTactics(ctx context.Context, domain string, port string) <-chan *httpsDialerTactic {
+	// create the output channel
+	output := make(chan *httpsDialerTactic)
+
+	go func() {
+		// make sure we eventually close the output channel
+		defer close(output)
+
+		// drain the primary policy
+		var count int
+		for tx := range m.Primary.LookupTactics(ctx, domain, port) {
+			output <- tx
+			count++
+		}
+
+		// if the primary worked, we're good
+		if count > 0 {
+			return
+		}
+
+		// drain the fallback policy
+		for tx := range m.Fallback.LookupTactics(ctx, domain, port) {
+			output <- tx
+		}
+	}()
+
+	return output
+}
+
+// mixPolicyInterleave interleaves policies by a given interleaving
+// factor. Say the interleave factor is N, then we first read N tactics
+// from the primary policy, then N from the fallback one, and we keep
+// going on like this until we've read all the tactics from both.
+type mixPolicyInterleave struct {
+	// Primary is the primary policy. We will read N from this
+	// policy first, then N from fallback, and so on.
+	Primary httpsDialerPolicy
+
+	// Fallback is the fallback policy.
+	Fallback httpsDialerPolicy
+
+	// Factor is the interleaving factor to use. If this value is
+	// zero, we behave like it was set to one.
+	Factor uint8
+}
+
+var _ httpsDialerPolicy = &mixPolicyInterleave{}
+
+// LookupTactics implements httpsDialerPolicy.
+func (p *mixPolicyInterleave) LookupTactics(ctx context.Context, domain string, port string) <-chan *httpsDialerTactic {
+	// create the output channel
+	output := make(chan *httpsDialerTactic)
+
+	go func() {
+		// make sure we eventually close the output channel
+		defer close(output)
+
+		// obtain the primary channel
+		primary := optional.Some(p.Primary.LookupTactics(ctx, domain, port))
+
+		// obtain the fallback channel
+		fallback := optional.Some(p.Fallback.LookupTactics(ctx, domain, port))
+
+		// loop until both channels are drained
+		for !primary.IsNone() || !fallback.IsNone() {
+			// take N from primary if possible
+			primary = p.maybeTakeN(primary, output)
+
+			// take N from secondary if possible
+			fallback = p.maybeTakeN(fallback, output)
+		}
+	}()
+
+	return output
+}
+
+// maybeTakeN takes N entries from input if it's not none. When input is not
+// none and reading from it indicates EOF, this function returns none. Otherwise,
+// it returns the same value given as input.
+func (p *mixPolicyInterleave) maybeTakeN(
+	input optional.Value[<-chan *httpsDialerTactic],
+	output chan<- *httpsDialerTactic,
+) optional.Value[<-chan *httpsDialerTactic] {
+	// make sure we've not already drained this channel
+	if !input.IsNone() {
+
+		// obtain the underlying channel
+		ch := input.Unwrap()
+
+		// take N entries from the channel
+		for idx := uint8(0); idx < max(1, p.Factor); idx++ {
+
+			// attempt to get the next tactic
+			tactic, good := <-ch
+
+			// handle the case where the channel has been drained
+			if !good {
+				return optional.None[<-chan *httpsDialerTactic]()
+			}
+
+			// emit the tactic
+			output <- tactic
+		}
+	}
+
+	return input
+}