refactor(webconnectivitylte): make analysis flags public (#1447)

This PR contains yak shaving before doing
ooni/probe#2640.

We extracted this PR from #1446.

internal/experiment/webconnectivitylte/analysiscore.go

@@ -17,23 +17,23 @@ import (
 // These flags determine the context of TestKeys.Blocking. However, while .Blocking
 // is an enumeration, these flags allow to describe multiple blocking methods.
 const (
-	// analysisFlagDNSBlocking indicates there's blocking at the DNS level.
-	analysisFlagDNSBlocking = 1 << iota
+	// AnalysisBlockingFlagDNSBlocking indicates there's blocking at the DNS level.
+	AnalysisBlockingFlagDNSBlocking = 1 << iota
 
-	// analysisFlagTCPIPBlocking indicates there's blocking at the TCP/IP level.
-	analysisFlagTCPIPBlocking
+	// AnalysisBlockingFlagTCPIPBlocking indicates there's blocking at the TCP/IP level.
+	AnalysisBlockingFlagTCPIPBlocking
 
-	// analysisFlagTLSBlocking indicates there were TLS issues.
-	analysisFlagTLSBlocking
+	// AnalysisBlockingFlagTLSBlocking indicates there were TLS issues.
+	AnalysisBlockingFlagTLSBlocking
 
-	// analysisFlagHTTPBlocking indicates there was an HTTP failure.
-	analysisFlagHTTPBlocking
+	// AnalysisBlockingFlagHTTPBlocking indicates there was an HTTP failure.
+	AnalysisBlockingFlagHTTPBlocking
 
-	// analysisFlagHTTPDiff indicates there's an HTTP diff.
-	analysisFlagHTTPDiff
+	// AnalysisBlockingFlagHTTPDiff indicates there's an HTTP diff.
+	AnalysisBlockingFlagHTTPDiff
 
-	// analysisFlagSuccess indicates we did not detect any blocking.
-	analysisFlagSuccess
+	// AnalysisBlockingFlagSuccess indicates we did not detect any blocking.
+	AnalysisBlockingFlagSuccess
 )
 
 // AnalysisEngineFn is the function that runs the analysis engine for
@@ -98,6 +98,8 @@ var AnalysisEngineFn func(tk *TestKeys, logger model.Logger) = AnalysisEngineCla
 //
 // As an improvement over Web Connectivity v0.4, we also attempt to identify
 // special subcases of a null, null result to provide the user with more information.
+//
+// This function MUTATES the test keys.
 func (tk *TestKeys) analysisToplevel(logger model.Logger) {
 	AnalysisEngineFn(tk, logger)
 }
@@ -121,15 +123,15 @@ func (tk *TestKeys) analysisOrig(logger model.Logger) {
 
 	// now, let's determine .Accessible and .Blocking
 	switch {
-	case (tk.BlockingFlags & analysisFlagDNSBlocking) != 0:
+	case (tk.BlockingFlags & AnalysisBlockingFlagDNSBlocking) != 0:
 		tk.Blocking = "dns"
 		tk.Accessible = false
 		logger.Warnf(
 			"ANOMALY: flags=%d, accessible=%+v, blocking=%+v",
 			tk.BlockingFlags, tk.Accessible, tk.Blocking,
 		)
 
-	case (tk.BlockingFlags & analysisFlagTCPIPBlocking) != 0:
+	case (tk.BlockingFlags & AnalysisBlockingFlagTCPIPBlocking) != 0:
 		tk.Blocking = "tcp_ip"
 		tk.Accessible = false
 		logger.Warnf(
@@ -139,22 +141,22 @@ func (tk *TestKeys) analysisOrig(logger model.Logger) {
 
 	// Assigning "http-failure" for both TLS and HTTP blocking is a legacy behavior
 	// because the spec does not consider the case of TLS based blocking
-	case (tk.BlockingFlags & (analysisFlagTLSBlocking | analysisFlagHTTPBlocking)) != 0:
+	case (tk.BlockingFlags & (AnalysisBlockingFlagTLSBlocking | AnalysisBlockingFlagHTTPBlocking)) != 0:
 		tk.Blocking = "http-failure"
 		tk.Accessible = false
 		logger.Warnf("ANOMALY: flags=%d, accessible=%+v, blocking=%+v",
 			tk.BlockingFlags, tk.Accessible, tk.Blocking,
 		)
 
-	case (tk.BlockingFlags & analysisFlagHTTPDiff) != 0:
+	case (tk.BlockingFlags & AnalysisBlockingFlagHTTPDiff) != 0:
 		tk.Blocking = "http-diff"
 		tk.Accessible = false
 		logger.Warnf(
 			"ANOMALY: flags=%d, accessible=%+v, blocking=%+v",
 			tk.BlockingFlags, tk.Accessible, tk.Blocking,
 		)
 
-	case tk.BlockingFlags == analysisFlagSuccess:
+	case tk.BlockingFlags == AnalysisBlockingFlagSuccess:
 		tk.Blocking = false
 		tk.Accessible = true
 		logger.Infof(

internal/experiment/webconnectivitylte/analysisdns.go

@@ -14,16 +14,16 @@ import (
 )
 
 const (
-	// AnalysisDNSBogon indicates we got any bogon reply
-	AnalysisDNSBogon = 1 << iota
+	// AnalysisFlagDNSBogon indicates we got any bogon reply
+	AnalysisFlagDNSBogon = 1 << iota
 
-	// AnalysisDNSUnexpectedFailure indicates the TH could
+	// AnalysisDNSFlagUnexpectedFailure indicates the TH could
 	// resolve a domain while the probe couldn't
-	AnalysisDNSUnexpectedFailure
+	AnalysisDNSFlagUnexpectedFailure
 
-	// AnalysisDNSUnexpectedAddrs indicates the TH resolved
+	// AnalysisDNSFlagUnexpectedAddrs indicates the TH resolved
 	// different addresses from the probe
-	AnalysisDNSUnexpectedAddrs
+	AnalysisDNSFlagUnexpectedAddrs
 )
 
 // analysisDNSToplevel is the toplevel analysis function for DNS results.
@@ -64,7 +64,7 @@ func (tk *TestKeys) analysisDNSToplevel(logger model.Logger, lookupper model.Geo
 	if tk.DNSFlags != 0 {
 		logger.Warn("DNSConsistency: inconsistent")
 		tk.DNSConsistency = optional.Some("inconsistent")
-		tk.BlockingFlags |= analysisFlagDNSBlocking
+		tk.BlockingFlags |= AnalysisBlockingFlagDNSBlocking
 	} else {
 		logger.Info("DNSConsistency: consistent")
 		tk.DNSConsistency = optional.Some("consistent")
@@ -102,7 +102,7 @@ func (tk *TestKeys) analysisDNSExperimentFailure() {
 	}
 }
 
-// analysisDNSBogon computes the AnalysisDNSBogon flag. We set this flag if
+// analysisDNSBogon computes the AnalysisFlagDNSBogon flag. We set this flag if
 // we dectect any bogon in the .Queries field of the TestKeys.
 func (tk *TestKeys) analysisDNSBogon(logger model.Logger) {
 	for _, query := range tk.Queries {
@@ -122,7 +122,7 @@ func (tk *TestKeys) analysisDNSBogon(logger model.Logger) {
 						query.Hostname,
 						query.TransactionID,
 					)
-					tk.DNSFlags |= AnalysisDNSBogon
+					tk.DNSFlags |= AnalysisFlagDNSBogon
 					// continue processing so we print all the bogons we have
 				}
 			case "AAAA":
@@ -133,7 +133,7 @@ func (tk *TestKeys) analysisDNSBogon(logger model.Logger) {
 						query.Hostname,
 						query.TransactionID,
 					)
-					tk.DNSFlags |= AnalysisDNSBogon
+					tk.DNSFlags |= AnalysisFlagDNSBogon
 					// continue processing so we print all the bogons we have
 				}
 			default:
@@ -143,7 +143,7 @@ func (tk *TestKeys) analysisDNSBogon(logger model.Logger) {
 	}
 }
 
-// analysisDNSUnexpectedFailure computes the AnalysisDNSUnexpectedFailure flags. We say
+// analysisDNSUnexpectedFailure computes the AnalysisDNSFlagUnexpectedFailure flags. We say
 // a failure is unexpected when the TH could resolve a domain and the probe couldn't.
 func (tk *TestKeys) analysisDNSUnexpectedFailure(logger model.Logger) {
 	// make sure we have control before proceeding futher
@@ -212,15 +212,15 @@ func (tk *TestKeys) analysisDNSUnexpectedFailure(logger model.Logger) {
 			continue
 		}
 		logger.Warnf("DNS: unexpected failure %s in #%d", *query.Failure, query.TransactionID)
-		tk.DNSFlags |= AnalysisDNSUnexpectedFailure
+		tk.DNSFlags |= AnalysisDNSFlagUnexpectedFailure
 		// continue processing so we print all the unexpected failures
 
 		// TODO(https://github.com/ooni/probe/issues/2029#issuecomment-1411716295): we need
 		// to ensure we correctly handle the android_dns_cache_no_data case.
 	}
 }
 
-// analysisDNSUnexpectedAddrs computes the AnalysisDNSUnexpectedAddrs flags. This
+// analysisDNSUnexpectedAddrs computes the AnalysisDNSFlagUnexpectedAddrs flags. This
 // algorithm builds upon the original DNSDiff algorithm by introducing an additional
 // TLS based heuristic for determining whether an IP address was legit.
 func (tk *TestKeys) analysisDNSUnexpectedAddrs(
@@ -282,7 +282,7 @@ func (tk *TestKeys) analysisDNSUnexpectedAddrs(
 	// definitely suspicious and counts as a difference
 	if len(probeAddrs) <= 0 {
 		logger.Warnf("DNS: the probe did not resolve any IP address")
-		tk.DNSFlags |= AnalysisDNSUnexpectedAddrs
+		tk.DNSFlags |= AnalysisDNSFlagUnexpectedAddrs
 		return
 	}
 
@@ -320,7 +320,7 @@ func (tk *TestKeys) analysisDNSUnexpectedAddrs(
 			addr, asn,
 		)
 	}
-	tk.DNSFlags |= AnalysisDNSUnexpectedAddrs
+	tk.DNSFlags |= AnalysisDNSFlagUnexpectedAddrs
 }
 
 // analysisDNSDiffAddrs returns all the IP addresses that are

internal/experiment/webconnectivitylte/analysisdns_test.go

@@ -64,7 +64,7 @@ func TestTestKeys_analysisDNSToplevel(t *testing.T) {
 			"17.248.248.105":       {ASNumber: 714, Organization: "Apple Inc."},
 			"17.248.248.100":       {ASNumber: 714, Organization: "Apple Inc."},
 		},
-		expecteBlockingFlags: analysisFlagDNSBlocking,
+		expecteBlockingFlags: AnalysisBlockingFlagDNSBlocking,
 	}}
 
 	for _, tc := range testcases {

internal/experiment/webconnectivitylte/analysishttpcore.go

@@ -19,9 +19,9 @@ import (
 //
 // This results in possibly setting these XBlockingFlags:
 //
-// - analysisFlagHTTPBlocking
+// - AnalysisBlockingFlagHTTPBlocking
 //
-// - analysisFlagHTTPDiff
+// - AnalysisBlockingFlagHTTPDiff
 //
 // In websteps fashion, we don't stop at the first failure, rather we
 // process all the available data and evaluate all possible errors.
@@ -58,7 +58,7 @@ func (tk *TestKeys) analysisHTTPToplevel(logger model.Logger) {
 		case netxlite.FailureConnectionReset,
 			netxlite.FailureGenericTimeoutError,
 			netxlite.FailureEOFError:
-			tk.BlockingFlags |= analysisFlagHTTPBlocking
+			tk.BlockingFlags |= AnalysisBlockingFlagHTTPBlocking
 			logger.Warnf(
 				"HTTP: unexpected failure %s for %s (see #%d)",
 				*failure,

internal/experiment/webconnectivitylte/analysishttpdiff.go

@@ -34,7 +34,7 @@ func (tk *TestKeys) analysisHTTPDiff(logger model.Logger,
 	}
 	if URL.Scheme == "https" {
 		logger.Infof("HTTP: HTTPS && no error => #%d is successful", probe.TransactionID)
-		tk.BlockingFlags |= analysisFlagSuccess
+		tk.BlockingFlags |= AnalysisBlockingFlagSuccess
 		return
 	}
 
@@ -50,7 +50,7 @@ func (tk *TestKeys) analysisHTTPDiff(logger model.Logger,
 				"HTTP: statusCodeMatch && bodyLengthMatch => #%d is successful",
 				probe.TransactionID,
 			)
-			tk.BlockingFlags |= analysisFlagSuccess
+			tk.BlockingFlags |= AnalysisBlockingFlagSuccess
 			return
 		}
 		logger.Infof("HTTP: body length: MISMATCH (see #%d)", probe.TransactionID)
@@ -59,7 +59,7 @@ func (tk *TestKeys) analysisHTTPDiff(logger model.Logger,
 				"HTTP: statusCodeMatch && headersMatch => #%d is successful",
 				probe.TransactionID,
 			)
-			tk.BlockingFlags |= analysisFlagSuccess
+			tk.BlockingFlags |= AnalysisBlockingFlagSuccess
 			return
 		}
 		logger.Infof("HTTP: uncommon headers: MISMATCH (see #%d)", probe.TransactionID)
@@ -68,15 +68,15 @@ func (tk *TestKeys) analysisHTTPDiff(logger model.Logger,
 				"HTTP: statusCodeMatch && titleMatch => #%d is successful",
 				probe.TransactionID,
 			)
-			tk.BlockingFlags |= analysisFlagSuccess
+			tk.BlockingFlags |= AnalysisBlockingFlagSuccess
 			return
 		}
 		logger.Infof("HTTP: title: MISMATCH (see #%d)", probe.TransactionID)
 	} else {
 		logger.Infof("HTTP: status code: MISMATCH (see #%d)", probe.TransactionID)
 	}
 
-	tk.BlockingFlags |= analysisFlagHTTPDiff
+	tk.BlockingFlags |= AnalysisBlockingFlagHTTPDiff
 	logger.Warnf("HTTP: it seems #%d is a case of httpDiff", probe.TransactionID)
 }
 

internal/experiment/webconnectivitylte/analysistcpip.go

@@ -21,7 +21,7 @@ import (
 // for the same set of IP addresses (it's ugly to modify a data struct
 // in place, but this algorithm is defined by the spec);
 //
-// 2. assign the analysisFlagTCPIPBlocking flag to XBlockingFlags if
+// 2. assign the AnalysisBlockingFlagTCPIPBlocking flag to XBlockingFlags if
 // we see any TCP endpoint for which Status.Blocked is true.
 func (tk *TestKeys) analysisTCPIPToplevel(logger model.Logger) {
 	// if we don't have a control result, do nothing.
@@ -77,6 +77,6 @@ func (tk *TestKeys) analysisTCPIPToplevel(logger model.Logger) {
 			entry.TransactionID,
 		)
 		entry.Status.Blocked = &istrue
-		tk.BlockingFlags |= analysisFlagTCPIPBlocking
+		tk.BlockingFlags |= AnalysisBlockingFlagTCPIPBlocking
 	}
 }

internal/experiment/webconnectivitylte/analysistls.go

@@ -45,6 +45,6 @@ func (tk *TestKeys) analysisTLSToplevel(logger model.Logger) {
 			epnt,
 			entry.TransactionID,
 		)
-		tk.BlockingFlags |= analysisFlagTLSBlocking
+		tk.BlockingFlags |= AnalysisBlockingFlagTLSBlocking
 	}
 }

internal/experiment/webconnectivityqa/badssl.go

@@ -92,8 +92,8 @@ func badSSLWithUnknownAuthorityWithInconsistentDNS() *TestCase {
 			DNSConsistency:        "inconsistent",
 			HTTPExperimentFailure: "ssl_unknown_authority",
 			XStatus:               9248, // StatusExperimentHTTP | StatusAnomalyTLSHandshake | StatusAnomalyDNS
-			XDNSFlags:             4,    // AnalysisDNSUnexpectedAddrs
-			XBlockingFlags:        33,   // analysisFlagSuccess | analysisFlagDNSBlocking
+			XDNSFlags:             4,    // AnalysisDNSFlagUnexpectedAddrs
+			XBlockingFlags:        33,   // AnalysisBlockingFlagSuccess | AnalysisBlockingFlagDNSBlocking
 			Accessible:            false,
 			Blocking:              "dns",
 		},

internal/experiment/webconnectivityqa/dnsblocking.go

@@ -24,8 +24,8 @@ func dnsBlockingAndroidDNSCacheNoData() *TestCase {
 			DNSExperimentFailure: "android_dns_cache_no_data",
 			DNSConsistency:       "inconsistent",
 			XStatus:              2080, // StatusExperimentDNS | StatusAnomalyDNS
-			XDNSFlags:            2,    // AnalysisDNSUnexpectedFailure
-			XBlockingFlags:       33,   // analysisFlagDNSBlocking | analysisFlagSuccess
+			XDNSFlags:            2,    // AnalysisDNSFlagUnexpectedFailure
+			XBlockingFlags:       33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
 			Accessible:           false,
 			Blocking:             "dns",
 		},
@@ -55,8 +55,8 @@ func dnsBlockingNXDOMAIN() *TestCase {
 			DNSExperimentFailure: "dns_nxdomain_error",
 			DNSConsistency:       "inconsistent",
 			XStatus:              2080, // StatusExperimentDNS | StatusAnomalyDNS
-			XDNSFlags:            2,    // AnalysisDNSUnexpectedFailure
-			XBlockingFlags:       33,   // analysisFlagDNSBlocking | analysisFlagSuccess
+			XDNSFlags:            2,    // AnalysisDNSFlagUnexpectedFailure
+			XBlockingFlags:       33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
 			Accessible:           false,
 			Blocking:             "dns",
 		},
@@ -79,8 +79,8 @@ func dnsBlockingBOGON() *TestCase {
 			DNSExperimentFailure:  nil,
 			DNSConsistency:        "inconsistent",
 			XStatus:               4256, // StatusExperimentConnect | StatusAnomalyConnect | StatusAnomalyDNS
-			XDNSFlags:             1,    // AnalysisDNSBogon
-			XBlockingFlags:        33,   // analysisFlagDNSBlocking | analysisFlagSuccess
+			XDNSFlags:             1,    // AnalysisFlagDNSBogon
+			XBlockingFlags:        33,   // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
 			Accessible:            false,
 			Blocking:              "dns",
 		},

internal/experiment/webconnectivityqa/dnshijacking.go

@@ -35,7 +35,7 @@ func dnsHijackingToProxyWithHTTPURL() *TestCase {
 			TitleMatch:      true,
 			XStatus:         2, // StatusSuccessCleartext
 			XDNSFlags:       0,
-			XBlockingFlags:  32, // analysisFlagSuccess
+			XBlockingFlags:  32, // AnalysisBlockingFlagSuccess
 			Accessible:      true,
 			Blocking:        false,
 		},
@@ -72,7 +72,7 @@ func dnsHijackingToProxyWithHTTPSURL() *TestCase {
 			TitleMatch:      true,
 			XStatus:         1, // StatusSuccessSecure
 			XDNSFlags:       0,
-			XBlockingFlags:  32, // analysisFlagSuccess
+			XBlockingFlags:  32, // AnalysisBlockingFlagSuccess
 			Accessible:      true,
 			Blocking:        false,
 		},

internal/experiment/webconnectivityqa/httpblocking.go

@@ -29,7 +29,7 @@ func httpBlockingConnectionReset() *TestCase {
 			// the HTTPExperimentFailure field, why?
 			HTTPExperimentFailure: "connection_reset",
 			XStatus:               8448, // StatusExperimentHTTP | StatusAnomalyReadWrite
-			XBlockingFlags:        8,    // analysisFlagHTTPBlocking
+			XBlockingFlags:        8,    // AnalysisBlockingFlagHTTPBlocking
 			Accessible:            false,
 			Blocking:              "http-failure",
 		},

internal/experiment/webconnectivityqa/httpdiff.go

@@ -37,7 +37,7 @@ func httpDiffWithConsistentDNS() *TestCase {
 			HTTPExperimentFailure: nil,
 			XStatus:               64, // StatusAnomalyHTTPDiff
 			XDNSFlags:             0,
-			XBlockingFlags:        16, // analysisFlagHTTPDiff
+			XBlockingFlags:        16, // AnalysisBlockingFlagHTTPDiff
 			Accessible:            false,
 			Blocking:              "http-diff",
 		},
@@ -91,8 +91,8 @@ func httpDiffWithInconsistentDNS() *TestCase {
 			HeadersMatch:          false,
 			TitleMatch:            false,
 			XStatus:               96, // StatusAnomalyHTTPDiff | StatusAnomalyDNS
-			XDNSFlags:             4,  // AnalysisDNSUnexpectedAddrs
-			XBlockingFlags:        35, // analysisFlagSuccess | analysisFlagDNSBlocking | analysisFlagTCPIPBlocking
+			XDNSFlags:             4,  // AnalysisDNSFlagUnexpectedAddrs
+			XBlockingFlags:        35, // AnalysisBlockingFlagSuccess | AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagTCPIPBlocking
 			Accessible:            false,
 			Blocking:              "dns",
 		},