diff --git a/internal/cmd/miniooni/libminiooni.go b/internal/cmd/miniooni/libminiooni.go index e7cba04..ea25898 100644 --- a/internal/cmd/miniooni/libminiooni.go +++ b/internal/cmd/miniooni/libminiooni.go @@ -416,10 +416,11 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { OnWiFi: true, // meaning: not on 4G Charging: true, }, - InputPolicy: builder.InputPolicy(), - StaticInputs: currentOptions.Inputs, - SourceFiles: currentOptions.InputFilePaths, - Session: sess, + ExperimentName: experimentName, + InputPolicy: builder.InputPolicy(), + StaticInputs: currentOptions.Inputs, + SourceFiles: currentOptions.InputFilePaths, + Session: sess, } inputs, err := inputLoader.Load(context.Background()) fatalOnError(err, "cannot load inputs") diff --git a/internal/engine/allexperiments.go b/internal/engine/allexperiments.go index 06b899e..25750c4 100644 --- a/internal/engine/allexperiments.go +++ b/internal/engine/allexperiments.go @@ -49,7 +49,7 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ )) }, config: &dnscheck.Config{}, - inputPolicy: InputStrictlyRequired, + inputPolicy: InputOrStaticDefault, } }, @@ -198,7 +198,7 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ )) }, config: &stunreachability.Config{}, - inputPolicy: InputStrictlyRequired, + inputPolicy: InputOrStaticDefault, } }, diff --git a/internal/engine/experimentbuilder.go b/internal/engine/experimentbuilder.go index 97d8299..0177d98 100644 --- a/internal/engine/experimentbuilder.go +++ b/internal/engine/experimentbuilder.go @@ -36,6 +36,11 @@ const ( // InputNone indicates that the experiment does not want any // input and ignores the input if provided with it. InputNone = InputPolicy("none") + + // We gather input from StaticInput and SourceFiles. If there is + // input, we return it. Otherwise, we return an internal static + // list of inputs to be used with this experiment. + InputOrStaticDefault = InputPolicy("or_static_default") ) // ExperimentBuilder is an experiment builder. @@ -183,10 +188,18 @@ func (b *ExperimentBuilder) NewExperiment() *Experiment { // canonicalizeExperimentName allows code to provide experiment names // in a more flexible way, where we have aliases. +// +// Because we allow for uppercase experiment names for backwards +// compatibility with MK, we need to add some exceptions here when +// mapping (e.g., DNSCheck => dnscheck). func canonicalizeExperimentName(name string) string { switch name = strcase.ToSnake(name); name { case "ndt_7": name = "ndt" // since 2020-03-18, we use ndt7 to implement ndt by default + case "dns_check": + name = "dnscheck" + case "stun_reachability": + name = "stunreachability" default: } return name diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 3e4640d..ee1316c 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -6,10 +6,12 @@ import ( "errors" "fmt" "io/fs" + "net/url" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/fsx" + "github.com/ooni/probe-cli/v3/internal/stuninput" ) // These errors are returned by the InputLoader. @@ -18,6 +20,7 @@ var ( ErrDetectedEmptyFile = errors.New("file did not contain any input") ErrInputRequired = errors.New("no input provided") ErrNoInputExpected = errors.New("we did not expect any input") + ErrNoStaticInput = errors.New("no static input for this experiment") ) // InputLoaderSession is the session according to an InputLoader. We @@ -58,6 +61,12 @@ type InputLoaderLogger interface { // input, we return it. Otherwise, we use OONI's probe services // to gather input using the best API for the task. // +// InputOrStaticDefault +// +// We gather input from StaticInput and SourceFiles. If there is +// input, we return it. Otherwise, we return an internal static +// list of inputs to be used with this experiment. +// // InputStrictlyRequired // // We gather input from StaticInput and SourceFiles. If there is @@ -69,6 +78,10 @@ type InputLoader struct { // will set them to a default value. CheckInConfig *model.CheckInConfig + // ExperimentName is the name of the experiment. This field + // is only used together with the InputOrStaticDefault policy. + ExperimentName string + // InputPolicy specifies the input policy for the // current experiment. We will not load any input if // the policy says we should not. You MUST fill in @@ -105,6 +118,8 @@ func (il *InputLoader) Load(ctx context.Context) ([]model.URLInfo, error) { return il.loadOrQueryBackend(ctx) case InputStrictlyRequired: return il.loadStrictlyRequired(ctx) + case InputOrStaticDefault: + return il.loadOrStaticDefault(ctx) default: return il.loadNone() } @@ -147,6 +162,59 @@ func (il *InputLoader) loadOrQueryBackend(ctx context.Context) ([]model.URLInfo, return il.loadRemote(ctx) } +// TODO(https://github.com/ooni/probe/issues/1390): we need to +// implement serving DNSCheck targets from the API +var dnsCheckDefaultInput = []string{ + "https://dns.google/dns-query", + "https://8.8.8.8/dns-query", + "dot://8.8.8.8:853/", + "dot://8.8.4.4:853/", + "https://8.8.4.4/dns-query", + "https://cloudflare-dns.com/dns-query", + "https://1.1.1.1/dns-query", + "https://1.0.0.1/dns-query", + "dot://1.1.1.1:853/", + "dot://1.0.0.1:853/", + "https://dns.quad9.net/dns-query", + "https://9.9.9.9/dns-query", + "dot://9.9.9.9:853/", + "dot://dns.quad9.net/", +} + +var stunReachabilityDefaultInput = stuninput.AsnStunReachabilityInput() + +// staticBareInputForExperiment returns the list of strings an +// experiment should use as static input. In case there is no +// static input for this experiment, we return an error. +func staticBareInputForExperiment(name string) ([]string, error) { + // Implementation note: we may be called from pkg/oonimkall + // with a non-canonical experiment name, so we need to convert + // the experiment name to be canonical before proceeding. + switch canonicalizeExperimentName(name) { + case "dnscheck": + return dnsCheckDefaultInput, nil + case "stunreachability": + return stunReachabilityDefaultInput, nil + default: + return nil, ErrNoStaticInput + } +} + +// staticInputForExperiment returns the static input for the given experiment +// or an error if there's no static input for the experiment. +func staticInputForExperiment(name string) ([]model.URLInfo, error) { + return stringListToModelURLInfo(staticBareInputForExperiment(name)) +} + +// loadOrStaticDefault implements the InputOrStaticDefault policy. +func (il *InputLoader) loadOrStaticDefault(ctx context.Context) ([]model.URLInfo, error) { + inputs, err := il.loadLocal() + if err != nil || len(inputs) > 0 { + return inputs, err + } + return staticInputForExperiment(il.ExperimentName) +} + // loadLocal loads inputs from StaticInputs and SourceFiles. func (il *InputLoader) loadLocal() ([]model.URLInfo, error) { inputs := []model.URLInfo{} @@ -264,3 +332,26 @@ func (il *InputLoader) logger() InputLoaderLogger { } return log.Log } + +// stringListToModelURLInfo is an utility function to convert +// a list of strings containing URLs into a list of model.URLInfo +// which would have been returned by an hypothetical backend +// API serving input for a test for which we don't have an API +// yet (e.g., stunreachability and dnscheck). +func stringListToModelURLInfo(input []string, err error) ([]model.URLInfo, error) { + if err != nil { + return nil, err + } + var output []model.URLInfo + for _, URL := range input { + if _, err := url.Parse(URL); err != nil { + return nil, err + } + output = append(output, model.URLInfo{ + CategoryCode: "MISC", // hard to find a category + CountryCode: "XX", // representing no country + URL: URL, + }) + } + return output, nil +} diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index fb3e940..b74d4ca 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -6,6 +6,7 @@ import ( "io" "io/fs" "os" + "strings" "syscall" "testing" @@ -206,6 +207,134 @@ func TestInputLoaderInputStrictlyRequiredWithEmptyFile(t *testing.T) { } } +func TestInputLoaderInputOrStaticDefaultWithInput(t *testing.T) { + il := &InputLoader{ + ExperimentName: "dnscheck", + StaticInputs: []string{"https://www.google.com/"}, + SourceFiles: []string{ + "testdata/inputloader1.txt", + "testdata/inputloader2.txt", + }, + InputPolicy: InputOrStaticDefault, + } + ctx := context.Background() + out, err := il.Load(ctx) + if err != nil { + t.Fatal(err) + } + if len(out) != 5 { + t.Fatal("not the output length we expected") + } + expect := []model.URLInfo{ + {URL: "https://www.google.com/"}, + {URL: "https://www.x.org/"}, + {URL: "https://www.slashdot.org/"}, + {URL: "https://abc.xyz/"}, + {URL: "https://run.ooni.io/"}, + } + if diff := cmp.Diff(out, expect); diff != "" { + t.Fatal(diff) + } +} + +func TestInputLoaderInputOrStaticDefaultWithEmptyFile(t *testing.T) { + il := &InputLoader{ + ExperimentName: "dnscheck", + InputPolicy: InputOrStaticDefault, + SourceFiles: []string{ + "testdata/inputloader1.txt", + "testdata/inputloader3.txt", // we want it before inputloader2.txt + "testdata/inputloader2.txt", + }, + } + ctx := context.Background() + out, err := il.Load(ctx) + if !errors.Is(err, ErrDetectedEmptyFile) { + t.Fatalf("not the error we expected: %+v", err) + } + if out != nil { + t.Fatal("not the output we expected") + } +} + +func TestInputLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { + il := &InputLoader{ + ExperimentName: "dnscheck", + InputPolicy: InputOrStaticDefault, + } + ctx := context.Background() + out, err := il.Load(ctx) + if err != nil { + t.Fatal(err) + } + if len(out) != len(dnsCheckDefaultInput) { + t.Fatal("invalid output length") + } + for idx := 0; idx < len(dnsCheckDefaultInput); idx++ { + e := out[idx] + if e.CategoryCode != "MISC" { + t.Fatal("invalid category code") + } + if e.CountryCode != "XX" { + t.Fatal("invalid country code") + } + if e.URL != dnsCheckDefaultInput[idx] { + t.Fatal("invalid URL") + } + } +} + +func TestInputLoaderInputOrStaticDefaultWithoutInputStunReachability(t *testing.T) { + il := &InputLoader{ + ExperimentName: "stunreachability", + InputPolicy: InputOrStaticDefault, + } + ctx := context.Background() + out, err := il.Load(ctx) + if err != nil { + t.Fatal(err) + } + if len(out) != len(stunReachabilityDefaultInput) { + t.Fatal("invalid output length") + } + for idx := 0; idx < len(stunReachabilityDefaultInput); idx++ { + e := out[idx] + if e.CategoryCode != "MISC" { + t.Fatal("invalid category code") + } + if e.CountryCode != "XX" { + t.Fatal("invalid country code") + } + if e.URL != stunReachabilityDefaultInput[idx] { + t.Fatal("invalid URL") + } + } +} + +func TestStaticBareInputForExperimentWorksWithNonCanonicalNames(t *testing.T) { + names := []string{"DNSCheck", "STUNReachability"} + for _, name := range names { + if _, err := staticInputForExperiment(name); err != nil { + t.Fatal("failure for", name, ":", err) + } + } +} + +func TestInputLoaderInputOrStaticDefaultWithoutInputOtherName(t *testing.T) { + il := &InputLoader{ + ExperimentName: "xx", + InputPolicy: InputOrStaticDefault, + } + ctx := context.Background() + out, err := il.Load(ctx) + if !errors.Is(err, ErrNoStaticInput) { + t.Fatal("not the error we expected", err) + } + if out != nil { + t.Fatal("expected nil result here") + } +} + func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { il := &InputLoader{ StaticInputs: []string{"https://www.google.com/"}, @@ -494,3 +623,59 @@ func TestInputLoaderLoggerWorksAsIntended(t *testing.T) { t.Fatal("logger not working as intended") } } + +func TestStringListToModelURLInfoWithValidInput(t *testing.T) { + input := []string{ + "stun://stun.voip.blackberry.com:3478", + "stun://stun.altar.com.pl:3478", + } + output, err := stringListToModelURLInfo(input, nil) + if err != nil { + t.Fatal(err) + } + if len(input) != len(output) { + t.Fatal("unexpected output length") + } + for idx := 0; idx < len(input); idx++ { + if input[idx] != output[idx].URL { + t.Fatal("unexpected entry") + } + if output[idx].CategoryCode != "MISC" { + t.Fatal("unexpected category") + } + if output[idx].CountryCode != "XX" { + t.Fatal("unexpected country") + } + } +} + +func TestStringListToModelURLInfoWithInvalidInput(t *testing.T) { + input := []string{ + "stun://stun.voip.blackberry.com:3478", + "\t", // <- not a valid URL + "stun://stun.altar.com.pl:3478", + } + output, err := stringListToModelURLInfo(input, nil) + if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") { + t.Fatal("no the error we expected", err) + } + if output != nil { + t.Fatal("unexpected nil output") + } +} + +func TestStringListToModelURLInfoWithError(t *testing.T) { + input := []string{ + "stun://stun.voip.blackberry.com:3478", + "\t", + "stun://stun.altar.com.pl:3478", + } + expected := errors.New("mocked error") + output, err := stringListToModelURLInfo(input, expected) + if !errors.Is(err, expected) { + t.Fatal("no the error we expected", err) + } + if output != nil { + t.Fatal("unexpected nil output") + } +}