feat(webconnectivity@v0.5): flag case where noone resolved any address (

#953)

See ooni/probe#2290

While there, notice that in such a case the priority selector would hang because of the WaitGroup, so get rid of the WaitGroup and accept that the priority selector is going to hang around for the whole duration of the measurement in some cases. The cancellable `measurer.go`'s context will cause the priority selector to eventually exit when we return from `measurer.go`'s `Run` method.

internal/experiment/webconnectivity/analysiscore.go

@@ -67,7 +67,7 @@ const (
 // values of .Blocking and .Accessible:
 //
 //     +--------------------------------------+----------------+-------------+
-//     | XBlockingFlags                       | .Blocking      | .Accessible |
+//     | .BlockingFlags                       | .Blocking      | .Accessible |
 //     +--------------------------------------+----------------+-------------+
 //     | (& DNSBlocking) != 0                 | "dns"          | false       |
 //     +--------------------------------------+----------------+-------------+
@@ -135,6 +135,15 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) {
 		)
 
 	default:
+		if tk.analysisNullNullDetectNoAddrs(logger) {
+			tk.Blocking = false
+			tk.Accessible = false
+			logger.Infof(
+				"NO_AVAILABLE_ADDRS: flags=%d, accessible=%+v, blocking=%+v",
+				tk.BlockingFlags, tk.Accessible, tk.Blocking,
+			)
+			return
+		}
 		tk.Blocking = nil
 		tk.Accessible = nil
 		logger.Warnf(
@@ -143,3 +152,55 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) {
 		)
 	}
 }
+
+const (
+	// analysisFlagNullNullNoAddrs indicates neither the probe nor the TH were
+	// able to get any IP addresses from any resolver.
+	analysisFlagNullNullNoAddrs = 1 << iota
+)
+
+// analysisNullNullDetectNoAddrs attempts to see whether we
+// ended up into the .Blocking = nil, .Accessible = nil case because
+// the domain is expired and all queries returned no addresses.
+//
+// See https://github.com/ooni/probe/issues/2290 for further
+// documentation about the issue we're solving here.
+//
+// It would be tempting to check specifically for NXDOMAIN here, but we
+// know it is problematic do that. In fact, on Android the getaddrinfo
+// resolver always returns EAI_NODATA on error, regardless of the actual
+// error that may have occurred in the Android DNS backend.
+//
+// See https://github.com/ooni/probe/issues/2029 for more information
+// on Android's getaddrinfo behavior.
+func (tk *TestKeys) analysisNullNullDetectNoAddrs(logger model.Logger) bool {
+	if tk.Control == nil {
+		// we need control data to say we're in this case
+		return false
+	}
+	for _, query := range tk.Queries {
+		if len(query.Answers) > 0 {
+			// when a query has answers, we're not in the NoAddresses case
+			return false
+		}
+	}
+	if len(tk.TCPConnect) > 0 {
+		// if we attempted TCP connect, we're not in the NoAddresses case
+		return false
+	}
+	if len(tk.TLSHandshakes) > 0 {
+		// if we attempted TLS handshakes, we're not in the NoAddresses case
+		return false
+	}
+	if len(tk.Control.DNS.Addrs) > 0 {
+		// when the TH resolved addresses, we're not in the NoAddresses case
+		return false
+	}
+	if len(tk.Control.TCPConnect) > 0 {
+		// when the TH used addresses, we're not in the NoAddresses case
+		return false
+	}
+	logger.Infof("Neither the probe nor the TH resolved any addresses")
+	tk.NullNullFlags |= analysisFlagNullNullNoAddrs
+	return true
+}

internal/experiment/webconnectivity/dnsresolvers.go

@@ -172,7 +172,7 @@ func (t *DNSResolvers) Run(parentCtx context.Context) {
 	}
 
 	// create priority selector
-	ps := newPrioritySelector(parentCtx, t.ZeroTime, t.TestKeys, t.Logger, t.WaitGroup, addresses)
+	ps := newPrioritySelector(parentCtx, t.ZeroTime, t.TestKeys, t.Logger, addresses)
 
 	// fan out a number of child async tasks to use the IP addrs
 	t.startCleartextFlows(parentCtx, ps, addresses)

internal/experiment/webconnectivity/measurer.go

@@ -36,7 +36,7 @@ func (m *Measurer) ExperimentName() string {
 
 // ExperimentVersion implements model.ExperimentMeasurer.
 func (m *Measurer) ExperimentVersion() string {
-	return "0.5.10"
+	return "0.5.11"
 }
 
 // Run implements model.ExperimentMeasurer.

internal/experiment/webconnectivity/priority.go

@@ -30,7 +30,6 @@ import (
 	"context"
 	"fmt"
 	"net"
-	"sync"
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
@@ -80,7 +79,6 @@ func newPrioritySelector(
 	zeroTime time.Time,
 	tk *TestKeys,
 	logger model.Logger,
-	wg *sync.WaitGroup,
 	addrs []DNSEntry,
 ) *prioritySelector {
 	ps := &prioritySelector{
@@ -107,8 +105,7 @@ func newPrioritySelector(
 			ps.nhttps++
 		}
 	}
-	wg.Add(1)
-	go ps.selector(ctx, wg)
+	go ps.selector(ctx)
 	return ps
 }
 
@@ -151,10 +148,7 @@ func (ps *prioritySelector) permissionToFetch(address string) bool {
 // background goroutine and terminates when [ctx] is done.
 //
 // This function implements https://github.com/ooni/probe/issues/2276.
-func (ps *prioritySelector) selector(ctx context.Context, wg *sync.WaitGroup) {
-	// synchronize with the parent
-	defer wg.Done()
-
+func (ps *prioritySelector) selector(ctx context.Context) {
 	// Implementation note: setting an arbitrary timeout here would
 	// be ~an issue because we want this goroutine to be available in
 	// case the only connections from which we could fetch a webpage

internal/experiment/webconnectivity/testkeys.go

@@ -90,6 +90,11 @@ type TestKeys struct {
 	// BlockingFlags contains blocking flags.
 	BlockingFlags int64 `json:"x_blocking_flags"`
 
+	// NullNullFlags explains why we determined that a measurement is not
+	// failed by detecting specific conditions that would have otherwise
+	// caused .Accessible = nil and .Blocking = nil
+	NullNullFlags int64 `json:"x_null_null_flags"`
+
 	// BodyLength match tells us whether the body length matches.
 	BodyLengthMatch *bool `json:"body_length_match"`
 
@@ -334,6 +339,7 @@ func NewTestKeys() *TestKeys {
 		DNSConsistency:        "",
 		HTTPExperimentFailure: nil,
 		BlockingFlags:         0,
+		NullNullFlags:         0,
 		BodyLengthMatch:       nil,
 		HeadersMatch:          nil,
 		StatusCodeMatch:       nil,