From 2dd4e75945177381970490df4a7fd2fab054d465 Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Sun, 11 Sep 2022 22:48:28 +0200
Subject: [PATCH] fix(webconnectivity@v0.5): status code always match with
 equal codes (#951)

See https://github.com/ooni/probe/issues/2287
---
 .../webconnectivity/analysishttpdiff.go       | 23 ++++++++++++++++---
 .../experiment/webconnectivity/measurer.go    |  2 +-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/internal/experiment/webconnectivity/analysishttpdiff.go b/internal/experiment/webconnectivity/analysishttpdiff.go
index 2eb3916..f65b388 100644
--- a/internal/experiment/webconnectivity/analysishttpdiff.go
+++ b/internal/experiment/webconnectivity/analysishttpdiff.go
@@ -112,10 +112,27 @@ func (tk *TestKeys) httpDiffStatusCodeMatch(
 	if measurement <= 0 {
 		return // no real status code
 	}
-	if control/100 != 2 {
-		return // avoid comparison if it seems the TH failed
-	}
 	good := control == measurement
+	if !good && control/100 != 2 {
+		// Avoid comparison if it seems the TH failed _and_ the two
+		// status codes are not equal. Originally, this algorithm was
+		// https://github.com/measurement-kit/measurement-kit/blob/b55fbecb205be62c736249b689df0c45ae342804/src/libmeasurement_kit/ooni/web_connectivity.cpp#L60
+		// and excluded the case where the TH failed with 5xx.
+		//
+		// Then, we discovered when implementing websteps a bunch
+		// of control failure modes that suggested to be more
+		// cautious. See https://github.com/bassosimone/websteps-illustrated/blob/632f27443ab9d94fb05efcf5e0b0c1ce190221e2/internal/engine/experiment/websteps/analysisweb.go#L137.
+		//
+		// However, it seems a bit retarded to avoid comparison
+		// when both the TH and the probe failed equallty. See
+		// https://github.com/ooni/probe/issues/2287, which refers
+		// to a measurement where both the probe and the TH fail
+		// with 404, but we fail to say "status_code_match = true".
+		//
+		// See https://explorer.ooni.org/measurement/20220911T203447Z_webconnectivity_IT_30722_n1_YDZQZOHAziEJk6o9?input=http%3A%2F%2Fwww.webbox.com%2Findex.php
+		// for a measurement where this was fixed.
+		return
+	}
 	tk.StatusCodeMatch = &good
 }
 
diff --git a/internal/experiment/webconnectivity/measurer.go b/internal/experiment/webconnectivity/measurer.go
index 207dd1e..6e084c1 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.8"
+	return "0.5.9"
 }
 
 // Run implements model.ExperimentMeasurer.