feat: extend port-filtering experiment

internal/cmd/ooporthelper/main.go

@@ -9,7 +9,7 @@ import (
 	"time"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/engine/experiment/portfiltering"
+	"github.com/ooni/probe-cli/v3/internal/engine"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
@@ -64,7 +64,7 @@ func main() {
 	flag.Parse()
 	log.SetLevel(logmap[*debug])
 	defer srvCancel()
-	ports := portfiltering.Ports
+	ports := engine.PortFilteringDefaultInput
 	if srvTest {
 		ports = TestPorts
 	}

internal/engine/experiment/portfiltering/config.go

@@ -4,17 +4,15 @@ package portfiltering
 // Config for the port-filtering experiment
 //
 
-import "time"
-
 // Config contains the experiment configuration.
 type Config struct {
-	// Delay is the delay between each repetition (in milliseconds).
-	Delay int64 `ooni:"number of milliseconds to wait before testing each port"`
+	// TestHelper is the URL to use for port-scanning
+	TestHelper string `ooni:"testhelper URL for port scanning"`
 }
 
-func (c *Config) delay() time.Duration {
-	if c.Delay > 0 {
-		return time.Duration(c.Delay) * time.Millisecond
+func (c *Config) testhelper() string {
+	if c.TestHelper != "" {
+		return c.TestHelper
 	}
-	return 100 * time.Millisecond
+	return "http://127.0.0.1"
 }

internal/engine/experiment/portfiltering/config_test.go

@@ -2,12 +2,11 @@ package portfiltering
 
 import (
 	"testing"
-	"time"
 )
 
 func TestConfig_delay(t *testing.T) {
 	c := Config{}
-	if c.delay() != 100*time.Millisecond {
-		t.Fatal("invalid default delay")
+	if c.testhelper() != "http://127.0.0.1" {
+		t.Fatal("invalid default testhelper")
 	}
 }

internal/engine/experiment/portfiltering/measurer.go

@@ -7,7 +7,9 @@ package portfiltering
 import (
 	"context"
 	"errors"
+	"net"
 	"net/url"
+	"strconv"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
@@ -33,6 +35,12 @@ func (m *Measurer) ExperimentVersion() string {
 }
 
 var (
+	// errInputRequired indicates that no input was provided
+	errInputRequired = errors.New("this experiment needs input")
+
+	// errInvalidInput indicates an invalid port number
+	errInvalidInput = errors.New("port number is invalid")
+
 	// errInvalidTestHelper indicates that the given test helper is not an URL
 	errInvalidTestHelper = errors.New("testhelper is not an URL")
 )
@@ -44,21 +52,26 @@ func (m *Measurer) Run(
 	measurement *model.Measurement,
 	callbacks model.ExperimentCallbacks,
 ) error {
+	input := string(measurement.Input)
+	if input == "" {
+		return errInputRequired
+	}
+	port, err := strconv.Atoi(input)
+	if err != nil || port >= 65536 || port < 0 {
+		return errInvalidInput
+	}
 	// TODO(DecFox): Replace the localhost deployment with an OONI testhelper
 	// Ensure that we only do this once we have a deployed testhelper
-	testhelper := "http://127.0.0.1"
-	parsed, err := url.Parse(testhelper)
+	th := m.config.testhelper()
+	parsed, err := url.Parse(th)
 	if err != nil {
 		return errInvalidTestHelper
 	}
 	tk := new(TestKeys)
 	measurement.TestKeys = tk
-	out := make(chan *model.ArchivalTCPConnectResult)
-	go m.tcpConnectLoop(ctx, measurement.MeasurementStartTimeSaved, sess.Logger(), parsed.Host, out)
-	for len(tk.TCPConnect) < len(Ports) {
-		tk.TCPConnect = append(tk.TCPConnect, <-out)
-	}
-	return nil // return nil so we always submit the measurement
+	addr := net.JoinHostPort(parsed.Hostname(), input)
+	m.tcpConnect(ctx, int64(0), measurement.MeasurementStartTimeSaved, sess.Logger(), tk, addr)
+	return nil
 }
 
 // NewExperimentMeasurer creates a new ExperimentMeasurer.

internal/engine/experiment/portfiltering/measurer_test.go

@@ -2,10 +2,15 @@ package portfiltering
 
 import (
 	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
 	"testing"
 
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/model/mocks"
+	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
 
 func TestMeasurerExperimentNameVersion(t *testing.T) {
@@ -18,31 +23,136 @@ func TestMeasurerExperimentNameVersion(t *testing.T) {
 	}
 }
 
-// TODO(DecFox): Skip this test with -short in a future iteration.
 func TestMeasurer_run(t *testing.T) {
-	m := NewExperimentMeasurer(Config{})
-	meas := &model.Measurement{}
-	sess := &mocks.Session{
-		MockLogger: func() model.Logger {
-			return model.DiscardLogger
-		},
-	}
-	callbacks := model.NewPrinterCallbacks(model.DiscardLogger)
-	ctx := context.Background()
-	err := m.Run(ctx, sess, meas, callbacks)
-	if err != nil {
-		t.Fatal(err)
-	}
-	tk := meas.TestKeys.(*TestKeys)
-	if len(tk.TCPConnect) != len(Ports) {
-		t.Fatal("unexpected number of ports")
-	}
-	ask, err := m.GetSummaryKeys(meas)
-	if err != nil {
-		t.Fatal("cannot obtain summary")
-	}
-	summary := ask.(SummaryKeys)
-	if summary.IsAnomaly {
-		t.Fatal("expected no anomaly")
+	runHelper := func(ctx context.Context, input string, url string) (*model.Measurement, model.ExperimentMeasurer, error) {
+		m := NewExperimentMeasurer(Config{
+			TestHelper: url,
+		})
+		meas := &model.Measurement{
+			Input: model.MeasurementTarget(input),
+		}
+		sess := &mocks.Session{
+			MockLogger: func() model.Logger {
+				return model.DiscardLogger
+			},
+		}
+		callbacks := model.NewPrinterCallbacks(model.DiscardLogger)
+		err := m.Run(ctx, sess, meas, callbacks)
+		return meas, m, err
 	}
+
+	t.Run("with no input", func(t *testing.T) {
+		ctx := context.Background()
+		_, _, err := runHelper(ctx, "", "")
+		if err == nil || err != errInputRequired {
+			t.Fatal("unexpected error")
+		}
+	})
+
+	t.Run("with invalid input", func(t *testing.T) {
+		t.Run("with negative port number", func(t *testing.T) {
+			ctx := context.Background()
+			_, _, err := runHelper(ctx, "-1", "")
+			if err == nil || err != errInvalidInput {
+				t.Fatal(err)
+			}
+		})
+
+		t.Run("with large invalid port number", func(t *testing.T) {
+			ctx := context.Background()
+			_, _, err := runHelper(ctx, "70000", "")
+			if err == nil || err != errInvalidInput {
+				t.Fatal(err)
+			}
+		})
+
+		t.Run("with non-integer port number", func(t *testing.T) {
+			ctx := context.Background()
+			_, _, err := runHelper(ctx, "\t", "")
+			if err == nil || err != errInvalidInput {
+				t.Fatal(err)
+			}
+		})
+	})
+
+	// TODO(DecFox): Add a test that checks ports on the OONI API
+	t.Run("with API testhelper", func(t *testing.T) {
+	})
+
+	t.Run("with local listener and successful outcome", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(200)
+		}))
+		defer server.Close()
+		URL, err := url.Parse(server.URL)
+		if err != nil {
+			t.Fatal(err)
+		}
+		ctx := context.Background()
+		meas, m, err := runHelper(ctx, URL.Port(), URL.String())
+		if err != nil {
+			t.Fatal(err)
+		}
+		ask, err := m.GetSummaryKeys(meas)
+		if err != nil {
+			t.Fatal("cannot obtain summary")
+		}
+		summary := ask.(SummaryKeys)
+		if summary.IsAnomaly {
+			t.Fatal("expected no anomaly")
+		}
+
+		t.Run("testkeys", func(t *testing.T) {
+			tk := meas.TestKeys.(*TestKeys)
+			port, _ := strconv.Atoi(URL.Port())
+			if tk.TCPConnect.IP != URL.Hostname() {
+				t.Fatal("unexpected target IP")
+			}
+			if tk.TCPConnect.Port != port {
+				t.Fatal("unexpected port")
+			}
+			if tk.TCPConnect.Status.Failure != nil {
+				t.Fatal("unexpected error")
+			}
+		})
+	})
+
+	t.Run("with local listener and cancel", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(200)
+		}))
+		defer server.Close()
+		URL, err := url.Parse(server.URL)
+		if err != nil {
+			t.Fatal(err)
+		}
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		meas, m, err := runHelper(ctx, URL.Port(), URL.String())
+		if err != nil {
+			t.Fatal(err)
+		}
+		ask, err := m.GetSummaryKeys(meas)
+		if err != nil {
+			t.Fatal("cannot obtain summary")
+		}
+		summary := ask.(SummaryKeys)
+		if summary.IsAnomaly {
+			t.Fatal("expected no anomaly")
+		}
+
+		t.Run("testkeys", func(t *testing.T) {
+			tk := meas.TestKeys.(*TestKeys)
+			port, _ := strconv.Atoi(URL.Port())
+			if tk.TCPConnect.IP != URL.Hostname() {
+				t.Fatal("unexpected target IP")
+			}
+			if tk.TCPConnect.Port != port {
+				t.Fatal("unexpected port")
+			}
+			if *tk.TCPConnect.Status.Failure != netxlite.FailureInterrupted {
+				t.Fatal("unexpected error")
+			}
+		})
+	})
 }