[backport] refactor(stunreachability): input required and must be an …

…URL (#630)

Here we're refactoring stunreachability to not provide internally a
default input and to take in input an URL rather than a string.

The related ooni/spec change is ooni/spec#227.

This diff has been extracted from #539.

Because the original diff was large, I'm splitting it in a set of
more easily manageable diffs.

The reference issue is ooni/probe#1814, which
is complex enough to require us to proceed incrementally.

This diff WILL need to be backported to release/3.11.

internal/engine/allexperiments.go

@@ -198,7 +198,7 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{
 				))
 			},
 			config:      &stunreachability.Config{},
-			inputPolicy: InputOptional,
+			inputPolicy: InputStrictlyRequired,
 		}
 	},
 

internal/engine/experiment/stunreachability/fake_test.go

@@ -6,6 +6,9 @@ import (
 	"time"
 )
 
+// TODO(bassosimone): we should use internal/netxlite/mocks rather
+// than rolling out a custom type private to this package.
+
 type FakeConn struct {
 	ReadError             error
 	ReadData              []byte

internal/engine/experiment/stunreachability/stunreachability.go

@@ -5,8 +5,10 @@ package stunreachability
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
+	"net/url"
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx"
@@ -20,7 +22,7 @@ import (
 
 const (
 	testName    = "stunreachability"
-	testVersion = "0.2.0"
+	testVersion = "0.3.0"
 )
 
 // Config contains the experiment config.
@@ -64,6 +66,15 @@ func wrap(err error) error {
 	}.MaybeBuild()
 }
 
+// errStunMissingInput means that the user did not provide any input
+var errStunMissingInput = errors.New("stun: missing input")
+
+// errStunMissingPortInURL means the URL is missing the port
+var errStunMissingPortInURL = errors.New("stun: missing port in URL")
+
+// errUnsupportedURLScheme means we don't support the URL scheme
+var errUnsupportedURLScheme = errors.New("stun: unsupported URL scheme")
+
 // Run implements ExperimentMeasurer.Run.
 func (m *Measurer) Run(
 	ctx context.Context, sess model.ExperimentSession,
@@ -72,7 +83,21 @@ func (m *Measurer) Run(
 	tk := new(TestKeys)
 	measurement.TestKeys = tk
 	registerExtensions(measurement)
-	if err := wrap(tk.run(ctx, m.config, sess, measurement, callbacks)); err != nil {
+	input := string(measurement.Input)
+	if input == "" {
+		return errStunMissingInput
+	}
+	URL, err := url.Parse(input)
+	if err != nil {
+		return err
+	}
+	if URL.Port() == "" {
+		return errStunMissingPortInURL
+	}
+	if URL.Scheme != "stun" {
+		return errUnsupportedURLScheme
+	}
+	if err := wrap(tk.run(ctx, m.config, sess, measurement, callbacks, URL.Host)); err != nil {
 		s := err.Error()
 		tk.Failure = &s
 		return err
@@ -83,12 +108,8 @@ func (m *Measurer) Run(
 func (tk *TestKeys) run(
 	ctx context.Context, config Config, sess model.ExperimentSession,
 	measurement *model.Measurement, callbacks model.ExperimentCallbacks,
+	endpoint string,
 ) error {
-	const defaultAddress = "stun.l.google.com:19302"
-	endpoint := string(measurement.Input)
-	if endpoint == "" {
-		endpoint = defaultAddress
-	}
 	callbacks.OnProgress(0, fmt.Sprintf("stunreachability: measuring: %s...", endpoint))
 	defer callbacks.OnProgress(
 		1, fmt.Sprintf("stunreachability: measuring: %s... done", endpoint))