diff --git a/internal/experiment/webconnectivity/analysiscore.go b/internal/experiment/webconnectivity/analysiscore.go index 1ed20bbf48..e92c908049 100644 --- a/internal/experiment/webconnectivity/analysiscore.go +++ b/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 +} diff --git a/internal/experiment/webconnectivity/dnsresolvers.go b/internal/experiment/webconnectivity/dnsresolvers.go index 856b4fa382..89465f16e4 100644 --- a/internal/experiment/webconnectivity/dnsresolvers.go +++ b/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) diff --git a/internal/experiment/webconnectivity/measurer.go b/internal/experiment/webconnectivity/measurer.go index 21ce49dd2d..592c8ba598 100644 --- a/internal/experiment/webconnectivity/measurer.go +++ b/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. diff --git a/internal/experiment/webconnectivity/priority.go b/internal/experiment/webconnectivity/priority.go index 4f84af1273..6c043dd010 100644 --- a/internal/experiment/webconnectivity/priority.go +++ b/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 diff --git a/internal/experiment/webconnectivity/testkeys.go b/internal/experiment/webconnectivity/testkeys.go index 94d6282980..327bb7c7a8 100644 --- a/internal/experiment/webconnectivity/testkeys.go +++ b/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,