fix(telegram): stop measuring http://web.telegram.org/ (#999)

While there also stop processing the page that we get back when using https like we do for whatsapp. Generally, this set of changes is proactive to reduce possible false positives in the future. See ooni/probe#2382.
ooni · Dec 7, 2022 · fccb821 · fccb821
1 parent e7f0755
commit fccb821
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 133 deletions.
diff --git a/internal/engine/experiment/telegram/telegram.go b/internal/engine/experiment/telegram/telegram.go
@@ -6,7 +6,6 @@ package telegram
 import (
 	"context"
 	"errors"
-	"strings"
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter"
@@ -16,7 +15,7 @@ import (
 
 const (
 	testName    = "telegram"
-	testVersion = "0.2.0"
+	testVersion = "0.3.0"
 )
 
 // Config contains the telegram experiment config.
@@ -61,23 +60,11 @@ func (tk *TestKeys) Update(v urlgetter.MultiOutput) {
 		}
 		return
 	}
-	// now take care of web
-	if tk.TelegramWebStatus != "ok" {
-		return // we already flipped the state
-	}
 	if v.TestKeys.Failure != nil {
 		tk.TelegramWebStatus = "blocked"
 		tk.TelegramWebFailure = v.TestKeys.Failure
 		return
 	}
-	title := `<title>Telegram Web</title>`
-	if strings.Contains(v.TestKeys.HTTPResponseBody, title) == false {
-		failureString := "telegram_missing_title_error"
-		tk.TelegramWebFailure = &failureString
-		tk.TelegramWebStatus = "blocked"
-		return
-	}
-	return
 }
 
 // Measurer performs the measurement
@@ -118,7 +105,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 		{Target: "http://95.161.76.100/", Config: urlgetter.Config{Method: "POST"}},
 
 		// Note: the following list contains the same endpoints as above with HTTP (not a typo using
-		// https would not work here) _and_ port 443.
+		// https _would not work_ here) _and_ port 443.
 		{Target: "http://149.154.175.50:443/", Config: urlgetter.Config{Method: "POST"}},
 		{Target: "http://149.154.167.51:443/", Config: urlgetter.Config{Method: "POST"}},
 		{Target: "http://149.154.175.100:443/", Config: urlgetter.Config{Method: "POST"}},
@@ -128,13 +115,8 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 
 		// Here we need to provide the method explicitly. See
 		// https://github.com/ooni/probe-engine/issues/827.
-		{Target: "http://web.telegram.org/", Config: urlgetter.Config{
-			Method:          "GET",
-			FailOnHTTPError: true,
-		}},
 		{Target: "https://web.telegram.org/", Config: urlgetter.Config{
-			Method:          "GET",
-			FailOnHTTPError: true,
+			Method: "GET",
 		}},
 	}
 	multi := urlgetter.Multi{Begin: time.Now(), Getter: m.Getter, Session: sess}

diff --git a/internal/engine/experiment/telegram/telegram_test.go b/internal/engine/experiment/telegram/telegram_test.go
@@ -6,8 +6,6 @@ import (
 	"io"
 	"testing"
 
-	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/atomicx"
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/telegram"
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter"
 	"github.com/ooni/probe-cli/v3/internal/engine/mockable"
@@ -20,7 +18,7 @@ func TestNewExperimentMeasurer(t *testing.T) {
 	if measurer.ExperimentName() != "telegram" {
 		t.Fatal("unexpected name")
 	}
-	if measurer.ExperimentVersion() != "0.2.0" {
+	if measurer.ExperimentVersion() != "0.3.0" {
 		t.Fatal("unexpected version")
 	}
 }
@@ -29,10 +27,10 @@ func TestGood(t *testing.T) {
 	measurer := telegram.NewExperimentMeasurer(telegram.Config{})
 	measurement := new(model.Measurement)
 	args := &model.ExperimentArgs{
-		Callbacks:   model.NewPrinterCallbacks(log.Log),
+		Callbacks:   model.NewPrinterCallbacks(model.DiscardLogger),
 		Measurement: measurement,
 		Session: &mockable.Session{
-			MockableLogger: log.Log,
+			MockableLogger: model.DiscardLogger,
 		},
 	}
 	err := measurer.Run(context.Background(), args)
@@ -234,114 +232,24 @@ func TestUpdateWithAllConnectsFailed(t *testing.T) {
 	}
 }
 
-func TestUpdateWebWithMixedResults(t *testing.T) {
+func TestUpdateWithWebFailure(t *testing.T) {
+	failure := netxlite.FailureEOFError
+	failedOperation := netxlite.TLSHandshakeOperation
 	tk := telegram.NewTestKeys()
-	tk.Update(urlgetter.MultiOutput{
-		Input: urlgetter.MultiInput{
-			Config: urlgetter.Config{Method: "GET"},
-			Target: "http://web.telegram.org/",
-		},
-		TestKeys: urlgetter.TestKeys{
-			FailedOperation: (func() *string {
-				s := netxlite.HTTPRoundTripOperation
-				return &s
-			})(),
-			Failure: (func() *string {
-				s := netxlite.FailureEOFError
-				return &s
-			})(),
-		},
-	})
 	tk.Update(urlgetter.MultiOutput{
 		Input: urlgetter.MultiInput{
 			Config: urlgetter.Config{Method: "GET"},
 			Target: "https://web.telegram.org/",
 		},
 		TestKeys: urlgetter.TestKeys{
-			HTTPResponseBody:   `<title>Telegram Web</title>`,
-			HTTPResponseStatus: 200,
-		},
-	})
-	if tk.TelegramWebStatus != "blocked" {
-		t.Fatal("TelegramWebStatus should be blocked")
-	}
-	if *tk.TelegramWebFailure != netxlite.FailureEOFError {
-		t.Fatal("invalid TelegramWebFailure")
-	}
-}
-
-func TestWeConfigureWebChecksToFailOnHTTPError(t *testing.T) {
-	called := &atomicx.Int64{}
-	failOnErrorHTTPS := &atomicx.Int64{}
-	failOnErrorHTTP := &atomicx.Int64{}
-	measurer := telegram.Measurer{
-		Config: telegram.Config{},
-		Getter: func(ctx context.Context, g urlgetter.Getter) (urlgetter.TestKeys, error) {
-			called.Add(1)
-			switch g.Target {
-			case "https://web.telegram.org/":
-				if g.Config.FailOnHTTPError {
-					failOnErrorHTTPS.Add(1)
-				}
-			case "http://web.telegram.org/":
-				if g.Config.FailOnHTTPError {
-					failOnErrorHTTP.Add(1)
-				}
-			}
-			return urlgetter.DefaultMultiGetter(ctx, g)
-		},
-	}
-	ctx := context.Background()
-	sess := &mockable.Session{
-		MockableLogger: log.Log,
-	}
-	measurement := new(model.Measurement)
-	callbacks := model.NewPrinterCallbacks(log.Log)
-	args := &model.ExperimentArgs{
-		Callbacks:   callbacks,
-		Measurement: measurement,
-		Session:     sess,
-	}
-	if err := measurer.Run(ctx, args); err != nil {
-		t.Fatal(err)
-	}
-	if called.Load() < 1 {
-		t.Fatal("not called")
-	}
-	if failOnErrorHTTPS.Load() != 1 {
-		t.Fatal("not configured fail on error for HTTPS")
-	}
-	if failOnErrorHTTP.Load() != 1 {
-		t.Fatal("not configured fail on error for HTTP")
-	}
-}
-
-func TestUpdateWithMissingTitle(t *testing.T) {
-	tk := telegram.NewTestKeys()
-	tk.Update(urlgetter.MultiOutput{
-		Input: urlgetter.MultiInput{
-			Config: urlgetter.Config{Method: "GET"},
-			Target: "http://web.telegram.org/",
-		},
-		TestKeys: urlgetter.TestKeys{
-			HTTPResponseStatus: 200,
-			HTTPResponseBody:   "<HTML><title>Telegram Web</title></HTML>",
-		},
-	})
-	tk.Update(urlgetter.MultiOutput{
-		Input: urlgetter.MultiInput{
-			Config: urlgetter.Config{Method: "GET"},
-			Target: "http://web.telegram.org/",
-		},
-		TestKeys: urlgetter.TestKeys{
-			HTTPResponseStatus: 200,
-			HTTPResponseBody:   "<HTML><title>Antani Web</title></HTML>",
+			Failure:         &failure,
+			FailedOperation: &failedOperation,
 		},
 	})
 	if tk.TelegramWebStatus != "blocked" {
 		t.Fatal("TelegramWebStatus should be blocked")
 	}
-	if *tk.TelegramWebFailure != "telegram_missing_title_error" {
+	if *tk.TelegramWebFailure != failure {
 		t.Fatal("invalid TelegramWebFailure")
 	}
 }
@@ -351,17 +259,7 @@ func TestUpdateWithAllGood(t *testing.T) {
 	tk.Update(urlgetter.MultiOutput{
 		Input: urlgetter.MultiInput{
 			Config: urlgetter.Config{Method: "GET"},
-			Target: "http://web.telegram.org/",
-		},
-		TestKeys: urlgetter.TestKeys{
-			HTTPResponseStatus: 200,
-			HTTPResponseBody:   "<HTML><title>Telegram Web</title></HTML>",
-		},
-	})
-	tk.Update(urlgetter.MultiOutput{
-		Input: urlgetter.MultiInput{
-			Config: urlgetter.Config{Method: "GET"},
-			Target: "http://web.telegram.org/",
+			Target: "https://web.telegram.org/",
 		},
 		TestKeys: urlgetter.TestKeys{
 			HTTPResponseStatus: 200,