Merge 4bd4f4960f into 7dab5a2981

We need deterministic+random mixing of HTTPS dial tactics to ensure that
we prioritize some tactics coming from the DNS before attempting all the
previous tactics, which would make the bootstrap super slow.

Part of https://github.com/ooni/probe/issues/2704.

internal/cmd/oohelper/internal/client.go

@@ -108,7 +108,8 @@ func (oo OOClient) Do(ctx context.Context, config OOConfig) (*CtrlResponse, erro
 			"Accept-Language": {model.HTTPHeaderAcceptLanguage},
 			"User-Agent":      {model.HTTPHeaderUserAgent},
 		},
-		TCPConnect: endpoints,
+		TCPConnect:   endpoints,
+		XQUICEnabled: true,
 	}
 	data, err := json.Marshal(creq)
 	runtimex.PanicOnError(err, "oohelper: cannot marshal control request")

internal/enginenetx/bridgespolicy.go

@@ -27,33 +27,18 @@ var _ httpsDialerPolicy = &bridgesPolicy{}
 
 // LookupTactics implements httpsDialerPolicy.
 func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
-	out := make(chan *httpsDialerTactic)
-
-	go func() {
-		defer close(out) // tell the parent when we're done
-		index := 0
-
+	return mixSequentially(
 		// emit bridges related tactics first which are empty if there are
 		// no bridges for the givend domain and port
-		for tx := range p.bridgesTacticsForDomain(domain, port) {
-			tx.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- tx
-		}
+		p.bridgesTacticsForDomain(domain, port),
 
 		// now fallback to get more tactics (typically here the fallback
 		// uses the DNS and obtains some extra tactics)
 		//
 		// we wrap whatever the underlying policy returns us with some
 		// extra logic for better communicating with test helpers
-		for tx := range p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)) {
-			tx.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- tx
-		}
-	}()
-
-	return out
+		p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)),
+	)
 }
 
 var bridgesPolicyTestHelpersDomains = []string{
@@ -92,7 +77,7 @@ func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialer
 			for _, sni := range p.bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        tactic.Address,
-					InitialDelay:   0,
+					InitialDelay:   0, // set when dialing
 					Port:           tactic.Port,
 					SNI:            sni,
 					VerifyHostname: tactic.VerifyHostname,
@@ -119,7 +104,7 @@ func (p *bridgesPolicy) bridgesTacticsForDomain(domain, port string) <-chan *htt
 			for _, sni := range p.bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        ipAddr,
-					InitialDelay:   0,
+					InitialDelay:   0, // set when dialing
 					Port:           port,
 					SNI:            sni,
 					VerifyHostname: domain,

internal/enginenetx/bridgespolicy_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
-func TestBeaconsPolicy(t *testing.T) {
+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")
 		p := &bridgesPolicy{
@@ -62,6 +62,10 @@ func TestBeaconsPolicy(t *testing.T) {
 				t.Fatal("the host should always be 93.184.216.34")
 			}
 
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
 			if tactic.SNI != "www.example.com" {
 				t.Fatal("the SNI field should always be like `www.example.com`")
 			}
@@ -76,7 +80,7 @@ func TestBeaconsPolicy(t *testing.T) {
 		}
 	})
 
-	t.Run("for the api.ooni.io domain", func(t *testing.T) {
+	t.Run("for the api.ooni.io domain with DNS failure", func(t *testing.T) {
 		expected := errors.New("mocked error")
 		p := &bridgesPolicy{
 			Fallback: &dnsPolicy{
@@ -92,6 +96,7 @@ func TestBeaconsPolicy(t *testing.T) {
 		ctx := context.Background()
 		tactics := p.LookupTactics(ctx, "api.ooni.io", "443")
 
+		// since the DNS fails, we should only see tactics generated by bridges
 		var count int
 		for tactic := range tactics {
 			count++
@@ -103,6 +108,10 @@ func TestBeaconsPolicy(t *testing.T) {
 				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")
 			}
@@ -117,6 +126,81 @@ func TestBeaconsPolicy(t *testing.T) {
 		}
 	})
 
+	t.Run("for the api.ooni.io domain with DNS success", func(t *testing.T) {
+		p := &bridgesPolicy{
+			Fallback: &dnsPolicy{
+				Logger: model.DiscardLogger,
+				Resolver: &mocks.Resolver{
+					MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+						return []string{"130.192.91.211"}, nil
+					},
+				},
+			},
+		}
+
+		ctx := context.Background()
+		tactics := p.LookupTactics(ctx, "api.ooni.io", "443")
+
+		// since the DNS succeeds we should see bridge tactics mixed with DNS tactics
+		var (
+			bridgesCount int
+			dnsCount     int
+			overallCount int
+		)
+		const expectedDNSEntryCount = 153 // yikes!
+		for tactic := range tactics {
+			overallCount++
+
+			t.Log(overallCount, tactic)
+
+			if tactic.Port != "443" {
+				t.Fatal("the port should always be 443")
+			}
+
+			switch {
+			case overallCount == expectedDNSEntryCount:
+				if tactic.Address != "130.192.91.211" {
+					t.Fatal("the host should be 130.192.91.211 for count ==", expectedDNSEntryCount)
+				}
+
+				if tactic.SNI != "api.ooni.io" {
+					t.Fatal("we should see the `api.ooni.io` SNI on the wire for count ==", expectedDNSEntryCount)
+				}
+
+				dnsCount++
+
+			default:
+				if tactic.Address != "162.55.247.208" {
+					t.Fatal("the host should be 162.55.247.208 for count !=", expectedDNSEntryCount)
+				}
+
+				if tactic.SNI == "api.ooni.io" {
+					t.Fatal("we should not see the `api.ooni.io` SNI on the wire for count !=", expectedDNSEntryCount)
+				}
+
+				bridgesCount++
+			}
+
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
+			if tactic.VerifyHostname != "api.ooni.io" {
+				t.Fatal("the VerifyHostname field should always be like `api.ooni.io`")
+			}
+		}
+
+		if overallCount <= 0 {
+			t.Fatal("expected to see at least one tactic")
+		}
+		if dnsCount != 1 {
+			t.Fatal("expected to see exactly one DNS based tactic")
+		}
+		if bridgesCount <= 0 {
+			t.Fatal("expected to see at least one bridge tactic")
+		}
+	})
+
 	t.Run("for test helper domains", func(t *testing.T) {
 		for _, domain := range bridgesPolicyTestHelpersDomains {
 			t.Run(domain, func(t *testing.T) {
@@ -134,27 +218,25 @@ func TestBeaconsPolicy(t *testing.T) {
 				}
 
 				ctx := context.Background()
-				index := 0
-				for tactics := range p.LookupTactics(ctx, domain, "443") {
+				for tactic := range p.LookupTactics(ctx, domain, "443") {
 
-					if tactics.Address != "164.92.180.7" {
+					if tactic.Address != "164.92.180.7" {
 						t.Fatal("unexpected .Address")
 					}
 
-					if tactics.InitialDelay != happyEyeballsDelay(index) {
+					if tactic.InitialDelay != 0 {
 						t.Fatal("unexpected .InitialDelay")
 					}
-					index++
 
-					if tactics.Port != "443" {
+					if tactic.Port != "443" {
 						t.Fatal("unexpected .Port")
 					}
 
-					if tactics.SNI == domain {
+					if tactic.SNI == domain {
 						t.Fatal("unexpected .Domain")
 					}
 
-					if tactics.VerifyHostname != domain {
+					if tactic.VerifyHostname != domain {
 						t.Fatal("unexpected .VerifyHostname")
 					}
 				}

internal/enginenetx/dnspolicy.go

@@ -56,10 +56,10 @@ func (p *dnsPolicy) LookupTactics(
 		}
 
 		// The tactics we generate here have SNI == VerifyHostname == domain
-		for idx, addr := range addrs {
+		for _, addr := range addrs {
 			tactic := &httpsDialerTactic{
 				Address:        addr,
-				InitialDelay:   happyEyeballsDelay(idx),
+				InitialDelay:   0, // set when dialing
 				Port:           port,
 				SNI:            domain,
 				VerifyHostname: domain,

internal/enginenetx/dnspolicy_test.go

@@ -54,6 +54,9 @@ func TestDNSPolicy(t *testing.T) {
 			if tactic.Address != "130.192.91.211" {
 				t.Fatal("invalid endpoint address")
 			}
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected .InitialDelay")
+			}
 			if tactic.Port != "443" {
 				t.Fatal("invalid endpoint port")
 			}

internal/enginenetx/filter.go

@@ -0,0 +1,73 @@
+package enginenetx
+
+// filterOutNilTactics filters out nil tactics.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func filterOutNilTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		for tx := range input {
+			if tx != nil {
+				output <- tx
+			}
+		}
+	}()
+	return output
+}
+
+// filterOnlyKeepUniqueTactics only keeps unique tactics.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func filterOnlyKeepUniqueTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+
+		// make sure we close output chan
+		defer close(output)
+
+		// useful to make sure we don't emit two equal policy in a single run
+		uniq := make(map[string]int)
+
+		for tx := range input {
+			// handle the case in which we already emitted a tactic
+			key := tx.tacticSummaryKey()
+			if uniq[key] > 0 {
+				continue
+			}
+			uniq[key]++
+
+			// emit the tactic
+			output <- tx
+		}
+
+	}()
+	return output
+}
+
+// filterAssignInitialDelays assigns initial delays to tactics.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func filterAssignInitialDelays(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+
+		// make sure we close output chan
+		defer close(output)
+
+		index := 0
+		for tx := range input {
+			// rewrite the delays
+			tx.InitialDelay = happyEyeballsDelay(index)
+			index++
+
+			// emit the tactic
+			output <- tx
+		}
+
+	}()
+	return output
+}

internal/enginenetx/filter_test.go

@@ -0,0 +1,111 @@
+package enginenetx
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestFilterOutNilTactics(t *testing.T) {
+	inputs := []*httpsDialerTactic{
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+	}
+
+	expect := []*httpsDialerTactic{
+		inputs[2], inputs[4],
+	}
+
+	var output []*httpsDialerTactic
+	for tx := range filterOutNilTactics(streamTacticsFromSlice(inputs)) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestFilterOnlyKeepUniqueTactics(t *testing.T) {
+	templates := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "www.example.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "www.kernel.org",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "x.org",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "www.polito.it",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	inputs := []*httpsDialerTactic{
+		templates[2], templates[1], templates[1],
+		templates[2], templates[2], templates[1],
+		templates[0], templates[1], templates[0],
+		templates[2], templates[1], templates[2],
+		templates[1], templates[0], templates[1],
+		templates[3], // only once at the end
+	}
+
+	expect := []*httpsDialerTactic{
+		templates[2], templates[1], templates[0], templates[3],
+	}
+
+	var output []*httpsDialerTactic
+	for tx := range filterOnlyKeepUniqueTactics(streamTacticsFromSlice(inputs)) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestFilterAssignInitalDelays(t *testing.T) {
+	inputs := []*httpsDialerTactic{}
+	ff := &testingx.FakeFiller{}
+	ff.Fill(&inputs)
+	idx := 0
+	for tx := range filterAssignInitialDelays(streamTacticsFromSlice(inputs)) {
+		if tx.InitialDelay != happyEyeballsDelay(idx) {
+			t.Fatal("unexpected .InitialDelay", tx.InitialDelay, "for", idx)
+		}
+		idx++
+	}
+	if idx < 1 {
+		t.Fatal("expected to see at least one entry")
+	}
+}

internal/enginenetx/httpsdialer.go

@@ -97,7 +97,7 @@ type httpsDialerPolicy interface {
 
 // httpsDialerEventsHandler handles events occurring while we try dialing TLS.
 type httpsDialerEventsHandler interface {
-	// These callbacks are invoked during the TLS handshake to inform this
+	// These callbacks are invoked during the TLS dialing to inform this
 	// interface about events that occurred. A policy SHOULD keep track of which
 	// addresses, SNIs, etc. work and return them more frequently.
 	//
@@ -209,7 +209,7 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 
 	// The emitter will emit tactics and then close the channel when done. We spawn 16 workers
 	// that handle tactics in parallel and post results on the collector channel.
-	emitter := hd.policy.LookupTactics(ctx, hostname, port)
+	emitter := httpsDialerFilterTactics(hd.policy.LookupTactics(ctx, hostname, port))
 	collector := make(chan *httpsDialerErrorOrConn)
 	joiner := make(chan any)
 	const parallelism = 16
@@ -236,8 +236,10 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 				continue
 			}
 
-			// Save the conn and tell goroutines to stop ASAP
+			// Save the conn
 			connv = append(connv, result.Conn)
+
+			// Interrupt other concurrent dialing attempts
 			cancel()
 		}
 	}
@@ -245,6 +247,20 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 	return httpsDialerReduceResult(connv, errorv)
 }
 
+// httpsDialerFilterTactics filters the tactics to:
+//
+// 1. be paranoid and filter out nil tactics if any;
+//
+// 2. avoid emitting duplicate tactics as part of the same run;
+//
+// 3. rewrite the happy eyeball delays.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func httpsDialerFilterTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	return filterAssignInitialDelays(filterOnlyKeepUniqueTactics(filterOutNilTactics(input)))
+}
+
 // httpsDialerReduceResult returns either an established conn or an error, using [errDNSNoAnswer] in
 // case the list of connections and the list of errors are empty.
 func httpsDialerReduceResult(connv []model.TLSConn, errorv []error) (model.TLSConn, error) {

internal/enginenetx/httpsdialer_test.go

@@ -632,3 +632,231 @@ func TestHTTPSDialerReduceResult(t *testing.T) {
 		}
 	})
 }
+
+// Make sure that (1) we remove nils; (2) we avoid emitting duplicate tactics; (3) we fill
+// the happy-eyeballs delays for each entry we return.
+func TestHTTPSDialerFilterTactics(t *testing.T) {
+	// define the inputs vector including duplicates and nils
+	inputs := []*httpsDialerTactic{
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.com",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "freebsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "dragonflybsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "netbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+	}
+
+	// define the expectations
+	expect := []*httpsDialerTactic{
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   time.Second,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   2 * time.Second,
+			Port:           "443",
+			SNI:            "x.com",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   4 * time.Second,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   8 * time.Second,
+			Port:           "443",
+			SNI:            "freebsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   16 * time.Second,
+			Port:           "443",
+			SNI:            "dragonflybsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   24 * time.Second,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   32 * time.Second,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   40 * time.Second,
+			Port:           "443",
+			SNI:            "netbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+	}
+
+	// run the algorithm
+	var results []*httpsDialerTactic
+	for tx := range httpsDialerFilterTactics(streamTacticsFromSlice(inputs)) {
+		results = append(results, tx)
+	}
+
+	// compare the results
+	if diff := cmp.Diff(expect, results); diff != "" {
+		t.Fatal(diff)
+	}
+}

internal/enginenetx/mix.go

@@ -0,0 +1,87 @@
+package enginenetx
+
+import "sync"
+
+// mixSequentially mixes entries from primary followed by entries from fallback.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func mixSequentially(primary, fallback <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		for tx := range primary {
+			output <- tx
+		}
+		for tx := range fallback {
+			output <- tx
+		}
+	}()
+	return output
+}
+
+// mixDeterministicThenRandomConfig contains config for [mixDeterministicThenRandom].
+type mixDeterministicThenRandomConfig struct {
+	// C is the channel to mix from.
+	C <-chan *httpsDialerTactic
+
+	// N is the number of entries to read from at the
+	// beginning before starting random mixing.
+	N int
+}
+
+// mixDeterministicThenRandom reads the first N entries from primary, if any, then the first N
+// entries from fallback, if any, and then randomly mixes the entries.
+func mixDeterministicThenRandom(primary, fallback *mixDeterministicThenRandomConfig) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		mixTryEmitN(primary.C, primary.N, output)
+		mixTryEmitN(fallback.C, fallback.N, output)
+		for tx := range mixRandomly(primary.C, fallback.C) {
+			output <- tx
+		}
+	}()
+	return output
+}
+
+func mixTryEmitN(input <-chan *httpsDialerTactic, numToRead int, output chan<- *httpsDialerTactic) {
+	for idx := 0; idx < numToRead; idx++ {
+		tactic, good := <-input
+		if !good {
+			return
+		}
+		output <- tactic
+	}
+}
+
+func mixRandomly(left, right <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		// read from left
+		waitg := &sync.WaitGroup{}
+		waitg.Add(1)
+		go func() {
+			defer waitg.Done()
+			for tx := range left {
+				output <- tx
+			}
+		}()
+
+		// read from right
+		waitg.Add(1)
+		go func() {
+			defer waitg.Done()
+			for tx := range right {
+				output <- tx
+			}
+		}()
+
+		// close when done
+		go func() {
+			waitg.Wait()
+			close(output)
+		}()
+	}()
+	return output
+}

internal/enginenetx/mix_test.go

@@ -0,0 +1,227 @@
+package enginenetx
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestMixSequentially(t *testing.T) {
+	primary := []*httpsDialerTactic{}
+	fallback := []*httpsDialerTactic{}
+
+	ff := &testingx.FakeFiller{}
+	ff.Fill(&primary)
+	ff.Fill(&fallback)
+
+	expect := append([]*httpsDialerTactic{}, primary...)
+	expect = append(expect, fallback...)
+
+	var output []*httpsDialerTactic
+	for tx := range mixSequentially(streamTacticsFromSlice(primary), streamTacticsFromSlice(fallback)) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestMixDeterministicThenRandom(t *testing.T) {
+	// define primary data source
+	primary := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a3.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a4.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a5.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a6.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a7.com",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	// define fallback data source
+	fallback := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b3.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b4.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b5.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b6.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b7.com",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	// define the expectations for the beginning of the result
+	expectBeginning := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b3.com",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	// remix
+	outch := mixDeterministicThenRandom(
+		&mixDeterministicThenRandomConfig{
+			C: streamTacticsFromSlice(primary),
+			N: 2,
+		},
+		&mixDeterministicThenRandomConfig{
+			C: streamTacticsFromSlice(fallback),
+			N: 3,
+		},
+	)
+	var output []*httpsDialerTactic
+	for tx := range outch {
+		output = append(output, tx)
+	}
+
+	// make sure we have the expected number of entries
+	if len(output) != 14 {
+		t.Fatal("we need 14 entries")
+	}
+	if diff := cmp.Diff(expectBeginning, output[:5]); diff != "" {
+		t.Fatal(diff)
+	}
+
+	// make sure each entry is represented
+	const (
+		inprimary = 1 << 0
+		infallback
+		inoutput
+	)
+	mapping := make(map[string]int)
+	for _, entry := range primary {
+		mapping[entry.tacticSummaryKey()] |= inprimary
+	}
+	for _, entry := range fallback {
+		mapping[entry.tacticSummaryKey()] |= infallback
+	}
+	for _, entry := range output {
+		mapping[entry.tacticSummaryKey()] |= inoutput
+	}
+	for entry, flags := range mapping {
+		if flags != (inprimary|inoutput) && flags != (infallback|inoutput) {
+			t.Fatal("unexpected flags", flags, "for entry", entry)
+		}
+	}
+}
+
+func TestMixTryEmitNWithClosedChannel(t *testing.T) {
+	// create an already closed channel
+	inputch := make(chan *httpsDialerTactic)
+	close(inputch)
+
+	// create channel for collecting the results
+	outputch := make(chan *httpsDialerTactic)
+
+	go func() {
+		// Implementation note: mixTryEmitN does not close the channel
+		// when done, therefore we need to close it ourselves.
+		mixTryEmitN(inputch, 10, outputch)
+		close(outputch)
+	}()
+
+	// read the output channel
+	var output []*httpsDialerTactic
+	for tx := range outputch {
+		output = append(output, tx)
+	}
+
+	// make sure we didn't read anything
+	if len(output) != 0 {
+		t.Fatal("expected zero entries")
+	}
+}

internal/enginenetx/network.go

@@ -93,7 +93,8 @@ func NewNetwork(
 	netx := &netxlite.Netx{}
 	dialer := netx.NewDialerWithResolver(logger, resolver)
 
-	// Create manager for keeping track of statistics
+	// Create manager for keeping track of statistics. This implies creating a background
+	// goroutine that we'll need to close when we're done.
 	const trimInterval = 30 * time.Second
 	stats := newStatsManager(kvStore, logger, trimInterval)
 
@@ -118,15 +119,8 @@ func NewNetwork(
 	// the proxy, otherwise it means that we're using the ooni/oohttp library
 	// to dial for proxies, which has some restrictions.
 	//
-	// In particular, the returned transport uses dialer for dialing with
-	// cleartext proxies (e.g., socks5 and http) and httpsDialer for dialing
-	// with encrypted proxies (e.g., https). After this has happened,
-	// the code currently falls back to using the standard library's tls
-	// client code for establishing TLS connections over the proxy. The main
-	// implication here is that we're not using our custom mozilla CA for
-	// validating TLS certificates, rather we're using the system's cert store.
-	//
-	// Fixing this issue is TODO(https://github.com/ooni/probe/issues/2536).
+	// - this code does not work as intended when using netem and proxies
+	// as documented by TODO(https://github.com/ooni/probe/issues/2536).
 	txp := netxlite.NewHTTPTransportWithOptions(
 		logger, dialer, httpsDialer,
 		netxlite.HTTPTransportOptionDisableCompression(false),

internal/enginenetx/statsmanager.go

@@ -137,6 +137,8 @@ func statsDefensivelySortTacticsByDescendingSuccessRateWithAcceptPredicate(
 	input []*statsTactic, acceptfunc func(*statsTactic) bool) []*statsTactic {
 	// first let's create a working list such that we don't modify
 	// the input in place thus avoiding any data race
+	//
+	// make sure we explicitly filter out malformed entries
 	work := []*statsTactic{}
 	for _, t := range input {
 		if t != nil && t.Tactic != nil {
@@ -193,8 +195,8 @@ func (st *statsTactic) Clone() *statsTactic {
 	// a pointer to a location which is typically immutable, so it's perfectly
 	// fine to copy the LastUpdate field by assignment.
 	//
-	// here we're using a bunch of robustness aware mechanisms to clone
-	// considering that the struct may be edited by the user
+	// here we're using safe functions to clone the original struct considering
+	// that a user can edit the content on disk freely introducing nulls.
 	return &statsTactic{
 		CountStarted:               st.CountStarted,
 		CountTCPConnectError:       st.CountTCPConnectError,

internal/enginenetx/statspolicy.go

@@ -30,51 +30,17 @@ var _ httpsDialerPolicy = &statsPolicy{}
 
 // LookupTactics implements HTTPSDialerPolicy.
 func (p *statsPolicy) LookupTactics(ctx context.Context, domain string, port string) <-chan *httpsDialerTactic {
-	out := make(chan *httpsDialerTactic)
-
-	go func() {
-		defer close(out) // make sure the parent knows when we're done
-		index := 0
-
-		// useful to make sure we don't emit two equal policy in a single run
-		uniq := make(map[string]int)
-
-		// function that emits a given tactic unless we already emitted it
-		maybeEmitTactic := func(t *httpsDialerTactic) {
-			// as a safety mechanism let's gracefully handle the
-			// case in which the tactic is nil
-			if t == nil {
-				return
-			}
-
-			// handle the case in which we already emitted a policy
-			key := t.tacticSummaryKey()
-			if uniq[key] > 0 {
-				return
-			}
-			uniq[key]++
-
-			// 🚀!!!
-			t.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- t
-		}
-
+	// avoid emitting nil tactics and duplicate tactics
+	return filterOnlyKeepUniqueTactics(filterOutNilTactics(mixSequentially(
 		// give priority to what we know from stats
-		for _, t := range statsPolicyPostProcessTactics(p.Stats.LookupTactics(domain, port)) {
-			maybeEmitTactic(t)
-		}
+		streamTacticsFromSlice(statsPolicyFilterStatsTactics(p.Stats.LookupTactics(domain, port))),
 
 		// fallback to the secondary policy
-		for t := range p.Fallback.LookupTactics(ctx, domain, port) {
-			maybeEmitTactic(t)
-		}
-	}()
-
-	return out
+		p.Fallback.LookupTactics(ctx, domain, port),
+	)))
 }
 
-func statsPolicyPostProcessTactics(tactics []*statsTactic, good bool) (out []*httpsDialerTactic) {
+func statsPolicyFilterStatsTactics(tactics []*statsTactic, good bool) (out []*httpsDialerTactic) {
 	// when good is false, it means p.Stats.LookupTactics failed
 	if !good {
 		return

internal/enginenetx/statspolicy_test.go

@@ -169,21 +169,19 @@ func TestStatsPolicyWorkingAsIntended(t *testing.T) {
 
 		// compute the list of results we expect to see from the stats data
 		var expect []*httpsDialerTactic
-		idx := 0
 		for _, entry := range expectTacticsStats {
 			if entry.CountSuccess <= 0 || entry.Tactic == nil {
 				continue // we SHOULD NOT include entries that systematically failed
 			}
 			t := entry.Tactic.Clone()
-			t.InitialDelay = happyEyeballsDelay(idx)
+			t.InitialDelay = 0
 			expect = append(expect, t)
-			idx++
 		}
 
 		// extend the expected list to include DNS results
 		expect = append(expect, &httpsDialerTactic{
 			Address:        bridgeAddress,
-			InitialDelay:   2 * time.Second,
+			InitialDelay:   0,
 			Port:           "443",
 			SNI:            "api.ooni.io",
 			VerifyHostname: "api.ooni.io",
@@ -234,21 +232,19 @@ func TestStatsPolicyWorkingAsIntended(t *testing.T) {
 
 		// compute the list of results we expect to see from the stats data
 		var expect []*httpsDialerTactic
-		idx := 0
 		for _, entry := range expectTacticsStats {
 			if entry.CountSuccess <= 0 || entry.Tactic == nil {
 				continue // we SHOULD NOT include entries that systematically failed
 			}
 			t := entry.Tactic.Clone()
-			t.InitialDelay = happyEyeballsDelay(idx)
+			t.InitialDelay = 0
 			expect = append(expect, t)
-			idx++
 		}
 
 		// extend the expected list to include DNS results
 		expect = append(expect, &httpsDialerTactic{
 			Address:        bridgeAddress,
-			InitialDelay:   2 * time.Second,
+			InitialDelay:   0,
 			Port:           "443",
 			SNI:            "api.ooni.io",
 			VerifyHostname: "api.ooni.io",
@@ -290,15 +286,13 @@ func TestStatsPolicyWorkingAsIntended(t *testing.T) {
 
 		// compute the list of results we expect to see from the stats data
 		var expect []*httpsDialerTactic
-		idx := 0
 		for _, entry := range expectTacticsStats {
 			if entry.CountSuccess <= 0 || entry.Tactic == nil {
 				continue // we SHOULD NOT include entries that systematically failed
 			}
 			t := entry.Tactic.Clone()
-			t.InitialDelay = happyEyeballsDelay(idx)
+			t.InitialDelay = 0
 			expect = append(expect, t)
-			idx++
 		}
 
 		// perform the actual comparison
@@ -319,9 +313,9 @@ func (p *mocksPolicy) LookupTactics(ctx context.Context, domain string, port str
 	return p.MockLookupTactics(ctx, domain, port)
 }
 
-func TestStatsPolicyPostProcessTactics(t *testing.T) {
+func TestStatsPolicyFilterStatsTactics(t *testing.T) {
 	t.Run("we do nothing when good is false", func(t *testing.T) {
-		tactics := statsPolicyPostProcessTactics(nil, false)
+		tactics := statsPolicyFilterStatsTactics(nil, false)
 		if len(tactics) != 0 {
 			t.Fatal("expected zero-lenght return value")
 		}
@@ -390,7 +384,7 @@ func TestStatsPolicyPostProcessTactics(t *testing.T) {
 			},
 		}
 
-		got := statsPolicyPostProcessTactics(input, true)
+		got := statsPolicyFilterStatsTactics(input, true)
 
 		if len(got) != 1 {
 			t.Fatal("expected just one element")

internal/enginenetx/stream.go

@@ -0,0 +1,16 @@
+package enginenetx
+
+// streamTacticsFromSlice streams tactics from a given slice.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func streamTacticsFromSlice(input []*httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		for _, tx := range input {
+			output <- tx
+		}
+	}()
+	return output
+}

internal/enginenetx/stream_test.go

@@ -0,0 +1,23 @@
+package enginenetx
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestStreamTacticsFromSlice(t *testing.T) {
+	input := []*httpsDialerTactic{}
+	ff := &testingx.FakeFiller{}
+	ff.Fill(&input)
+
+	var output []*httpsDialerTactic
+	for tx := range streamTacticsFromSlice(input) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(input, output); diff != "" {
+		t.Fatal(diff)
+	}
+}

internal/enginenetx/userpolicy.go

@@ -104,7 +104,7 @@ func (ldp *userPolicy) LookupTactics(
 		return ldp.Fallback.LookupTactics(ctx, domain, port)
 	}
 
-	// emit the resuults, which may possibly be empty
+	// emit the results, which may possibly be empty
 	out := make(chan *httpsDialerTactic)
 	go func() {
 		defer close(out) // let the caller know we're done

internal/experiment/webconnectivitylte/analysisclassic.go

@@ -25,7 +25,7 @@ func analysisEngineClassic(tk *TestKeys, logger model.Logger) {
 	tk.analysisClassic(model.GeoIPASNLookupperFunc(geoipx.LookupASN), logger)
 }
 
-func (tk *TestKeys) analysisClassic(lookupper model.GeoIPASNLookupper, logger model.Logger) {
+func (tk *TestKeys) analysisClassic(lookupper model.GeoIPASNLookupper, _ model.Logger) {
 	// Since we run after all tasks have completed (or so we assume) we're
 	// not going to use any form of locking here.
 

internal/experiment/webconnectivitylte/cleartextflow.go

@@ -282,6 +282,9 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a
 	}
 	if err == nil && httpRedirectIsRedirect(resp) {
 		err = httpValidateRedirect(resp)
+		if err == nil && t.FollowRedirects && !t.NumRedirects.CanFollowOneMoreRedirect() {
+			err = ErrTooManyRedirects
+		}
 	}
 
 	finished := trace.TimeSince(trace.ZeroTime())
@@ -319,10 +322,7 @@ func (t *CleartextFlow) httpTransaction(ctx context.Context, network, address, a
 
 // maybeFollowRedirects follows redirects if configured and needed
 func (t *CleartextFlow) maybeFollowRedirects(ctx context.Context, resp *http.Response) {
-	if !t.FollowRedirects || !t.NumRedirects.CanFollowOneMoreRedirect() {
-		return // not configured or too many redirects
-	}
-	if httpRedirectIsRedirect(resp) {
+	if t.FollowRedirects && httpRedirectIsRedirect(resp) {
 		location, err := resp.Location()
 		if err != nil {
 			return // broken response from server

internal/experiment/webconnectivitylte/measurer.go

@@ -121,7 +121,7 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 		Domain:                  URL.Hostname(),
 		IDGenerator:             NewIDGenerator(),
 		Logger:                  sess.Logger(),
-		NumRedirects:            NewNumRedirects(5),
+		NumRedirects:            NewNumRedirects(10),
 		TestKeys:                tk,
 		URL:                     URL,
 		ZeroTime:                measurement.MeasurementStartTimeSaved,

internal/experiment/webconnectivitylte/redirects.go

@@ -19,5 +19,5 @@ func NewNumRedirects(n int64) *NumRedirects {
 // CanFollowOneMoreRedirect returns true if we are
 // allowed to follow one more redirect.
 func (nr *NumRedirects) CanFollowOneMoreRedirect() bool {
-	return nr.count.Add(-1) > 0
+	return nr.count.Add(-1) >= 0
 }

internal/experiment/webconnectivitylte/redirects_test.go

@@ -0,0 +1,16 @@
+package webconnectivitylte
+
+import "testing"
+
+func TestNumRedirects(t *testing.T) {
+	const count = 10
+	nr := NewNumRedirects(count)
+	for idx := 0; idx < count; idx++ {
+		if !nr.CanFollowOneMoreRedirect() {
+			t.Fatal("got false with idx=", idx)
+		}
+	}
+	if nr.CanFollowOneMoreRedirect() {
+		t.Fatal("got true after the loop")
+	}
+}

internal/experiment/webconnectivitylte/secureflow.go

@@ -9,6 +9,7 @@ package webconnectivitylte
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"io"
 	"net"
 	"net/http"
@@ -305,6 +306,9 @@ func (t *SecureFlow) newHTTPRequest(ctx context.Context) (*http.Request, error)
 	return httpReq, nil
 }
 
+// ErrTooManyRedirects indicates we have seen too many HTTP redirects.
+var ErrTooManyRedirects = errors.New("stopped after too many redirects")
+
 // httpTransaction runs the HTTP transaction and saves the results.
 func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn string,
 	txp model.HTTPTransport, req *http.Request, trace *measurexlite.Trace) (*http.Response, []byte, error) {
@@ -337,6 +341,9 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn
 	}
 	if err == nil && httpRedirectIsRedirect(resp) {
 		err = httpValidateRedirect(resp)
+		if err == nil && t.FollowRedirects && !t.NumRedirects.CanFollowOneMoreRedirect() {
+			err = ErrTooManyRedirects
+		}
 	}
 
 	finished := trace.TimeSince(trace.ZeroTime())
@@ -374,10 +381,7 @@ func (t *SecureFlow) httpTransaction(ctx context.Context, network, address, alpn
 
 // maybeFollowRedirects follows redirects if configured and needed
 func (t *SecureFlow) maybeFollowRedirects(ctx context.Context, resp *http.Response) {
-	if !t.FollowRedirects || !t.NumRedirects.CanFollowOneMoreRedirect() {
-		return // not configured or too many redirects
-	}
-	if httpRedirectIsRedirect(resp) {
+	if t.FollowRedirects && httpRedirectIsRedirect(resp) {
 		location, err := resp.Location()
 		if err != nil {
 			return // broken response from server

internal/measurexlite/conn.go

@@ -39,11 +39,29 @@ type connTrace struct {
 
 var _ net.Conn = &connTrace{}
 
+type remoteAddrProvider interface {
+	RemoteAddr() net.Addr
+}
+
+func safeRemoteAddrNetwork(rap remoteAddrProvider) (result string) {
+	if addr := rap.RemoteAddr(); addr != nil {
+		result = addr.Network()
+	}
+	return result
+}
+
+func safeRemoteAddrString(rap remoteAddrProvider) (result string) {
+	if addr := rap.RemoteAddr(); addr != nil {
+		result = addr.String()
+	}
+	return result
+}
+
 // Read implements net.Conn.Read and saves network events.
 func (c *connTrace) Read(b []byte) (int, error) {
 	// collect preliminary stats when the connection is surely active
-	network := c.RemoteAddr().Network()
-	addr := c.RemoteAddr().String()
+	network := safeRemoteAddrNetwork(c)
+	addr := safeRemoteAddrString(c)
 	started := c.tx.TimeSince(c.tx.ZeroTime())
 
 	// perform the underlying network operation
@@ -99,8 +117,8 @@ func (tx *Trace) CloneBytesReceivedMap() (out map[string]int64) {
 
 // Write implements net.Conn.Write and saves network events.
 func (c *connTrace) Write(b []byte) (int, error) {
-	network := c.RemoteAddr().Network()
-	addr := c.RemoteAddr().String()
+	network := safeRemoteAddrNetwork(c)
+	addr := safeRemoteAddrString(c)
 	started := c.tx.TimeSince(c.tx.ZeroTime())
 
 	count, err := c.Conn.Write(b)

internal/measurexlite/conn_test.go

@@ -12,6 +12,43 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/testingx"
 )
 
+func TestRemoteAddrProvider(t *testing.T) {
+	t.Run("for nil address", func(t *testing.T) {
+		conn := &mocks.Conn{
+			MockRemoteAddr: func() net.Addr {
+				return nil
+			},
+		}
+		if safeRemoteAddrNetwork(conn) != "" {
+			t.Fatal("expected empty network")
+		}
+		if safeRemoteAddrString(conn) != "" {
+			t.Fatal("expected empty string")
+		}
+	})
+
+	t.Run("for common case", func(t *testing.T) {
+		conn := &mocks.Conn{
+			MockRemoteAddr: func() net.Addr {
+				return &mocks.Addr{
+					MockString: func() string {
+						return "1.1.1.1:443"
+					},
+					MockNetwork: func() string {
+						return "tcp"
+					},
+				}
+			},
+		}
+		if safeRemoteAddrNetwork(conn) != "tcp" {
+			t.Fatal("unexpected network")
+		}
+		if safeRemoteAddrString(conn) != "1.1.1.1:443" {
+			t.Fatal("unexpected string")
+		}
+	})
+}
+
 func TestMaybeClose(t *testing.T) {
 	t.Run("with nil conn", func(t *testing.T) {
 		var conn net.Conn = nil

internal/netemx/httpbin.go

@@ -1,8 +1,11 @@
 package netemx
 
 import (
+	"fmt"
 	"net"
 	"net/http"
+	"strconv"
+	"strings"
 
 	"github.com/ooni/netem"
 )
@@ -29,7 +32,7 @@ func HTTPBinHandlerFactory() HTTPHandlerFactory {
 // Any other request URL causes a 404 respose.
 func HTTPBinHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Add("Date", "Thu, 24 Aug 2023 14:35:29 GMT")
+		w.Header().Set("Date", "Thu, 24 Aug 2023 14:35:29 GMT")
 
 		// missing address => 500
 		address, _, err := net.SplitHostPort(r.RemoteAddr)
@@ -44,6 +47,20 @@ func HTTPBinHandler() http.Handler {
 		secureRedirect := r.URL.Path == "/broken-redirect-https"
 
 		switch {
+		// redirect with count
+		case strings.HasPrefix(r.URL.Path, "/redirect/"):
+			count, err := strconv.Atoi(strings.TrimPrefix(r.URL.Path, "/redirect/"))
+			if err != nil {
+				w.WriteHeader(http.StatusBadRequest)
+				return
+			}
+			if count <= 0 {
+				w.WriteHeader(http.StatusOK)
+				return
+			}
+			w.Header().Set("Location", fmt.Sprintf("/redirect/%d", count-1))
+			w.WriteHeader(http.StatusFound)
+
 		// broken HTTP redirect for clients
 		case cleartextRedirect && client:
 			w.Header().Set("Location", "http://")

internal/netemx/httpbin_test.go

@@ -25,6 +25,88 @@ func TestHTTPBinHandler(t *testing.T) {
 		}
 	})
 
+	t.Run("/redirect/{n} with invalid number", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "https://", Path: "/redirect/antani"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusBadRequest {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+	})
+
+	t.Run("/redirect/0", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "https://", Path: "/redirect/0"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusOK {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+	})
+
+	t.Run("/redirect/1", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "https://", Path: "/redirect/1"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+		location, err := result.Location()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if location.Path != "/redirect/0" {
+			t.Fatal("unexpected location.Path", location.Path)
+		}
+	})
+
+	t.Run("/redirect/2", func(t *testing.T) {
+		req := &http.Request{
+			URL:        &url.URL{Scheme: "https://", Path: "/redirect/2"},
+			Body:       http.NoBody,
+			Close:      false,
+			Host:       "httpbin.com",
+			RemoteAddr: net.JoinHostPort("8.8.8.8", "54321"),
+		}
+		rr := httptest.NewRecorder()
+		handler := HTTPBinHandler()
+		handler.ServeHTTP(rr, req)
+		result := rr.Result()
+		if result.StatusCode != http.StatusFound {
+			t.Fatal("unexpected status code", result.StatusCode)
+		}
+		location, err := result.Location()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if location.Path != "/redirect/1" {
+			t.Fatal("unexpected location.Path", location.Path)
+		}
+	})
+
 	t.Run("/broken-redirect-http with client address", func(t *testing.T) {
 		req := &http.Request{
 			URL:        &url.URL{Scheme: "http://", Path: "/broken-redirect-http"},

internal/netemx/ooapi.go

@@ -35,7 +35,7 @@ func (p *OOAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (p *OOAPIHandler) getApiV1TestHelpers(w http.ResponseWriter, r *http.Request) {
+func (p *OOAPIHandler) getApiV1TestHelpers(w http.ResponseWriter, _ *http.Request) {
 	resp := map[string][]model.OOAPIService{
 		"web-connectivity": {
 			{

internal/netemx/oohelperd_test.go

@@ -73,13 +73,7 @@ func TestOOHelperDHandler(t *testing.T) {
 					Failure:    nil,
 				},
 			},
-			QUICHandshake: map[string]model.THTLSHandshakeResult{
-				"93.184.216.34:443": {
-					ServerName: "www.example.com",
-					Status:     true,
-					Failure:    nil,
-				},
-			},
+			QUICHandshake: map[string]model.THTLSHandshakeResult{}, // since https://github.com/ooni/probe-cli/pull/1549
 			HTTPRequest: model.THHTTPRequestResult{
 				BodyLength:           1533,
 				DiscoveredH3Endpoint: "www.example.com:443",
@@ -93,19 +87,7 @@ func TestOOHelperDHandler(t *testing.T) {
 				},
 				StatusCode: 200,
 			},
-			HTTP3Request: &model.THHTTPRequestResult{
-				BodyLength:           1533,
-				DiscoveredH3Endpoint: "",
-				Failure:              nil,
-				Title:                "Default Web Page",
-				Headers: map[string]string{
-					"Alt-Svc":        `h3=":443"`,
-					"Content-Length": "1533",
-					"Content-Type":   "text/html; charset=utf-8",
-					"Date":           "Thu, 24 Aug 2023 14:35:29 GMT",
-				},
-				StatusCode: 200,
-			},
+			HTTP3Request: nil, // since https://github.com/ooni/probe-cli/pull/1549
 			DNS: model.THDNSResult{
 				Failure: nil,
 				Addrs:   []string{"93.184.216.34"},

internal/oohelperd/handler.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/cookiejar"
+	"os"
 	"strings"
 	"sync/atomic"
 	"time"
@@ -31,6 +32,9 @@ const maxAcceptableBodySize = 1 << 24
 //
 // The zero value is invalid; construct using [NewHandler].
 type Handler struct {
+	// EnableQUIC OPTIONALLY enables QUIC.
+	EnableQUIC bool
+
 	// baseLogger is the MANDATORY logger to use.
 	baseLogger model.Logger
 
@@ -69,9 +73,13 @@ type Handler struct {
 
 var _ http.Handler = &Handler{}
 
+// enableQUIC allows to control whether to enable QUIC by using environment variables.
+var enableQUIC = (os.Getenv("OOHELPERD_ENABLE_QUIC") == "1")
+
 // NewHandler constructs the [handler].
 func NewHandler(logger model.Logger, netx *netxlite.Netx) *Handler {
 	return &Handler{
+		EnableQUIC:        enableQUIC,
 		baseLogger:        logger,
 		countRequests:     &atomic.Int64{},
 		indexer:           &atomic.Int64{},

internal/oohelperd/handler_test.go

@@ -262,3 +262,10 @@ func TestHandlerWorkingAsIntended(t *testing.T) {
 		})
 	}
 }
+
+func TestNewHandlerEnableQUIC(t *testing.T) {
+	handler := NewHandler(log.Log, &netxlite.Netx{Underlying: nil})
+	if handler.EnableQUIC != false {
+		t.Fatal("expected to see false here (is the the environment variable OOHELPERD_ENABLE_QUIC set?!)")
+	}
+}

internal/oohelperd/measure.go

@@ -125,7 +125,7 @@ func measure(ctx context.Context, config *Handler, creq *ctrlRequest) (*ctrlResp
 	// In the v3.17.x and possibly v3.18.x release cycles, QUIC is disabled by
 	// default but clients that know QUIC can enable it. We will eventually remove
 	// this flag and enable QUIC measurements for all clients.
-	if creq.XQUICEnabled && cresp.HTTPRequest.DiscoveredH3Endpoint != "" {
+	if config.EnableQUIC && creq.XQUICEnabled && cresp.HTTPRequest.DiscoveredH3Endpoint != "" {
 		// quicconnect: start over all the endpoints
 		for _, endpoint := range endpoints {
 			wg.Add(1)

internal/oohelperd/qa_test.go

@@ -0,0 +1,137 @@
+package oohelperd_test
+
+import (
+	"bytes"
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/must"
+	"github.com/ooni/probe-cli/v3/internal/netemx"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
+	"github.com/ooni/probe-cli/v3/internal/oohelperd"
+	"github.com/ooni/probe-cli/v3/internal/optional"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
+)
+
+// TestQAEnableDisableQUIC ensures that we can enable and disable QUIC.
+func TestQAEnableDisableQUIC(t *testing.T) {
+	// testcase is a test case for this function
+	type testcase struct {
+		name       string
+		enableQUIC optional.Value[bool]
+	}
+
+	cases := []testcase{{
+		name:       "with the default settings",
+		enableQUIC: optional.None[bool](),
+	}, {
+		name:       "with explicit false",
+		enableQUIC: optional.Some(false),
+	}, {
+		name:       "with explicit true",
+		enableQUIC: optional.Some(true),
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			// create a new testing scenario
+			env := netemx.MustNewScenario(netemx.InternetScenario)
+			defer env.Close()
+
+			// create a new handler
+			handler := oohelperd.NewHandler(
+				log.Log,
+				&netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: env.ClientStack}},
+			)
+
+			// optionally and conditionally enable QUIC
+			if !tc.enableQUIC.IsNone() {
+				handler.EnableQUIC = tc.enableQUIC.Unwrap()
+			}
+
+			// create request body
+			reqbody := &model.THRequest{
+				HTTPRequest: "https://www.example.com/",
+				HTTPRequestHeaders: map[string][]string{
+					"Accept-Language": {model.HTTPHeaderAcceptLanguage},
+					"Accept":          {model.HTTPHeaderAccept},
+					"User-Agent":      {model.HTTPHeaderUserAgent},
+				},
+				TCPConnect:   []string{netemx.AddressWwwExampleCom},
+				XQUICEnabled: true,
+			}
+
+			// create request
+			req := runtimex.Try1(http.NewRequest(
+				"POST",
+				"http://127.0.0.1:8080/",
+				bytes.NewReader(must.MarshalJSON(reqbody)),
+			))
+
+			// create response recorder
+			resprec := httptest.NewRecorder()
+
+			// invoke the handler
+			handler.ServeHTTP(resprec, req)
+
+			// get the response
+			resp := resprec.Result()
+			defer resp.Body.Close()
+
+			// make sure the status code indicates success
+			if resp.StatusCode != 200 {
+				t.Fatal("expected 200 Ok")
+			}
+
+			// make sure the content-type is OK
+			if v := resp.Header.Get("Content-Type"); v != "application/json" {
+				t.Fatal("unexpected content-type", v)
+			}
+
+			// read the response body
+			respbody := runtimex.Try1(netxlite.ReadAllContext(context.Background(), resp.Body))
+
+			// parse the response body
+			var jsonresp model.THResponse
+			must.UnmarshalJSON(respbody, &jsonresp)
+
+			// check whether we have an HTTP3 response
+			switch {
+			case !tc.enableQUIC.IsNone() && tc.enableQUIC.Unwrap() && jsonresp.HTTP3Request != nil:
+				// all good: we have QUIC enabled and we get an HTTP/3 response
+
+			case (tc.enableQUIC.IsNone() || tc.enableQUIC.Unwrap() == false) && jsonresp.HTTP3Request == nil:
+				// all good: either default behavior or QUIC not enabled and not HTTP/3 response
+
+			default:
+				t.Fatalf(
+					"tc.enableQUIC.IsNone() = %v, tc.enableQUIC.UnwrapOr(false) = %v, jsonresp.HTTP3Request = %v",
+					tc.enableQUIC.IsNone(),
+					tc.enableQUIC.UnwrapOr(false),
+					jsonresp.HTTP3Request,
+				)
+			}
+
+			// check whether we have QUIC handshakes
+			switch {
+			case !tc.enableQUIC.IsNone() && tc.enableQUIC.Unwrap() && len(jsonresp.QUICHandshake) > 0:
+				// all good: we have QUIC enabled and we get QUIC handshakes
+
+			case (tc.enableQUIC.IsNone() || tc.enableQUIC.Unwrap() == false) && len(jsonresp.QUICHandshake) <= 0:
+				// all good: either default behavior or QUIC not enabled and no QUIC handshakes
+
+			default:
+				t.Fatalf(
+					"tc.enableQUIC.IsNone() = %v, tc.enableQUIC.UnwrapOr(false) = %v, jsonresp.QUICHandshake = %v",
+					tc.enableQUIC.IsNone(),
+					tc.enableQUIC.UnwrapOr(false),
+					jsonresp.QUICHandshake,
+				)
+			}
+		})
+	}
+}

internal/webconnectivityqa/redirect.go

@@ -391,3 +391,47 @@ func redirectWithBrokenLocationForHTTPS() *TestCase {
 		},
 	}
 }
+
+// redirectWithMoreThanTenRedirectsAndHTTP is a scenario where the redirect
+// consists of more than ten redirects for http:// URLs.
+func redirectWithMoreThanTenRedirectsAndHTTP() *TestCase {
+	return &TestCase{
+		Name:      "redirectWithMoreThanTenRedirectsAndHTTP",
+		Flags:     TestCaseFlagNoV04,
+		Input:     "http://httpbin.com/redirect/15",
+		LongTest:  true,
+		ExpectErr: false,
+		ExpectTestKeys: &TestKeys{
+			DNSExperimentFailure:  nil,
+			DNSConsistency:        "consistent",
+			HTTPExperimentFailure: `unknown_failure: stopped after too many redirects`,
+			XStatus:               0,
+			XDNSFlags:             0,
+			XBlockingFlags:        0,
+			Accessible:            false,
+			Blocking:              false,
+		},
+	}
+}
+
+// redirectWithMoreThanTenRedirectsAndHTTPS is a scenario where the redirect
+// consists of more than ten redirects for https:// URLs.
+func redirectWithMoreThanTenRedirectsAndHTTPS() *TestCase {
+	return &TestCase{
+		Name:      "redirectWithMoreThanTenRedirectsAndHTTPS",
+		Flags:     TestCaseFlagNoV04,
+		Input:     "https://httpbin.com/redirect/15",
+		LongTest:  true,
+		ExpectErr: false,
+		ExpectTestKeys: &TestKeys{
+			DNSExperimentFailure:  nil,
+			DNSConsistency:        "consistent",
+			HTTPExperimentFailure: `unknown_failure: stopped after too many redirects`,
+			XStatus:               0,
+			XDNSFlags:             0,
+			XBlockingFlags:        0,
+			Accessible:            false,
+			Blocking:              false,
+		},
+	}
+}

internal/webconnectivityqa/testcase.go

@@ -90,6 +90,8 @@ func AllTestCases() []*TestCase {
 		redirectWithConsistentDNSAndThenEOFForHTTPS(),
 		redirectWithConsistentDNSAndThenTimeoutForHTTP(),
 		redirectWithConsistentDNSAndThenTimeoutForHTTPS(),
+		redirectWithMoreThanTenRedirectsAndHTTP(),
+		redirectWithMoreThanTenRedirectsAndHTTPS(),
 
 		successWithHTTP(),
 		successWithHTTPS(),