fix(webconnectivitylte): include client_resolver (ooni#1504)

Closes ooni/probe#2676

internal/experiment/webconnectivitylte/measurer.go

@@ -78,6 +78,11 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 	tk := NewTestKeys()
 	measurement.TestKeys = tk
 
+	// make sure we add the ClientResolver field
+	//
+	// See https://github.com/ooni/probe/issues/2676
+	tk.ClientResolver = sess.ResolverIP()
+
 	// create variables required to run parallel tasks
 	idGenerator := &atomic.Int64{}
 	wg := &sync.WaitGroup{}

internal/webconnectivityqa/checker.go

@@ -2,12 +2,12 @@ package webconnectivityqa
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/must"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
-	"github.com/ooni/probe-cli/v3/internal/x/dslx"
 )
 
 // Checker checks whether a measurement is correct.
@@ -26,6 +26,10 @@ var ErrCheckerNoReadWriteEvents = errors.New("no read or write events")
 // ErrCheckerUnexpectedWebConnectivityVersion indicates that the version is unexpected
 var ErrCheckerUnexpectedWebConnectivityVersion = errors.New("unexpected Web Connectivity version")
 
+type readWriteEventsExistentialCheckerTestKeys struct {
+	NetworkEvents []*model.ArchivalNetworkEvent `json:"network_events"`
+}
+
 // Check implements Checker.
 func (*ReadWriteEventsExistentialChecker) Check(mx *model.Measurement) error {
 	// we don't care about v0.4
@@ -39,7 +43,7 @@ func (*ReadWriteEventsExistentialChecker) Check(mx *model.Measurement) error {
 	}
 
 	// serialize and reparse the test keys
-	var tk *dslx.Observations
+	var tk *readWriteEventsExistentialCheckerTestKeys
 	must.UnmarshalJSON(must.MarshalJSON(mx.TestKeys), &tk)
 
 	// count the read/write events
@@ -59,3 +63,33 @@ func (*ReadWriteEventsExistentialChecker) Check(mx *model.Measurement) error {
 	}
 	return nil
 }
+
+// ClientResolverCorrectnessChecker checks whether the client_resolver field
+// inside of the test_keys has been correctly configured.
+type ClientResolverCorrectnessChecker struct{}
+
+var _ Checker = &ClientResolverCorrectnessChecker{}
+
+type clientResolverCorrectnessCheckerTestKeys struct {
+	ClientResolver string `json:"client_resolver"`
+}
+
+// ErrCheckerInvalidClientResolver indicates that the client_resolver field is invalid.
+var ErrCheckerInvalidClientResolver = errors.New("invalid client_resolver field")
+
+// Check implements Checker.
+func (*ClientResolverCorrectnessChecker) Check(mx *model.Measurement) error {
+	var tk *clientResolverCorrectnessCheckerTestKeys
+	must.UnmarshalJSON(must.MarshalJSON(mx.TestKeys), &tk)
+
+	if mx.ResolverIP != tk.ClientResolver {
+		return fmt.Errorf(
+			"%w: expected '%s', got '%s'",
+			ErrCheckerInvalidClientResolver,
+			mx.ResolverIP,
+			tk.ClientResolver,
+		)
+	}
+
+	return nil
+}

internal/webconnectivityqa/checker_test.go

@@ -3,12 +3,14 @@ package webconnectivityqa_test
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 
 	"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivitylte"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/must"
+	"github.com/ooni/probe-cli/v3/internal/netemx"
 	"github.com/ooni/probe-cli/v3/internal/optional"
 	"github.com/ooni/probe-cli/v3/internal/webconnectivityqa"
 )
@@ -110,3 +112,64 @@ func TestReadWriteEventsExistentialChecker(t *testing.T) {
 		})
 	}
 }
+
+func TestClientResolverCorrectnessChecker(t *testing.T) {
+	type testcase struct {
+		name   string
+		tk     string
+		expect error
+	}
+
+	cases := []testcase{{
+		name:   "with correct value",
+		tk:     fmt.Sprintf(`{"client_resolver":"%s"}`, netemx.ISPResolverAddress),
+		expect: nil,
+	}, {
+		name: "with empty",
+		tk:   `{"client_resolver":""}`,
+		expect: fmt.Errorf(
+			"%w: expected '%s', got ''",
+			webconnectivityqa.ErrCheckerInvalidClientResolver,
+			netemx.ISPResolverAddress,
+		),
+	}, {
+		name: "with different value",
+		tk:   `{"client_resolver":"10.0.0.1"}`,
+		expect: fmt.Errorf(
+			"%w: expected '%s', got '%s'",
+			webconnectivityqa.ErrCheckerInvalidClientResolver,
+			netemx.ISPResolverAddress,
+			"10.0.0.1",
+		),
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var tks map[string]any
+			must.UnmarshalJSON([]byte(tc.tk), &tks)
+
+			meas := &model.Measurement{
+				TestKeys:   tks,
+				ResolverIP: netemx.ISPResolverAddress,
+			}
+
+			err := (&webconnectivityqa.ClientResolverCorrectnessChecker{}).Check(meas)
+
+			switch {
+			case tc.expect == nil && err == nil:
+				return
+
+			case tc.expect == nil && err != nil:
+				t.Fatal("expected", tc.expect, "got", err)
+
+			case tc.expect != nil && err == nil:
+				t.Fatal("expected", tc.expect, "got", err)
+
+			case tc.expect != nil && err != nil:
+				if err.Error() != tc.expect.Error() {
+					t.Fatal("expected", tc.expect, "got", err)
+				}
+			}
+		})
+	}
+}

internal/webconnectivityqa/success.go

@@ -1,5 +1,13 @@
 package webconnectivityqa
 
+var successCheckers = []Checker{
+	// See https://github.com/ooni/probe/issues/2674
+	&ReadWriteEventsExistentialChecker{},
+
+	// See https://github.com/ooni/probe/issues/2676
+	&ClientResolverCorrectnessChecker{},
+}
+
 // successWithHTTP ensures we can successfully measure an HTTP URL.
 func successWithHTTP() *TestCase {
 	return &TestCase{
@@ -20,10 +28,7 @@ func successWithHTTP() *TestCase {
 			Accessible:      true,
 			Blocking:        false,
 		},
-		Checkers: []Checker{
-			// See https://github.com/ooni/probe/issues/2674
-			&ReadWriteEventsExistentialChecker{},
-		},
+		Checkers: successCheckers,
 	}
 }
 
@@ -47,5 +52,6 @@ func successWithHTTPS() *TestCase {
 			Accessible:      true,
 			Blocking:        false,
 		},
+		Checkers: successCheckers,
 	}
 }