Merge 9bf52a9e96 into d6d201d85e

Bumps [golang.org/x/net](https://github.com/golang/net) from 0.22.0 to
0.23.0.
<details>
<summary>Commits</summary>
<ul>
<li><a
href="c48da13158"><code>c48da13</code></a>
http2: fix TestServerContinuationFlood flakes</li>
<li><a
href="762b58d1cf"><code>762b58d</code></a>
http2: fix tipos in comment</li>
<li><a
href="ba872109ef"><code>ba87210</code></a>
http2: close connections when receiving too many headers</li>
<li><a
href="ebc8168ac8"><code>ebc8168</code></a>
all: fix some typos</li>
<li><a
href="3678185f8a"><code>3678185</code></a>
http2: make TestCanonicalHeaderCacheGrowth faster</li>
<li><a
href="448c44f928"><code>448c44f</code></a>
http2: remove clientTester</li>
<li><a
href="c7877ac421"><code>c7877ac</code></a>
http2: convert the remaining clientTester tests to testClientConn</li>
<li><a
href="d8870b0bf2"><code>d8870b0</code></a>
http2: use synthetic time in TestIdleConnTimeout</li>
<li><a
href="d73acffdc9"><code>d73acff</code></a>
http2: only set up deadline when Server.IdleTimeout is positive</li>
<li><a
href="89f602b7bb"><code>89f602b</code></a>
http2: validate client/outgoing trailers</li>
<li>Additional commits viewable in <a
href="https://github.com/golang/net/compare/v0.22.0...v0.23.0">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=golang.org/x/net&package-manager=go_modules&previous-version=0.22.0&new-version=0.23.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/ooni/probe-cli/network/alerts).

</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

cmd/ooniprobe/internal/cli/geoip/geoip.go

@@ -43,7 +43,7 @@ func dogeoip(config dogeoipconfig) error {
 	}
 	defer engine.Close()
 
-	err = engine.MaybeLookupLocation()
+	err = engine.MaybeLookupLocationContext(context.Background())
 	if err != nil {
 		return err
 	}

cmd/ooniprobe/internal/nettests/nettests.go

@@ -231,7 +231,7 @@ func (c *Controller) Run(builder model.ExperimentBuilder, inputs []string) error
 		}
 		// We only save the measurement to disk if we failed to upload the measurement
 		if saveToDisk {
-			if err := exp.SaveMeasurement(measurement, msmt.MeasurementFilePath.String); err != nil {
+			if err := engine.SaveMeasurement(measurement, msmt.MeasurementFilePath.String); err != nil {
 				return errors.Wrap(err, "failed to save measurement on disk")
 			}
 		}

cmd/ooniprobe/internal/nettests/run.go

@@ -66,7 +66,7 @@ func RunGroup(config RunGroupConfig) error {
 	}
 	defer sess.Close()
 
-	err = sess.MaybeLookupLocation()
+	err = sess.MaybeLookupLocationContext(context.Background())
 	if err != nil {
 		log.WithError(err).Error("Failed to lookup the location of the probe")
 		return err
@@ -77,7 +77,7 @@ func RunGroup(config RunGroupConfig) error {
 		log.WithError(err).Error("Failed to create the network row")
 		return err
 	}
-	if err := sess.MaybeLookupBackends(); err != nil {
+	if err := sess.MaybeLookupBackendsContext(context.Background()); err != nil {
 		log.WithError(err).Errorf("Failed to discover OONI backends")
 		return err
 	}

cmd/ooniprobe/internal/ooni/ooni.go

@@ -42,7 +42,7 @@ type ProbeCLI interface {
 // ProbeEngine is an instance of the OONI Probe engine.
 type ProbeEngine interface {
 	Close() error
-	MaybeLookupLocation() error
+	MaybeLookupLocationContext(context.Context) error
 	ProbeASNString() string
 	ProbeCC() string
 	ProbeIP() string

cmd/ooniprobe/internal/oonitest/oonitest.go

@@ -84,7 +84,7 @@ func (eng *FakeProbeEngine) Close() error {
 }
 
 // MaybeLookupLocation implements ProbeEngine.MaybeLookupLocation
-func (eng *FakeProbeEngine) MaybeLookupLocation() error {
+func (eng *FakeProbeEngine) MaybeLookupLocationContext(_ context.Context) error {
 	return eng.FakeMaybeLookupLocation
 }
 

go.mod

@@ -41,7 +41,7 @@ require (
 	gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/goptlib v1.5.0
 	gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/v2 v2.6.1
 	golang.org/x/crypto v0.21.0
-	golang.org/x/net v0.22.0
+	golang.org/x/net v0.23.0
 	golang.org/x/sys v0.18.0
 )
 

go.sum

@@ -652,8 +652,8 @@ golang.org/x/net v0.11.0/go.mod h1:2L/ixqYpgIVXmeoSA/4Lu7BzTG4KIyPIryS4IsOd1oQ=
 golang.org/x/net v0.13.0/go.mod h1:zEVYFnQC7m/vmpQFELhcD1EWkZlX69l4oqgmer6hfKA=
 golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI=
 golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk=
-golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc=
-golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
+golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
+golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

internal/cmd/miniooni/javascript.go

@@ -10,7 +10,7 @@ import (
 )
 
 // registerJavaScript registers the javascript subcommand
-func registerJavaScript(rootCmd *cobra.Command, globalOptions *Options) {
+func registerJavaScript(rootCmd *cobra.Command, _ *Options) {
 	subCmd := &cobra.Command{
 		Use:   "javascript",
 		Short: "Very experimental command to run JavaScript snippets",

internal/cmd/miniooni/main.go

@@ -40,6 +40,8 @@ type Options struct {
 	RepeatEvery         int64
 	ReportFile          string
 	SnowflakeRendezvous string
+	SoftwareName        string
+	SoftwareVersion     string
 	TorArgs             []string
 	TorBinary           string
 	Tunnel              string
@@ -133,6 +135,20 @@ func main() {
 		"rendezvous method for --tunnel=torsf (one of: \"domain_fronting\" and \"amp\")",
 	)
 
+	flags.StringVar(
+		&globalOptions.SoftwareName,
+		"software-name",
+		"miniooni",
+		"Set the name of the application",
+	)
+
+	flags.StringVar(
+		&globalOptions.SoftwareVersion,
+		"software-version",
+		version.Version,
+		"Set the version of the application",
+	)
+
 	flags.StringSliceVar(
 		&globalOptions.TorArgs,
 		"tor-args",
@@ -268,7 +284,7 @@ func registerAllExperiments(rootCmd *cobra.Command, globalOptions *Options) {
 			// nothing
 		}
 
-		if doc := documentationForOptions(name, factory); doc != "" {
+		if doc := documentationForOptions(factory); doc != "" {
 			flags.StringSliceVarP(
 				&globalOptions.ExtraOptions,
 				"option",
@@ -376,7 +392,7 @@ func mainSingleIteration(logger model.Logger, experimentName string, currentOpti
 	runx(ctx, sess, experimentName, annotations, extraOptions, currentOptions)
 }
 
-func documentationForOptions(name string, factory *registry.Factory) string {
+func documentationForOptions(factory *registry.Factory) string {
 	var sb strings.Builder
 	options, err := factory.Options()
 	if err != nil || len(options) < 1 {

internal/cmd/miniooni/main_test.go

@@ -1,12 +1,18 @@
 package main
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/version"
+)
 
 func TestSimple(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skip test in short mode")
 	}
 	MainWithConfiguration("example", &Options{
-		Yes: true,
+		SoftwareName:    "miniooni",
+		SoftwareVersion: version.Version,
+		Yes:             true,
 	})
 }

internal/cmd/miniooni/session.go

@@ -12,12 +12,6 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/legacy/kvstore2dir"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
-	"github.com/ooni/probe-cli/v3/internal/version"
-)
-
-const (
-	softwareName    = "miniooni"
-	softwareVersion = version.Version
 )
 
 // newSessionOrPanic creates and starts a new session or panics on failure
@@ -44,8 +38,8 @@ func newSessionOrPanic(ctx context.Context, currentOptions *Options,
 		Logger:              logger,
 		ProxyURL:            proxyURL,
 		SnowflakeRendezvous: currentOptions.SnowflakeRendezvous,
-		SoftwareName:        softwareName,
-		SoftwareVersion:     softwareVersion,
+		SoftwareName:        currentOptions.SoftwareName,
+		SoftwareVersion:     currentOptions.SoftwareVersion,
 		TorArgs:             currentOptions.TorArgs,
 		TorBinary:           currentOptions.TorBinary,
 		TunnelDir:           tunnelDir,

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/cmd/oonireport/oonireport.go

@@ -15,7 +15,6 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/engine"
 	"github.com/ooni/probe-cli/v3/internal/fsx"
 	"github.com/ooni/probe-cli/v3/internal/model"
-	"github.com/ooni/probe-cli/v3/internal/probeservices"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 	"github.com/ooni/probe-cli/v3/internal/version"
 	"github.com/pborman/getopt/v2"
@@ -43,8 +42,7 @@ const (
 )
 
 var (
-	path    string
-	control bool
+	path string
 )
 
 func fatalIfFalse(cond bool, msg string) {
@@ -89,11 +87,8 @@ func newSession(ctx context.Context) *engine.Session {
 }
 
 // new Submitter creates a probe services client and submitter
-func newSubmitter(sess *engine.Session, ctx context.Context) *probeservices.Submitter {
-	psc, err := sess.NewProbeServicesClient(ctx)
-	runtimex.PanicOnError(err, "error occurred while creating client")
-	submitter := probeservices.NewSubmitter(psc, sess.Logger())
-	return submitter
+func newSubmitter(sess *engine.Session, ctx context.Context) model.Submitter {
+	return runtimex.Try1(sess.NewSubmitter(ctx))
 }
 
 // toMeasurement loads an input string as model.Measurement
@@ -106,7 +101,7 @@ func toMeasurement(s string) *model.Measurement {
 
 // submitAll submits the measurements in input. Returns the count of submitted measurements, both
 // on success and on error, and the error that occurred (nil on success).
-func submitAll(ctx context.Context, lines []string, subm *probeservices.Submitter) (int, error) {
+func submitAll(ctx context.Context, lines []string, subm model.Submitter) (int, error) {
 	submitted := 0
 	for _, line := range lines {
 		mm := toMeasurement(line)

internal/engine/experiment.go

@@ -6,10 +6,8 @@ package engine
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"net/http"
-	"os"
 	"runtime"
 	"time"
 
@@ -185,16 +183,6 @@ func (e *experiment) MeasureWithContext(
 	return
 }
 
-// SaveMeasurement implements Experiment.SaveMeasurement.
-func (e *experiment) SaveMeasurement(measurement *model.Measurement, filePath string) error {
-	return e.saveMeasurement(
-		measurement, filePath, json.Marshal, os.OpenFile,
-		func(fp *os.File, b []byte) (int, error) {
-			return fp.Write(b)
-		},
-	)
-}
-
 // SubmitAndUpdateMeasurementContext implements Experiment.SubmitAndUpdateMeasurementContext.
 func (e *experiment) SubmitAndUpdateMeasurementContext(
 	ctx context.Context, measurement *model.Measurement) error {
@@ -250,7 +238,7 @@ func (e *experiment) OpenReportContext(ctx context.Context) error {
 			e.byteCounter,
 		),
 	}
-	client, err := e.session.NewProbeServicesClient(ctx)
+	client, err := e.session.newProbeServicesClient(ctx)
 	if err != nil {
 		e.session.logger.Debugf("%+v", err)
 		return err
@@ -278,24 +266,3 @@ func (e *experiment) newReportTemplate() model.OOAPIReportTemplate {
 		TestVersion:       e.testVersion,
 	}
 }
-
-func (e *experiment) saveMeasurement(
-	measurement *model.Measurement, filePath string,
-	marshal func(v interface{}) ([]byte, error),
-	openFile func(name string, flag int, perm os.FileMode) (*os.File, error),
-	write func(fp *os.File, b []byte) (n int, err error),
-) error {
-	data, err := marshal(measurement)
-	if err != nil {
-		return err
-	}
-	data = append(data, byte('\n'))
-	filep, err := openFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
-	if err != nil {
-		return err
-	}
-	if _, err := write(filep, data); err != nil {
-		return err
-	}
-	return filep.Close()
-}

internal/engine/experiment_integration_test.go

@@ -3,15 +3,13 @@ package engine
 import (
 	"context"
 	"encoding/json"
-	"errors"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/registry"
 )
@@ -264,18 +262,24 @@ func runexperimentflow(t *testing.T, experiment model.Experiment, input string)
 	measurement.AddAnnotations(map[string]string{
 		"probe-engine-ci": "yes",
 	})
-	data, err := json.Marshal(measurement)
+	savedMeasurement, err := json.Marshal(measurement)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if data == nil {
+	if savedMeasurement == nil {
 		t.Fatal("data is nil")
 	}
+	tempfile, err := os.CreateTemp("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	filename := tempfile.Name()
+	tempfile.Close()
 	err = experiment.SubmitAndUpdateMeasurementContext(ctx, measurement)
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = experiment.SaveMeasurement(measurement, "/tmp/experiment.jsonl")
+	err = SaveMeasurement(measurement, filename)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -289,54 +293,13 @@ func runexperimentflow(t *testing.T, experiment model.Experiment, input string)
 	if sk == nil {
 		t.Fatal("got nil summary keys")
 	}
-}
-
-func TestSaveMeasurementErrors(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skip test in short mode")
-	}
-	sess := newSessionForTesting(t)
-	defer sess.Close()
-	builder, err := sess.NewExperimentBuilder("example")
+	loadedMeasurement, err := os.ReadFile(filename)
 	if err != nil {
 		t.Fatal(err)
 	}
-	exp := builder.NewExperiment().(*experiment)
-	dirname, err := ioutil.TempDir("", "ooniprobe-engine-save-measurement")
-	if err != nil {
-		t.Fatal(err)
-	}
-	filename := filepath.Join(dirname, "report.jsonl")
-	m := new(model.Measurement)
-	err = exp.saveMeasurement(
-		m, filename, func(v interface{}) ([]byte, error) {
-			return nil, errors.New("mocked error")
-		}, os.OpenFile, func(fp *os.File, b []byte) (int, error) {
-			return fp.Write(b)
-		},
-	)
-	if err == nil {
-		t.Fatal("expected an error here")
-	}
-	err = exp.saveMeasurement(
-		m, filename, json.Marshal,
-		func(name string, flag int, perm os.FileMode) (*os.File, error) {
-			return nil, errors.New("mocked error")
-		}, func(fp *os.File, b []byte) (int, error) {
-			return fp.Write(b)
-		},
-	)
-	if err == nil {
-		t.Fatal("expected an error here")
-	}
-	err = exp.saveMeasurement(
-		m, filename, json.Marshal, os.OpenFile,
-		func(fp *os.File, b []byte) (int, error) {
-			return 0, errors.New("mocked error")
-		},
-	)
-	if err == nil {
-		t.Fatal("expected an error here")
+	withFinalNewline := append(savedMeasurement, '\n')
+	if diff := cmp.Diff(withFinalNewline, loadedMeasurement); diff != "" {
+		t.Fatal(diff)
 	}
 }
 

internal/engine/inputloader.go

@@ -27,8 +27,7 @@ var (
 // InputLoaderSession is the session according to an InputLoader. We
 // introduce this abstraction because it helps us with testing.
 type InputLoaderSession interface {
-	CheckIn(ctx context.Context,
-		config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error)
+	CheckIn(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error)
 }
 
 // InputLoaderLogger is the logger according to an InputLoader.
@@ -146,7 +145,7 @@ func (il *InputLoader) loadOptional() ([]model.OOAPIURLInfo, error) {
 }
 
 // loadStrictlyRequired implements the InputStrictlyRequired policy.
-func (il *InputLoader) loadStrictlyRequired(ctx context.Context) ([]model.OOAPIURLInfo, error) {
+func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.OOAPIURLInfo, error) {
 	inputs, err := il.loadLocal()
 	if err != nil || len(inputs) > 0 {
 		return inputs, err
@@ -242,7 +241,7 @@ func staticInputForExperiment(name string) ([]model.OOAPIURLInfo, error) {
 }
 
 // loadOrStaticDefault implements the InputOrStaticDefault policy.
-func (il *InputLoader) loadOrStaticDefault(ctx context.Context) ([]model.OOAPIURLInfo, error) {
+func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.OOAPIURLInfo, error) {
 	inputs, err := il.loadLocal()
 	if err != nil || len(inputs) > 0 {
 		return inputs, err
@@ -328,12 +327,12 @@ func (il *InputLoader) checkIn(
 		return nil, err
 	}
 	// Note: safe to assume that reply is not nil if err is nil
-	if reply.WebConnectivity != nil && len(reply.WebConnectivity.URLs) > 0 {
-		reply.WebConnectivity.URLs = il.preventMistakes(
-			reply.WebConnectivity.URLs, config.WebConnectivity.CategoryCodes,
+	if reply.Tests.WebConnectivity != nil && len(reply.Tests.WebConnectivity.URLs) > 0 {
+		reply.Tests.WebConnectivity.URLs = il.preventMistakes(
+			reply.Tests.WebConnectivity.URLs, config.WebConnectivity.CategoryCodes,
 		)
 	}
-	return reply, nil
+	return &reply.Tests, nil
 }
 
 // preventMistakes makes the code more robust with respect to any possible

internal/engine/inputloader_test.go

@@ -446,7 +446,7 @@ func TestInputLoaderReadfileScannerFailure(t *testing.T) {
 type InputLoaderMockableSession struct {
 	// Output contains the output of CheckIn. It should
 	// be nil when Error is not-nil.
-	Output *model.OOAPICheckInResultNettests
+	Output *model.OOAPICheckInResult
 
 	// Error is the error to be returned by CheckIn. It
 	// should be nil when Output is not-nil.
@@ -455,7 +455,7 @@ type InputLoaderMockableSession struct {
 
 // CheckIn implements InputLoaderSession.CheckIn.
 func (sess *InputLoaderMockableSession) CheckIn(
-	ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error) {
+	ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) {
 	if sess.Output == nil && sess.Error == nil {
 		return nil, errors.New("both Output and Error are nil")
 	}
@@ -480,7 +480,9 @@ func TestInputLoaderCheckInFailure(t *testing.T) {
 func TestInputLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) {
 	il := &InputLoader{
 		Session: &InputLoaderMockableSession{
-			Output: &model.OOAPICheckInResultNettests{},
+			Output: &model.OOAPICheckInResult{
+				Tests: model.OOAPICheckInResultNettests{},
+			},
 		},
 	}
 	out, err := il.loadRemote(context.Background())
@@ -495,8 +497,10 @@ func TestInputLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) {
 func TestInputLoaderCheckInSuccessWithNoURLs(t *testing.T) {
 	il := &InputLoader{
 		Session: &InputLoaderMockableSession{
-			Output: &model.OOAPICheckInResultNettests{
-				WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{},
+			Output: &model.OOAPICheckInResult{
+				Tests: model.OOAPICheckInResultNettests{
+					WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{},
+				},
 			},
 		},
 	}
@@ -521,9 +525,11 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) {
 	}}
 	il := &InputLoader{
 		Session: &InputLoaderMockableSession{
-			Output: &model.OOAPICheckInResultNettests{
-				WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{
-					URLs: expect,
+			Output: &model.OOAPICheckInResult{
+				Tests: model.OOAPICheckInResultNettests{
+					WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{
+						URLs: expect,
+					},
 				},
 			},
 		},

internal/engine/savemeasurement.go

@@ -0,0 +1,39 @@
+package engine
+
+import (
+	"encoding/json"
+	"os"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+// SaveMeasurement saves a measurement on the specified file path.
+func SaveMeasurement(measurement *model.Measurement, filePath string) error {
+	return saveMeasurement(
+		measurement, filePath, json.Marshal, os.OpenFile,
+		func(fp *os.File, b []byte) (int, error) {
+			return fp.Write(b)
+		},
+	)
+}
+
+func saveMeasurement(
+	measurement *model.Measurement, filePath string,
+	marshal func(v interface{}) ([]byte, error),
+	openFile func(name string, flag int, perm os.FileMode) (*os.File, error),
+	write func(fp *os.File, b []byte) (n int, err error),
+) error {
+	data, err := marshal(measurement)
+	if err != nil {
+		return err
+	}
+	data = append(data, byte('\n'))
+	filep, err := openFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
+	if err != nil {
+		return err
+	}
+	if _, err := write(filep, data); err != nil {
+		return err
+	}
+	return filep.Close()
+}

internal/engine/savemeasurement_test.go

@@ -0,0 +1,85 @@
+package engine
+
+import (
+	"encoding/json"
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/must"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestSaveMeasurementSuccess(t *testing.T) {
+	// get temporary file where to write the measurement
+	filep, err := os.CreateTemp("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	filename := filep.Name()
+	filep.Close()
+
+	// create and fake-fill the measurement
+	m := &model.Measurement{}
+	ff := &testingx.FakeFiller{}
+	ff.Fill(m)
+
+	// write the measurement to disk
+	if err := SaveMeasurement(m, filename); err != nil {
+		t.Fatal(err)
+	}
+
+	// marshal the measurement to JSON with extra \n at the end
+	expect := append(must.MarshalJSON(m), '\n')
+
+	// read the measurement from file
+	got := runtimex.Try1(os.ReadFile(filename))
+
+	// make sure what we read matches what we expect
+	if diff := cmp.Diff(expect, got); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestSaveMeasurementErrors(t *testing.T) {
+	dirname, err := os.MkdirTemp("", "ooniprobe-engine-save-measurement")
+	if err != nil {
+		t.Fatal(err)
+	}
+	filename := filepath.Join(dirname, "report.jsonl")
+	m := new(model.Measurement)
+	err = saveMeasurement(
+		m, filename, func(v interface{}) ([]byte, error) {
+			return nil, errors.New("mocked error")
+		}, os.OpenFile, func(fp *os.File, b []byte) (int, error) {
+			return fp.Write(b)
+		},
+	)
+	if err == nil {
+		t.Fatal("expected an error here")
+	}
+	err = saveMeasurement(
+		m, filename, json.Marshal,
+		func(name string, flag int, perm os.FileMode) (*os.File, error) {
+			return nil, errors.New("mocked error")
+		}, func(fp *os.File, b []byte) (int, error) {
+			return fp.Write(b)
+		},
+	)
+	if err == nil {
+		t.Fatal("expected an error here")
+	}
+	err = saveMeasurement(
+		m, filename, json.Marshal, os.OpenFile,
+		func(fp *os.File, b []byte) (int, error) {
+			return 0, errors.New("mocked error")
+		},
+	)
+	if err == nil {
+		t.Fatal("expected an error here")
+	}
+}

internal/engine/session.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"net/url"
 	"os"
 	"sync"
@@ -155,7 +154,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) {
 	// use the temporary directory on the current system. This should
 	// work on Desktop. We tested that it did also work on iOS, but
 	// we have also seen on 2020-06-10 that it does not work on Android.
-	tempDir, err := ioutil.TempDir(config.TempDir, "ooniengine")
+	tempDir, err := os.MkdirTemp(config.TempDir, "ooniengine")
 	if err != nil {
 		return nil, err
 	}
@@ -265,9 +264,7 @@ func (s *Session) KibiBytesSent() float64 {
 //
 // The return value is either the check-in response or an error.
 func (s *Session) CheckIn(
-	ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error) {
-	// TODO(bassosimone): consider refactoring this function to return
-	// the whole check-in response to the caller.
+	ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) {
 	if err := s.maybeLookupLocationContext(ctx); err != nil {
 		return nil, err
 	}
@@ -300,7 +297,7 @@ func (s *Session) CheckIn(
 	if err != nil {
 		return nil, err
 	}
-	return &resp.Tests, nil
+	return resp, nil
 }
 
 // maybeLookupLocationContext is a wrapper for MaybeLookupLocationContext that calls
@@ -321,7 +318,7 @@ func (s *Session) newProbeServicesClientForCheckIn(
 	if s.testNewProbeServicesClientForCheckIn != nil {
 		return s.testNewProbeServicesClientForCheckIn(ctx)
 	}
-	client, err := s.NewProbeServicesClient(ctx)
+	client, err := s.newProbeServicesClient(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -367,7 +364,7 @@ func (s *Session) DefaultHTTPClient() model.HTTPClient {
 // FetchTorTargets fetches tor targets from the API.
 func (s *Session) FetchTorTargets(
 	ctx context.Context, cc string) (map[string]model.OOAPITorTarget, error) {
-	clnt, err := s.NewOrchestraClient(ctx)
+	clnt, err := s.newOrchestraClient(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -384,16 +381,6 @@ func (s *Session) Logger() model.Logger {
 	return s.logger
 }
 
-// MaybeLookupLocation is a caching location lookup call.
-func (s *Session) MaybeLookupLocation() error {
-	return s.MaybeLookupLocationContext(context.Background())
-}
-
-// MaybeLookupBackends is a caching OONI backends lookup call.
-func (s *Session) MaybeLookupBackends() error {
-	return s.MaybeLookupBackendsContext(context.Background())
-}
-
 // ErrAlreadyUsingProxy indicates that we cannot create a tunnel with
 // a specific name because we already configured a proxy.
 var ErrAlreadyUsingProxy = errors.New(
@@ -411,12 +398,12 @@ func (s *Session) NewExperimentBuilder(name string) (model.ExperimentBuilder, er
 	return eb, nil
 }
 
-// NewProbeServicesClient creates a new client for talking with the
+// newProbeServicesClient creates a new client for talking with the
 // OONI probe services. This function will benchmark the available
 // probe services, and select the fastest. In case all probe services
 // seem to be down, we try again applying circumvention tactics.
 // This function will fail IMMEDIATELY if given a cancelled context.
-func (s *Session) NewProbeServicesClient(ctx context.Context) (*probeservices.Client, error) {
+func (s *Session) newProbeServicesClient(ctx context.Context) (*probeservices.Client, error) {
 	if ctx.Err() != nil {
 		return nil, ctx.Err() // helps with testing
 	}
@@ -433,21 +420,18 @@ func (s *Session) NewProbeServicesClient(ctx context.Context) (*probeservices.Cl
 }
 
 // NewSubmitter creates a new submitter instance.
-func (s *Session) NewSubmitter(ctx context.Context) (Submitter, error) {
-	psc, err := s.NewProbeServicesClient(ctx)
+func (s *Session) NewSubmitter(ctx context.Context) (model.Submitter, error) {
+	psc, err := s.newProbeServicesClient(ctx)
 	if err != nil {
 		return nil, err
 	}
 	return probeservices.NewSubmitter(psc, s.Logger()), nil
 }
 
-// NewOrchestraClient creates a new orchestra client. This client is registered
+// newOrchestraClient creates a new orchestra client. This client is registered
 // and logged in with the OONI orchestra. An error is returned on failure.
-//
-// This function is DEPRECATED. New code SHOULD NOT use it. It will eventually
-// be made private or entirely removed from the codebase.
-func (s *Session) NewOrchestraClient(ctx context.Context) (*probeservices.Client, error) {
-	clnt, err := s.NewProbeServicesClient(ctx)
+func (s *Session) newOrchestraClient(ctx context.Context) (*probeservices.Client, error) {
+	clnt, err := s.newProbeServicesClient(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -665,9 +649,9 @@ func (s *Session) MaybeLookupBackendsContext(ctx context.Context) error {
 	return nil
 }
 
-// LookupLocationContext performs a location lookup. If you want memoisation
+// doLookupLocationContext performs a location lookup. If you want memoisation
 // of the results, you should use MaybeLookupLocationContext.
-func (s *Session) LookupLocationContext(ctx context.Context) (*enginelocate.Results, error) {
+func (s *Session) doLookupLocationContext(ctx context.Context) (*enginelocate.Results, error) {
 	task := enginelocate.NewTask(enginelocate.Config{
 		Logger:    s.Logger(),
 		Resolver:  s.resolver,
@@ -682,7 +666,7 @@ func (s *Session) lookupLocationContext(ctx context.Context) (*enginelocate.Resu
 	if s.testLookupLocationContext != nil {
 		return s.testLookupLocationContext(ctx)
 	}
-	return s.LookupLocationContext(ctx)
+	return s.doLookupLocationContext(ctx)
 }
 
 // MaybeLookupLocationContext is like MaybeLookupLocation but with a context

internal/engine/session_integration_test.go

@@ -146,7 +146,7 @@ func newSessionForTestingNoLookups(t *testing.T) *Session {
 
 func newSessionForTestingNoBackendsLookup(t *testing.T) *Session {
 	sess := newSessionForTestingNoLookups(t)
-	if err := sess.MaybeLookupLocation(); err != nil {
+	if err := sess.MaybeLookupLocationContext(context.Background()); err != nil {
 		t.Fatal(err)
 	}
 	log.Infof("Platform: %s", sess.Platform())
@@ -164,7 +164,7 @@ func newSessionForTestingNoBackendsLookup(t *testing.T) *Session {
 
 func newSessionForTesting(t *testing.T) *Session {
 	sess := newSessionForTestingNoBackendsLookup(t)
-	if err := sess.MaybeLookupBackends(); err != nil {
+	if err := sess.MaybeLookupBackendsContext(context.Background()); err != nil {
 		t.Fatal(err)
 	}
 	return sess
@@ -245,7 +245,7 @@ func TestBouncerError(t *testing.T) {
 	if sess.ProxyURL() == nil {
 		t.Fatal("expected to see explicit proxy here")
 	}
-	if err := sess.MaybeLookupBackends(); err == nil {
+	if err := sess.MaybeLookupBackendsContext(context.Background()); err == nil {
 		t.Fatal("expected an error here")
 	}
 }
@@ -260,7 +260,7 @@ func TestMaybeLookupBackendsNewClientError(t *testing.T) {
 		Address: "httpo://jehhrikjjqrlpufu.onion",
 	}}
 	defer sess.Close()
-	err := sess.MaybeLookupBackends()
+	err := sess.MaybeLookupBackendsContext(context.Background())
 	if !errors.Is(err, ErrAllProbeServicesFailed) {
 		t.Fatal("not the error we expected")
 	}
@@ -272,7 +272,7 @@ func TestSessionLocationLookup(t *testing.T) {
 	}
 	sess := newSessionForTestingNoLookups(t)
 	defer sess.Close()
-	if err := sess.MaybeLookupLocation(); err != nil {
+	if err := sess.MaybeLookupLocationContext(context.Background()); err != nil {
 		t.Fatal(err)
 	}
 	if sess.ProbeASNString() == model.DefaultProbeASNString {
@@ -419,7 +419,7 @@ func TestAllProbeServicesUnsupported(t *testing.T) {
 		Address: "mascetti",
 		Type:    "antani",
 	})
-	err = sess.MaybeLookupBackends()
+	err = sess.MaybeLookupBackendsContext(context.Background())
 	if !errors.Is(err, ErrAllProbeServicesFailed) {
 		t.Fatal("unexpected error")
 	}
@@ -447,7 +447,7 @@ func TestNewOrchestraClientMaybeLookupBackendsFailure(t *testing.T) {
 	sess.testMaybeLookupBackendsContext = func(ctx context.Context) error {
 		return errMocked
 	}
-	client, err := sess.NewOrchestraClient(context.Background())
+	client, err := sess.newOrchestraClient(context.Background())
 	if !errors.Is(err, errMocked) {
 		t.Fatal("not the error we expected", err)
 	}
@@ -465,7 +465,7 @@ func TestNewOrchestraClientMaybeLookupLocationFailure(t *testing.T) {
 	sess.testMaybeLookupLocationContext = func(ctx context.Context) error {
 		return errMocked
 	}
-	client, err := sess.NewOrchestraClient(context.Background())
+	client, err := sess.newOrchestraClient(context.Background())
 	if !errors.Is(err, errMocked) {
 		t.Fatalf("not the error we expected: %+v", err)
 	}
@@ -482,7 +482,7 @@ func TestNewOrchestraClientProbeServicesNewClientFailure(t *testing.T) {
 	sess.selectedProbeServiceHook = func(svc *model.OOAPIService) {
 		svc.Type = "antani" // should really not be supported for a long time
 	}
-	client, err := sess.NewOrchestraClient(context.Background())
+	client, err := sess.newOrchestraClient(context.Background())
 	if !errors.Is(err, probeservices.ErrUnsupportedEndpoint) {
 		t.Fatal("not the error we expected")
 	}

internal/engine/session_internal_test.go

@@ -65,24 +65,24 @@ func (c *mockableProbeServicesClientForCheckIn) CheckIn(
 }
 
 func TestSessionCheckInSuccessful(t *testing.T) {
-	results := &model.OOAPICheckInResultNettests{
-		WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{
-			ReportID: "xxx-x-xx",
-			URLs: []model.OOAPIURLInfo{{
-				CategoryCode: "NEWS",
-				CountryCode:  "IT",
-				URL:          "https://www.repubblica.it/",
-			}, {
-				CategoryCode: "NEWS",
-				CountryCode:  "IT",
-				URL:          "https://www.unita.it/",
-			}},
+	results := &model.OOAPICheckInResult{
+		Tests: model.OOAPICheckInResultNettests{
+			WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{
+				ReportID: "xxx-x-xx",
+				URLs: []model.OOAPIURLInfo{{
+					CategoryCode: "NEWS",
+					CountryCode:  "IT",
+					URL:          "https://www.repubblica.it/",
+				}, {
+					CategoryCode: "NEWS",
+					CountryCode:  "IT",
+					URL:          "https://www.unita.it/",
+				}},
+			},
 		},
 	}
 	mockedClnt := &mockableProbeServicesClientForCheckIn{
-		Results: &model.OOAPICheckInResult{
-			Tests: *results,
-		},
+		Results: results,
 	}
 	s := &Session{
 		location: &enginelocate.Results{

internal/engine/session_nopsiphon.go

@@ -9,7 +9,7 @@ import (
 
 // FetchPsiphonConfig fetches psiphon config from the API.
 func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
-	clnt, err := s.NewOrchestraClient(ctx)
+	clnt, err := s.newOrchestraClient(ctx)
 	if err != nil {
 		return nil, err
 	}

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/quicping/quic.go

@@ -128,5 +128,5 @@ type errUnexpectedResponse struct {
 
 // Error implements error.Error()
 func (e *errUnexpectedResponse) Error() string {
-	return fmt.Sprintf("unexptected response: %s", e.msg)
+	return fmt.Sprintf("unexpected response: %s", e.msg)
 }

internal/experiment/webconnectivity/webconnectivity_test.go

@@ -236,11 +236,11 @@ func newsession(t *testing.T, lookupBackends bool) model.ExperimentSession {
 		t.Fatal(err)
 	}
 	if lookupBackends {
-		if err := sess.MaybeLookupBackends(); err != nil {
+		if err := sess.MaybeLookupBackendsContext(context.Background()); err != nil {
 			t.Fatal(err)
 		}
 	}
-	if err := sess.MaybeLookupLocation(); err != nil {
+	if err := sess.MaybeLookupLocationContext(context.Background()); err != nil {
 		t.Fatal(err)
 	}
 	return sess

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/mocks/session.go

@@ -55,7 +55,7 @@ type Session struct {
 	MockNewSubmitter func(ctx context.Context) (model.Submitter, error)
 
 	MockCheckIn func(ctx context.Context,
-		config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error)
+		config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error)
 }
 
 func (sess *Session) GetTestHelpersByName(name string) ([]model.OOAPIService, bool) {
@@ -148,6 +148,6 @@ func (sess *Session) NewSubmitter(ctx context.Context) (model.Submitter, error)
 }
 
 func (sess *Session) CheckIn(ctx context.Context,
-	config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error) {
+	config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) {
 	return sess.MockCheckIn(ctx, config)
 }

internal/mocks/session_test.go

@@ -326,7 +326,7 @@ func TestSession(t *testing.T) {
 	t.Run("CheckIn", func(t *testing.T) {
 		expected := errors.New("mocked err")
 		s := &Session{
-			MockCheckIn: func(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error) {
+			MockCheckIn: func(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) {
 				return nil, expected
 			},
 		}

internal/model/experiment.go

@@ -196,11 +196,6 @@ type Experiment interface {
 	// when used with an asynchronous experiment.
 	MeasureWithContext(ctx context.Context, input string) (measurement *Measurement, err error)
 
-	// SaveMeasurement saves a measurement on the specified file path.
-	//
-	// Deprecated: new code should use a Saver.
-	SaveMeasurement(measurement *Measurement, filePath string) error
-
 	// SubmitAndUpdateMeasurementContext submits a measurement and updates the
 	// fields whose value has changed as part of the submission.
 	//

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{},
@@ -148,7 +156,20 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		version.Version,
 	))
 
-	// we only handle the POST method
+	// handle GET method for health check
+	if req.Method == "GET" {
+		metricRequestsCount.WithLabelValues("200", "ok").Inc()
+		resp := map[string]string{
+			"message": "Hello OONItarian!",
+		}
+		data, err := json.Marshal(resp)
+		runtimex.PanicOnError(err, "json.Marshal failed")
+		w.Header().Add("Content-Type", "application/json")
+		w.Write(data)
+		return
+	}
+
+	// we only handle the POST method for response generation
 	if req.Method != "POST" {
 		metricRequestsCount.WithLabelValues("400", "bad_request_method").Inc()
 		w.WriteHeader(400)

internal/oohelperd/handler_test.go

@@ -100,12 +100,20 @@ func TestHandlerWorkingAsIntended(t *testing.T) {
 
 	expectations := []expectationSpec{{
 		name:            "check for invalid method",
-		reqMethod:       "GET",
+		reqMethod:       "PUT",
 		reqContentType:  "",
 		reqBody:         strings.NewReader(""),
 		respStatusCode:  400,
 		respContentType: "",
 		parseBody:       false,
+	}, {
+		name:            "check for health message",
+		reqMethod:       "GET",
+		reqContentType:  "",
+		reqBody:         strings.NewReader(""),
+		respStatusCode:  200,
+		respContentType: "application/json",
+		parseBody:       true,
 	}, {
 		name:           "check for error reading request body",
 		reqMethod:      "POST",
@@ -254,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/oonirun/experiment.go

@@ -64,14 +64,14 @@ type Experiment struct {
 	newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader
 
 	// newSubmitterFn is OPTIONAL and used for testing.
-	newSubmitterFn func(ctx context.Context) (engine.Submitter, error)
+	newSubmitterFn func(ctx context.Context) (model.Submitter, error)
 
 	// newSaverFn is OPTIONAL and used for testing.
-	newSaverFn func(experiment model.Experiment) (engine.Saver, error)
+	newSaverFn func(experiment model.Experiment) (model.Saver, error)
 
 	// newInputProcessorFn is OPTIONAL and used for testing.
 	newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo,
-		saver engine.Saver, submitter engine.Submitter) inputProcessor
+		saver model.Saver, submitter model.Submitter) inputProcessor
 }
 
 // Run runs the given experiment.
@@ -138,47 +138,46 @@ type inputProcessor = model.ExperimentInputProcessor
 
 // newInputProcessor creates a new inputProcessor instance.
 func (ed *Experiment) newInputProcessor(experiment model.Experiment,
-	inputList []model.OOAPIURLInfo, saver engine.Saver, submitter engine.Submitter) inputProcessor {
+	inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor {
 	if ed.newInputProcessorFn != nil {
 		return ed.newInputProcessorFn(experiment, inputList, saver, submitter)
 	}
-	return &engine.InputProcessor{
+	return &InputProcessor{
 		Annotations: ed.Annotations,
 		Experiment: &experimentWrapper{
-			child:  engine.NewInputProcessorExperimentWrapper(experiment),
+			child:  NewInputProcessorExperimentWrapper(experiment),
 			logger: ed.Session.Logger(),
 			total:  len(inputList),
 		},
 		Inputs:     inputList,
 		MaxRuntime: time.Duration(ed.MaxRuntime) * time.Second,
 		Options:    experimentOptionsToStringList(ed.ExtraOptions),
-		Saver:      engine.NewInputProcessorSaverWrapper(saver),
+		Saver:      NewInputProcessorSaverWrapper(saver),
 		Submitter: &experimentSubmitterWrapper{
-			child:  engine.NewInputProcessorSubmitterWrapper(submitter),
+			child:  NewInputProcessorSubmitterWrapper(submitter),
 			logger: ed.Session.Logger(),
 		},
 	}
 }
 
 // newSaver creates a new engine.Saver instance.
-func (ed *Experiment) newSaver(experiment model.Experiment) (engine.Saver, error) {
+func (ed *Experiment) newSaver(experiment model.Experiment) (model.Saver, error) {
 	if ed.newSaverFn != nil {
 		return ed.newSaverFn(experiment)
 	}
-	return engine.NewSaver(engine.SaverConfig{
-		Enabled:    !ed.NoJSON,
-		Experiment: experiment,
-		FilePath:   ed.ReportFile,
-		Logger:     ed.Session.Logger(),
+	return NewSaver(SaverConfig{
+		Enabled:  !ed.NoJSON,
+		FilePath: ed.ReportFile,
+		Logger:   ed.Session.Logger(),
 	})
 }
 
 // newSubmitter creates a new engine.Submitter instance.
-func (ed *Experiment) newSubmitter(ctx context.Context) (engine.Submitter, error) {
+func (ed *Experiment) newSubmitter(ctx context.Context) (model.Submitter, error) {
 	if ed.newSubmitterFn != nil {
 		return ed.newSubmitterFn(ctx)
 	}
-	return engine.NewSubmitter(ctx, engine.SubmitterConfig{
+	return NewSubmitter(ctx, SubmitterConfig{
 		Enabled: !ed.NoCollector,
 		Session: ed.Session,
 		Logger:  ed.Session.Logger(),
@@ -234,7 +233,7 @@ func experimentOptionsToStringList(options map[string]any) (out []string) {
 // experimentWrapper wraps an experiment and logs progress
 type experimentWrapper struct {
 	// child is the child experiment wrapper
-	child engine.InputProcessorExperimentWrapper
+	child InputProcessorExperimentWrapper
 
 	// logger is the logger to use
 	logger model.Logger
@@ -255,7 +254,7 @@ func (ew *experimentWrapper) MeasureAsync(
 // fail if we cannot submit a measurement
 type experimentSubmitterWrapper struct {
 	// child is the child submitter wrapper
-	child engine.InputProcessorSubmitterWrapper
+	child InputProcessorSubmitterWrapper
 
 	// logger is the logger to use
 	logger model.Logger

internal/oonirun/experiment_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/ooni/probe-cli/v3/internal/engine"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/testingx"
@@ -81,7 +80,7 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) {
 		},
 		newExperimentBuilderFn: nil,
 		newInputLoaderFn:       nil,
-		newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+		newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 			subm := &mocks.Submitter{
 				MockSubmit: func(ctx context.Context, m *model.Measurement) error {
 					failedToSubmit++
@@ -171,9 +170,9 @@ func TestExperimentRun(t *testing.T) {
 		Session                Session
 		newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error)
 		newInputLoaderFn       func(inputPolicy model.InputPolicy) inputLoader
-		newSubmitterFn         func(ctx context.Context) (engine.Submitter, error)
-		newSaverFn             func(experiment model.Experiment) (engine.Saver, error)
-		newInputProcessorFn    func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver engine.Saver, submitter engine.Submitter) inputProcessor
+		newSubmitterFn         func(ctx context.Context) (model.Submitter, error)
+		newSaverFn             func(experiment model.Experiment) (model.Saver, error)
+		newInputProcessorFn    func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor
 	}
 	type args struct {
 		ctx context.Context
@@ -274,7 +273,7 @@ func TestExperimentRun(t *testing.T) {
 					},
 				}
 			},
-			newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+			newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 				return nil, errMocked
 			},
 		},
@@ -317,10 +316,10 @@ func TestExperimentRun(t *testing.T) {
 					},
 				}
 			},
-			newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+			newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 				return &mocks.Submitter{}, nil
 			},
-			newSaverFn: func(experiment model.Experiment) (engine.Saver, error) {
+			newSaverFn: func(experiment model.Experiment) (model.Saver, error) {
 				return nil, errMocked
 			},
 		},
@@ -363,14 +362,14 @@ func TestExperimentRun(t *testing.T) {
 					},
 				}
 			},
-			newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+			newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 				return &mocks.Submitter{}, nil
 			},
-			newSaverFn: func(experiment model.Experiment) (engine.Saver, error) {
+			newSaverFn: func(experiment model.Experiment) (model.Saver, error) {
 				return &mocks.Saver{}, nil
 			},
 			newInputProcessorFn: func(experiment model.Experiment, inputList []model.OOAPIURLInfo,
-				saver engine.Saver, submitter engine.Submitter) inputProcessor {
+				saver model.Saver, submitter model.Submitter) inputProcessor {
 				return &mocks.ExperimentInputProcessor{
 					MockRun: func(ctx context.Context) error {
 						return errMocked

internal/oonirun/inputprocessor.go

@@ -1,4 +1,4 @@
-package engine
+package oonirun
 
 import (
 	"context"
@@ -76,11 +76,11 @@ type InputProcessorSaverWrapper interface {
 }
 
 type inputProcessorSaverWrapper struct {
-	saver Saver
+	saver model.Saver
 }
 
 // NewInputProcessorSaverWrapper wraps a Saver for InputProcessor.
-func NewInputProcessorSaverWrapper(saver Saver) InputProcessorSaverWrapper {
+func NewInputProcessorSaverWrapper(saver model.Saver) InputProcessorSaverWrapper {
 	return inputProcessorSaverWrapper{saver: saver}
 }
 

internal/oonirun/inputprocessor_test.go

@@ -1,4 +1,4 @@
-package engine
+package oonirun
 
 import (
 	"context"

internal/oonirun/saver.go

@@ -1,22 +1,17 @@
-package engine
+package oonirun
 
 import (
 	"errors"
 
+	"github.com/ooni/probe-cli/v3/internal/engine"
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
-// Saver is an alias for model.Saver.
-type Saver = model.Saver
-
 // SaverConfig is the configuration for creating a new Saver.
 type SaverConfig struct {
 	// Enabled is true if saving is enabled.
 	Enabled bool
 
-	// Experiment is the experiment we're currently running.
-	Experiment SaverExperiment
-
 	// FilePath is the filepath where to append the measurement as a
 	// serialized JSON followed by a newline character.
 	FilePath string
@@ -25,23 +20,18 @@ type SaverConfig struct {
 	Logger model.Logger
 }
 
-// SaverExperiment is an experiment according to the Saver.
-type SaverExperiment interface {
-	SaveMeasurement(m *model.Measurement, filepath string) error
-}
-
 // NewSaver creates a new instance of Saver.
-func NewSaver(config SaverConfig) (Saver, error) {
+func NewSaver(config SaverConfig) (model.Saver, error) {
 	if !config.Enabled {
 		return fakeSaver{}, nil
 	}
 	if config.FilePath == "" {
 		return nil, errors.New("saver: passed an empty filepath")
 	}
-	return realSaver{
-		Experiment: config.Experiment,
-		FilePath:   config.FilePath,
-		Logger:     config.Logger,
+	return &realSaver{
+		FilePath: config.FilePath,
+		Logger:   config.Logger,
+		savefunc: engine.SaveMeasurement,
 	}, nil
 }
 
@@ -51,17 +41,17 @@ func (fs fakeSaver) SaveMeasurement(m *model.Measurement) error {
 	return nil
 }
 
-var _ Saver = fakeSaver{}
+var _ model.Saver = fakeSaver{}
 
 type realSaver struct {
-	Experiment SaverExperiment
-	FilePath   string
-	Logger     model.Logger
+	FilePath string
+	Logger   model.Logger
+	savefunc func(measurement *model.Measurement, filePath string) error
 }
 
-func (rs realSaver) SaveMeasurement(m *model.Measurement) error {
+func (rs *realSaver) SaveMeasurement(m *model.Measurement) error {
 	rs.Logger.Info("saving measurement to disk")
-	return rs.Experiment.SaveMeasurement(m, rs.FilePath)
+	return rs.savefunc(m, rs.FilePath)
 }
 
-var _ Saver = realSaver{}
+var _ model.Saver = &realSaver{}

internal/oonirun/saver_test.go

@@ -1,12 +1,14 @@
-package engine
+package oonirun
 
 import (
 	"errors"
+	"os"
 	"testing"
 
 	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
 func TestNewSaverDisabled(t *testing.T) {
@@ -38,43 +40,39 @@ func TestNewSaverWithEmptyFilePath(t *testing.T) {
 	}
 }
 
-type FakeSaverExperiment struct {
-	M        *model.Measurement
-	Error    error
-	FilePath string
-}
-
-func (fse *FakeSaverExperiment) SaveMeasurement(m *model.Measurement, filepath string) error {
-	fse.M = m
-	fse.FilePath = filepath
-	return fse.Error
-}
-
-var _ SaverExperiment = &FakeSaverExperiment{}
-
 func TestNewSaverWithFailureWhenSaving(t *testing.T) {
+	filep := runtimex.Try1(os.CreateTemp("", ""))
+	filename := filep.Name()
+	filep.Close()
 	expected := errors.New("mocked error")
-	fse := &FakeSaverExperiment{Error: expected}
 	saver, err := NewSaver(SaverConfig{
-		Enabled:    true,
-		FilePath:   "report.jsonl",
-		Experiment: fse,
-		Logger:     log.Log,
+		Enabled:  true,
+		FilePath: filename,
+		Logger:   log.Log,
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	if _, ok := saver.(realSaver); !ok {
+	realSaver, ok := saver.(*realSaver)
+	if !ok {
 		t.Fatal("not the type of saver we expected")
 	}
+	var (
+		gotMeasurement *model.Measurement
+		gotFilePath    string
+	)
+	realSaver.savefunc = func(measurement *model.Measurement, filePath string) error {
+		gotMeasurement, gotFilePath = measurement, filePath
+		return expected
+	}
 	m := &model.Measurement{Input: "www.kernel.org"}
 	if err := saver.SaveMeasurement(m); !errors.Is(err, expected) {
 		t.Fatalf("not the error we expected: %+v", err)
 	}
-	if diff := cmp.Diff(fse.M, m); diff != "" {
+	if diff := cmp.Diff(m, gotMeasurement); diff != "" {
 		t.Fatal(diff)
 	}
-	if fse.FilePath != "report.jsonl" {
+	if gotFilePath != filename {
 		t.Fatal("passed invalid filepath")
 	}
 }

internal/oonirun/session.go

@@ -20,7 +20,7 @@ type Session interface {
 	engine.InputLoaderSession
 
 	// A Session is also a SubmitterSession.
-	engine.SubmitterSession
+	SubmitterSession
 
 	// DefaultHTTPClient returns the session's default HTTPClient.
 	DefaultHTTPClient() model.HTTPClient

internal/oonirun/submitter.go

@@ -1,4 +1,4 @@
-package engine
+package oonirun
 
 import (
 	"context"

internal/oonirun/submitter_test.go

@@ -1,4 +1,4 @@
-package engine
+package oonirun
 
 import (
 	"context"

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(),

pkg/oonimkall/httpx.go

@@ -29,8 +29,10 @@ type HTTPRequest struct {
 	// Method is the MANDATORY request method.
 	Method string
 
-	// URL is the MANDATORY request URL.
-	URL string
+	// Url is the MANDATORY request URL.
+	//
+	// Note: this field MUST be named "Url" not "URL"; see https://github.com/ooni/probe/issues/2701.
+	Url string
 }
 
 // HTTPResponse is an HTTP response.
@@ -54,7 +56,7 @@ func (sess *Session) HTTPDo(ctx *Context, jreq *HTTPRequest) (*HTTPResponse, err
 func (sess *Session) httpDoLocked(ctx *Context, jreq *HTTPRequest) (*HTTPResponse, error) {
 	clnt := sess.sessp.DefaultHTTPClient()
 
-	req, err := http.NewRequestWithContext(ctx.ctx, jreq.Method, jreq.URL, nil)
+	req, err := http.NewRequestWithContext(ctx.ctx, jreq.Method, jreq.Url, nil)
 	if err != nil {
 		return nil, err
 	}

pkg/oonimkall/httpx_test.go

@@ -29,7 +29,7 @@ func TestSessionHTTPDo(t *testing.T) {
 
 		req := &oonimkall.HTTPRequest{
 			Method: "GET",
-			URL:    server.URL,
+			Url:    server.URL,
 		}
 
 		sess, err := NewSessionForTesting()
@@ -55,7 +55,7 @@ func TestSessionHTTPDo(t *testing.T) {
 
 		req := &oonimkall.HTTPRequest{
 			Method: "GET",
-			URL:    "\t", // this URL is invalid
+			Url:    "\t", // this URL is invalid
 		}
 
 		resp, err := sess.HTTPDo(sess.NewContext(), req)
@@ -75,7 +75,7 @@ func TestSessionHTTPDo(t *testing.T) {
 
 		req := &oonimkall.HTTPRequest{
 			Method: "GET",
-			URL:    server.URL,
+			Url:    server.URL,
 		}
 
 		sess, err := NewSessionForTesting()
@@ -107,7 +107,7 @@ func TestSessionHTTPDo(t *testing.T) {
 
 		req := &oonimkall.HTTPRequest{
 			Method: "GET",
-			URL:    URL.String(),
+			Url:    URL.String(),
 		}
 
 		sess, err := NewSessionForTesting()
@@ -142,7 +142,7 @@ func TestSessionHTTPDo(t *testing.T) {
 
 		req := &oonimkall.HTTPRequest{
 			Method: "GET",
-			URL:    server.URL,
+			Url:    server.URL,
 		}
 
 		sess, err := NewSessionForTesting()

pkg/oonimkall/session.go

@@ -13,7 +13,6 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/legacy/assetsdir"
 	"github.com/ooni/probe-cli/v3/internal/model"
-	"github.com/ooni/probe-cli/v3/internal/probeservices"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
@@ -127,7 +126,7 @@ type Session struct {
 
 	cl        []context.CancelFunc
 	mtx       sync.Mutex
-	submitter *probeservices.Submitter
+	submitter model.Submitter
 	sessp     *engine.Session
 }
 
@@ -277,15 +276,14 @@ type GeolocateResults struct {
 func (sess *Session) Geolocate(ctx *Context) (*GeolocateResults, error) {
 	sess.mtx.Lock()
 	defer sess.mtx.Unlock()
-	info, err := sess.sessp.LookupLocationContext(ctx.ctx)
-	if err != nil {
+	if err := sess.sessp.MaybeLookupLocationContext(ctx.ctx); err != nil {
 		return nil, err
 	}
 	return &GeolocateResults{
-		ASN:     info.ASNString(),
-		Country: info.CountryCode,
-		IP:      info.ProbeIP,
-		Org:     info.NetworkName,
+		ASN:     sess.sessp.ProbeASNString(),
+		Country: sess.sessp.ProbeCC(),
+		IP:      sess.sessp.ProbeIP(),
+		Org:     sess.sessp.ProbeNetworkName(),
 	}, nil
 }
 
@@ -307,11 +305,11 @@ func (sess *Session) Submit(ctx *Context, measurement string) (*SubmitMeasuremen
 	sess.mtx.Lock()
 	defer sess.mtx.Unlock()
 	if sess.submitter == nil {
-		psc, err := sess.sessp.NewProbeServicesClient(ctx.ctx)
+		submitter, err := sess.sessp.NewSubmitter(ctx.ctx)
 		if err != nil {
 			return nil, err
 		}
-		sess.submitter = probeservices.NewSubmitter(psc, sess.sessp.Logger())
+		sess.submitter = submitter
 	}
 	var mm model.Measurement
 	if err := json.Unmarshal([]byte(measurement), &mm); err != nil {
@@ -446,17 +444,12 @@ func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo,
 	if config.WebConnectivity == nil {
 		return nil, errors.New("oonimkall: missing webconnectivity config")
 	}
-	info, err := sess.sessp.LookupLocationContext(ctx.ctx)
-	if err != nil {
+	if err := sess.sessp.MaybeLookupLocationContext(ctx.ctx); err != nil {
 		return nil, err
 	}
 	if sess.TestingCheckInBeforeNewProbeServicesClient != nil {
 		sess.TestingCheckInBeforeNewProbeServicesClient(ctx) // for testing
 	}
-	psc, err := sess.sessp.NewProbeServicesClient(ctx.ctx)
-	if err != nil {
-		return nil, err
-	}
 	if sess.TestingCheckInBeforeCheckIn != nil {
 		sess.TestingCheckInBeforeCheckIn(ctx) // for testing
 	}
@@ -464,14 +457,14 @@ func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo,
 		Charging:        config.Charging,
 		OnWiFi:          config.OnWiFi,
 		Platform:        config.Platform,
-		ProbeASN:        info.ASNString(),
-		ProbeCC:         info.CountryCode,
+		ProbeASN:        sess.sessp.ProbeASNString(),
+		ProbeCC:         sess.sessp.ProbeCC(),
 		RunType:         model.RunType(config.RunType),
 		SoftwareName:    config.SoftwareName,
 		SoftwareVersion: config.SoftwareVersion,
 		WebConnectivity: config.WebConnectivity.toModel(),
 	}
-	result, err := psc.CheckIn(ctx.ctx, cfg)
+	result, err := sess.sessp.CheckIn(ctx.ctx, &cfg)
 	if err != nil {
 		return nil, err
 	}

pkg/oonimkall/session_integration_test.go

@@ -297,7 +297,7 @@ func TestCheckInLookupLocationFailure(t *testing.T) {
 	config.WebConnectivity.AddCategory("CULTR")
 	ctx.Cancel() // immediate failure
 	result, err := sess.CheckIn(ctx, &config)
-	if !errors.Is(err, enginelocate.ErrAllIPLookuppersFailed) {
+	if !errors.Is(err, context.Canceled) {
 		t.Fatalf("not the error we expected: %+v", err)
 	}
 	if result != nil {