fix(dnscheck): gracefully cast to Target (#1623)

Closes ooni/probe#2753

docs/design/dd-008-richer-input.md

@@ -33,7 +33,6 @@ the following command runs the `dnscheck` experiment measuring
 ./miniooni dnscheck -i https://8.8.8.8/dns-query -O HTTP3Enabled=true
 ```
 
-
 Additionally, OONI Run v2 allows to run experiments with options. For example,
 the following JSON document is equivalent to the previous `miniooni` command:
 
@@ -151,16 +150,17 @@ Solving this problem is crucial because most OONI measurements run automatically
 in the background with input provided by the backend. Therefore, by enabling
 richer input, we open up the possibility of answering specific research questions
 requiring options at scale. For example, richer input would enable us to
-study [DNS over HTTP/3 is blocking](https://github.com/ooni/probe/issues/2675))
+study [DNS over HTTP/3 blocking](https://github.com/ooni/probe/issues/2675))
 at scale.
 
 ## Design choice: deprecating the check-in API
 
 We originally envisioned distributing richer input through the check-in API
 but we later realized that this design would be problematic because:
 
-1. it prevents us from having experiments implemented as scripts, a solution
-that we heavily explored while researching richer input;
+1. it prevents us from having experiments implemented as scripts, [a solution
+that we heavily explored while researching richer input](
+https://github.com/bassosimone/2023-12-09-ooni-javascript);
 
 2. the check-in API serves URLs for Web Connectivity, which is the most
 important experiment we run, which means that changing the component serving
@@ -343,12 +343,24 @@ the changes roughly look like this:
 +	if args.Target == nil {
 +		return ErrInputRequired
 +	}
-+	input := args.Target.(*target).input
++	input, ok := args.Target.(*target).input
++	if !ok {
++		return ErrInvalidInputType
++	}
 +	options := args.Target.(*target).options
 	// [...]
  }
 ```
 
+Note how we MUST gracefully cast to `*target` (as we did in [probe-cli#1623](
+https://github.com/ooni/probe-cli/pull/1623)) because richer input could
+potentially come from ~any source, including the mobile app. While richer input
+is anything that fullfills the `model.ExperimentTarget` interface, mobile apps
+could, for example, construct a Java class implementing such an interface but we
+wouldn't be able to cast such an interface to the `*target` type. Therefore,
+unconditionally casting could lead to crashes when integrating new code
+and generally makes for a less robust codebase.
+
 ## Next steps
 
 This is a rough sequence of next steps that we should expand as we implement
@@ -387,5 +399,3 @@ inside of the `targetloader` package to have multiple target loaders.
 
 *  devise long term strategy for delivering richer input to `oonimkall`
 from mobile apps, which we'll need as soon as we convert the IM experiments
-
-

internal/experiment/dnscheck/dnscheck.go

@@ -115,6 +115,7 @@ func (m *Measurer) ExperimentVersion() string {
 // that are used by this experiment to implement its functionality.
 var (
 	ErrInputRequired        = targetloading.ErrInputRequired
+	ErrInvalidInputType     = targetloading.ErrInvalidInputType
 	ErrInvalidURL           = errors.New("the input URL is invalid")
 	ErrUnsupportedURLScheme = errors.New("unsupported URL scheme")
 )
@@ -125,11 +126,14 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
 	measurement := args.Measurement
 	sess := args.Session
 
-	// 0. obtain the richer input target, config, and input or panic
+	// 0. obtain the richer input target, config, and input or fail
 	if args.Target == nil {
 		return ErrInputRequired
 	}
-	target := args.Target.(*Target)
+	target, ok := args.Target.(*Target)
+	if !ok {
+		return ErrInvalidInputType
+	}
 	config, input := target.Options, target.URL
 	sess.Logger().Infof("dnscheck: using richer input: %+v %+v", config, input)
 

internal/experiment/dnscheck/dnscheck_test.go

@@ -50,6 +50,20 @@ func TestExperimentNameAndVersion(t *testing.T) {
 	}
 }
 
+func TestDNSCheckFailsWithInvalidInputType(t *testing.T) {
+	measurer := NewExperimentMeasurer()
+	args := &model.ExperimentArgs{
+		Callbacks:   model.NewPrinterCallbacks(log.Log),
+		Measurement: new(model.Measurement),
+		Session:     newsession(),
+		Target:      &model.OOAPIURLInfo{}, // not the expected input type
+	}
+	err := measurer.Run(context.Background(), args)
+	if !errors.Is(err, ErrInvalidInputType) {
+		t.Fatal("expected no input error")
+	}
+}
+
 func TestDNSCheckFailsWithoutInput(t *testing.T) {
 	measurer := NewExperimentMeasurer()
 	args := &model.ExperimentArgs{

internal/targetloading/targetloading.go

@@ -24,6 +24,7 @@ var (
 	ErrInputRequired     = errors.New("no input provided")
 	ErrNoInputExpected   = errors.New("we did not expect any input")
 	ErrNoStaticInput     = errors.New("no static input for this experiment")
+	ErrInvalidInputType  = errors.New("invalid richer input type")
 )
 
 // Session is the session according to a [*Loader] instance.