Merge branch 'master' into issue/2545-small

Conflicts: internal/dslx/dns.go internal/dslx/dns_test.go internal/dslx/quic.go internal/dslx/quic_test.go internal/dslx/tcp.go internal/dslx/tcp_test.go internal/dslx/tls.go internal/dslx/tls_test.go
ooni · Oct 25, 2023 · d1d3142 · d1d3142
2 parents e1f5bc9 + 18cef86
commit d1d3142
Show file tree

Hide file tree

Showing 9 changed files with 221 additions and 162 deletions.
diff --git a/NDKVERSION b/NDKVERSION
@@ -1 +1 @@
-26.0.10792818
+26.1.10909125
diff --git a/cmd/ooniprobe/internal/nettests/run.go b/cmd/ooniprobe/internal/nettests/run.go
@@ -78,7 +78,7 @@ func RunGroup(config RunGroupConfig) error {
 		return err
 	}
 	if err := sess.MaybeLookupBackends(); err != nil {
-		log.WithError(err).Warn("Failed to discover OONI backends")
+		log.WithError(err).Errorf("Failed to discover OONI backends")
 		return err
 	}
 
@@ -116,7 +116,9 @@ func RunGroup(config RunGroupConfig) error {
 		ctl.RunType = config.RunType
 		ctl.SetNettestIndex(i, len(group.Nettests))
 		if err = nt.Run(ctl); err != nil {
-			log.WithError(err).Errorf("Failed to run %s", group.Label)
+			// We used to emit an error here, now we emit a warning--the proper choice
+			// given that we continue running. See https://github.com/ooni/probe/issues/2576.
+			log.WithError(err).Warnf("Failed to run %s", group.Label)
 		}
 	}
 

diff --git a/internal/dslx/dns.go b/internal/dslx/dns.go
@@ -72,92 +72,117 @@ type ResolvedAddresses struct {
 // DNSLookupGetaddrinfo returns a function that resolves a domain name to
 // IP addresses using libc's getaddrinfo function.
 func DNSLookupGetaddrinfo(rt Runtime) Func[*DomainToResolve, *Maybe[*ResolvedAddresses]] {
-	return FuncAdapter[*DomainToResolve, *Maybe[*ResolvedAddresses]](func(ctx context.Context, input *DomainToResolve) *Maybe[*ResolvedAddresses] {
-		// create trace
-		trace := rt.NewTrace(rt.IDGenerator().Add(1), rt.ZeroTime(), input.Tags...)
-
-		// start the operation logger
-		ol := logx.NewOperationLogger(
-			rt.Logger(),
-			"[#%d] DNSLookup[getaddrinfo] %s",
-			trace.Index(),
-			input.Domain,
-		)
-
-		// setup
-		const timeout = 4 * time.Second
-		ctx, cancel := context.WithTimeout(ctx, timeout)
-		defer cancel()
-
-		// create the resolver
-		resolver := trace.NewStdlibResolver(rt.Logger())
-
-		// lookup
-		addrs, err := resolver.LookupHost(ctx, input.Domain)
-
-		// stop the operation logger
-		ol.Stop(err)
-
-		state := &ResolvedAddresses{
-			Addresses: addrs, // maybe empty
-			Domain:    input.Domain,
-			Trace:     trace,
-		}
-
-		return &Maybe[*ResolvedAddresses]{
-			Error:        err,
-			Observations: maybeTraceToObservations(trace),
-			Operation:    netxlite.ResolveOperation,
-			State:        state,
-		}
-	})
+	return &dnsLookupGetaddrinfoFunc{rt}
+}
+
+// dnsLookupGetaddrinfoFunc is the function returned by DNSLookupGetaddrinfo.
+type dnsLookupGetaddrinfoFunc struct {
+	rt Runtime
+}
+
+// Apply implements Func.
+func (f *dnsLookupGetaddrinfoFunc) Apply(
+	ctx context.Context, input *DomainToResolve) *Maybe[*ResolvedAddresses] {
+
+	// create trace
+	trace := f.rt.NewTrace(f.rt.IDGenerator().Add(1), f.rt.ZeroTime(), input.Tags...)
+
+	// start the operation logger
+	ol := logx.NewOperationLogger(
+		f.rt.Logger(),
+		"[#%d] DNSLookup[getaddrinfo] %s",
+		trace.Index(),
+		input.Domain,
+	)
+
+	// setup
+	const timeout = 4 * time.Second
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+
+	// create the resolver
+	resolver := trace.NewStdlibResolver(f.rt.Logger())
+
+	// lookup
+	addrs, err := resolver.LookupHost(ctx, input.Domain)
+
+	// stop the operation logger
+	ol.Stop(err)
+
+	state := &ResolvedAddresses{
+		Addresses: addrs, // maybe empty
+		Domain:    input.Domain,
+		Trace:     trace,
+	}
+
+	return &Maybe[*ResolvedAddresses]{
+		Error:        err,
+		Observations: maybeTraceToObservations(trace),
+		Operation:    netxlite.ResolveOperation,
+		State:        state,
+	}
 }
 
 // DNSLookupUDP returns a function that resolves a domain name to
 // IP addresses using the given DNS-over-UDP resolver.
-func DNSLookupUDP(rt Runtime, endpoint string) Func[*DomainToResolve, *Maybe[*ResolvedAddresses]] {
-	return FuncAdapter[*DomainToResolve, *Maybe[*ResolvedAddresses]](func(ctx context.Context, input *DomainToResolve) *Maybe[*ResolvedAddresses] {
-		// create trace
-		trace := rt.NewTrace(rt.IDGenerator().Add(1), rt.ZeroTime(), input.Tags...)
-
-		// start the operation logger
-		ol := logx.NewOperationLogger(
-			rt.Logger(),
-			"[#%d] DNSLookup[%s/udp] %s",
-			trace.Index(),
-			endpoint,
-			input.Domain,
-		)
-
-		// setup
-		const timeout = 4 * time.Second
-		ctx, cancel := context.WithTimeout(ctx, timeout)
-		defer cancel()
-
-		// create the resolver
-		resolver := trace.NewParallelUDPResolver(
-			rt.Logger(),
-			trace.NewDialerWithoutResolver(rt.Logger()),
-			endpoint,
-		)
-
-		// lookup
-		addrs, err := resolver.LookupHost(ctx, input.Domain)
-
-		// stop the operation logger
-		ol.Stop(err)
-
-		state := &ResolvedAddresses{
-			Addresses: addrs, // maybe empty
-			Domain:    input.Domain,
-			Trace:     trace,
-		}
-
-		return &Maybe[*ResolvedAddresses]{
-			Error:        err,
-			Observations: maybeTraceToObservations(trace),
-			Operation:    netxlite.ResolveOperation,
-			State:        state,
-		}
-	})
+func DNSLookupUDP(rt Runtime, resolver string) Func[*DomainToResolve, *Maybe[*ResolvedAddresses]] {
+	return &dnsLookupUDPFunc{
+		Resolver: resolver,
+		rt:       rt,
+	}
+}
+
+// dnsLookupUDPFunc is the function returned by DNSLookupUDP.
+type dnsLookupUDPFunc struct {
+	// Resolver is the MANDATORY endpointed of the resolver to use.
+	Resolver string
+	rt       Runtime
+}
+
+// Apply implements Func.
+func (f *dnsLookupUDPFunc) Apply(
+	ctx context.Context, input *DomainToResolve) *Maybe[*ResolvedAddresses] {
+
+	// create trace
+	trace := f.rt.NewTrace(f.rt.IDGenerator().Add(1), f.rt.ZeroTime(), input.Tags...)
+
+	// start the operation logger
+	ol := logx.NewOperationLogger(
+		f.rt.Logger(),
+		"[#%d] DNSLookup[%s/udp] %s",
+		trace.Index(),
+		f.Resolver,
+		input.Domain,
+	)
+
+	// setup
+	const timeout = 4 * time.Second
+	ctx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+
+	// create the resolver
+	resolver := trace.NewParallelUDPResolver(
+		f.rt.Logger(),
+		trace.NewDialerWithoutResolver(f.rt.Logger()),
+		f.Resolver,
+	)
+
+	// lookup
+	addrs, err := resolver.LookupHost(ctx, input.Domain)
+
+	// stop the operation logger
+	ol.Stop(err)
+
+	state := &ResolvedAddresses{
+		Addresses: addrs, // maybe empty
+		Domain:    input.Domain,
+		Trace:     trace,
+	}
+
+	return &Maybe[*ResolvedAddresses]{
+		Error:        err,
+		Observations: maybeTraceToObservations(trace),
+		Operation:    netxlite.ResolveOperation,
+		State:        state,
+	}
 }
diff --git a/internal/dslx/dns_test.go b/internal/dslx/dns_test.go
@@ -54,16 +54,23 @@ Test cases:
   - with success
 */
 func TestGetaddrinfo(t *testing.T) {
+	t.Run("Get dnsLookupGetaddrinfoFunc", func(t *testing.T) {
+		f := DNSLookupGetaddrinfo(NewMinimalRuntime(model.DiscardLogger, time.Now()))
+		if _, ok := f.(*dnsLookupGetaddrinfoFunc); !ok {
+			t.Fatal("unexpected type, want dnsLookupGetaddrinfoFunc")
+		}
+	})
+
 	t.Run("Apply dnsLookupGetaddrinfoFunc", func(t *testing.T) {
 		domain := &DomainToResolve{
 			Domain: "example.com",
 			Tags:   []string{"antani"},
 		}
 
 		t.Run("with nil resolver", func(t *testing.T) {
-			f := DNSLookupGetaddrinfo(
-				NewMinimalRuntime(model.DiscardLogger, time.Now()),
-			)
+			f := dnsLookupGetaddrinfoFunc{
+				rt: NewMinimalRuntime(model.DiscardLogger, time.Now()),
+			}
 			ctx, cancel := context.WithCancel(context.Background())
 			cancel() // immediately cancel the lookup
 			res := f.Apply(ctx, domain)
@@ -77,8 +84,8 @@ func TestGetaddrinfo(t *testing.T) {
 
 		t.Run("with lookup error", func(t *testing.T) {
 			mockedErr := errors.New("mocked")
-			f := DNSLookupGetaddrinfo(
-				NewMinimalRuntime(model.DiscardLogger, time.Now(), MinimalRuntimeOptionMeasuringNetwork(&mocks.MeasuringNetwork{
+			f := dnsLookupGetaddrinfoFunc{
+				rt: NewMinimalRuntime(model.DiscardLogger, time.Now(), MinimalRuntimeOptionMeasuringNetwork(&mocks.MeasuringNetwork{
 					MockNewStdlibResolver: func(logger model.DebugLogger) model.Resolver {
 						return &mocks.Resolver{
 							MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
@@ -87,7 +94,7 @@ func TestGetaddrinfo(t *testing.T) {
 						}
 					},
 				})),
-			)
+			}
 			res := f.Apply(context.Background(), domain)
 			if res.Observations == nil || len(res.Observations) <= 0 {
 				t.Fatal("unexpected empty observations")
@@ -104,8 +111,8 @@ func TestGetaddrinfo(t *testing.T) {
 		})
 
 		t.Run("with success", func(t *testing.T) {
-			f := DNSLookupGetaddrinfo(
-				NewRuntimeMeasurexLite(model.DiscardLogger, time.Now(), RuntimeMeasurexLiteOptionMeasuringNetwork(&mocks.MeasuringNetwork{
+			f := dnsLookupGetaddrinfoFunc{
+				rt: NewRuntimeMeasurexLite(model.DiscardLogger, time.Now(), RuntimeMeasurexLiteOptionMeasuringNetwork(&mocks.MeasuringNetwork{
 					MockNewStdlibResolver: func(logger model.DebugLogger) model.Resolver {
 						return &mocks.Resolver{
 							MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
@@ -114,7 +121,7 @@ func TestGetaddrinfo(t *testing.T) {
 						}
 					},
 				})),
-			)
+			}
 			res := f.Apply(context.Background(), domain)
 			if res.Observations == nil || len(res.Observations) <= 0 {
 				t.Fatal("unexpected empty observations")
@@ -144,14 +151,22 @@ Test cases:
   - with success
 */
 func TestLookupUDP(t *testing.T) {
+	t.Run("Get dnsLookupUDPFunc", func(t *testing.T) {
+		rt := NewMinimalRuntime(model.DiscardLogger, time.Now())
+		f := DNSLookupUDP(rt, "1.1.1.1:53")
+		if _, ok := f.(*dnsLookupUDPFunc); !ok {
+			t.Fatal("unexpected type, want dnsLookupUDPFunc")
+		}
+	})
+
 	t.Run("Apply dnsLookupGetaddrinfoFunc", func(t *testing.T) {
 		domain := &DomainToResolve{
 			Domain: "example.com",
 			Tags:   []string{"antani"},
 		}
 
 		t.Run("with nil resolver", func(t *testing.T) {
-			f := DNSLookupUDP(NewMinimalRuntime(model.DiscardLogger, time.Now()), "1.1.1.1:53")
+			f := dnsLookupUDPFunc{Resolver: "1.1.1.1:53", rt: NewMinimalRuntime(model.DiscardLogger, time.Now())}
 			ctx, cancel := context.WithCancel(context.Background())
 			cancel()
 			res := f.Apply(ctx, domain)
@@ -165,8 +180,9 @@ func TestLookupUDP(t *testing.T) {
 
 		t.Run("with lookup error", func(t *testing.T) {
 			mockedErr := errors.New("mocked")
-			f := DNSLookupUDP(
-				NewMinimalRuntime(model.DiscardLogger, time.Now(), MinimalRuntimeOptionMeasuringNetwork(&mocks.MeasuringNetwork{
+			f := dnsLookupUDPFunc{
+				Resolver: "1.1.1.1:53",
+				rt: NewMinimalRuntime(model.DiscardLogger, time.Now(), MinimalRuntimeOptionMeasuringNetwork(&mocks.MeasuringNetwork{
 					MockNewParallelUDPResolver: func(logger model.DebugLogger, dialer model.Dialer, endpoint string) model.Resolver {
 						return &mocks.Resolver{
 							MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
@@ -182,8 +198,7 @@ func TestLookupUDP(t *testing.T) {
 						}
 					},
 				})),
-				"1.1.1.1:53",
-			)
+			}
 			res := f.Apply(context.Background(), domain)
 			if res.Observations == nil || len(res.Observations) <= 0 {
 				t.Fatal("unexpected empty observations")
@@ -200,8 +215,9 @@ func TestLookupUDP(t *testing.T) {
 		})
 
 		t.Run("with success", func(t *testing.T) {
-			f := DNSLookupUDP(
-				NewRuntimeMeasurexLite(model.DiscardLogger, time.Now(), RuntimeMeasurexLiteOptionMeasuringNetwork(&mocks.MeasuringNetwork{
+			f := dnsLookupUDPFunc{
+				Resolver: "1.1.1.1:53",
+				rt: NewRuntimeMeasurexLite(model.DiscardLogger, time.Now(), RuntimeMeasurexLiteOptionMeasuringNetwork(&mocks.MeasuringNetwork{
 					MockNewParallelUDPResolver: func(logger model.DebugLogger, dialer model.Dialer, address string) model.Resolver {
 						return &mocks.Resolver{
 							MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
@@ -217,8 +233,7 @@ func TestLookupUDP(t *testing.T) {
 						}
 					},
 				})),
-				"1.1.1.1:53",
-			)
+			}
 			res := f.Apply(context.Background(), domain)
 			if res.Observations == nil || len(res.Observations) <= 0 {
 				t.Fatal("unexpected empty observations")

diff --git a/internal/dslx/fxcore.go b/internal/dslx/fxcore.go
@@ -17,14 +17,6 @@ type Func[A, B any] interface {
 	Apply(ctx context.Context, a A) B
 }
 
-// FuncAdapter adapts a func to be a Func.
-type FuncAdapter[A, B any] func(ctx context.Context, a A) B
-
-// Apply implements Func.
-func (fa FuncAdapter[A, B]) Apply(ctx context.Context, a A) B {
-	return fa(ctx, a)
-}
-
 // Maybe is the result of an operation implemented by this package
 // that may fail such as [TCPConnect] or [TLSHandshake].
 type Maybe[State any] struct {