From a97bcd8f2a6b4dcf975659b21f1e34362538f418 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 6 Jun 2024 18:54:39 +0200 Subject: [PATCH] dnscheck --- internal/engine/experiment.go | 19 ++- internal/engine/experiment_test.go | 2 +- internal/experiment/dnscheck/dnscheck.go | 43 ++++--- internal/experiment/dnscheck/dnscheck_test.go | 74 +++++++---- internal/experiment/dnscheck/richerinput.go | 120 ++++++++++++++++++ internal/model/experiment.go | 3 + internal/oonirun/experiment.go | 14 +- internal/registry/dnscheck.go | 7 +- internal/registry/factory.go | 9 +- internal/targetloading/targetloading.go | 56 ++++---- internal/targetloading/targetloading_test.go | 5 +- 11 files changed, 262 insertions(+), 90 deletions(-) create mode 100644 internal/experiment/dnscheck/richerinput.go diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index 61197c0d8d..f2934ce4fa 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -109,11 +109,12 @@ func (e *experiment) SubmitAndUpdateMeasurementContext( } // newMeasurement creates a new measurement for this experiment with the given input. -func (e *experiment) newMeasurement(input string) *model.Measurement { +func (e *experiment) newMeasurement(target model.ExperimentTarget) *model.Measurement { utctimenow := time.Now().UTC() + // TODO(bassosimone,DecFox): add support for unmarshaling options. m := &model.Measurement{ DataFormatVersion: model.OOAPIReportDefaultDataFormatVersion, - Input: model.MeasurementInput(input), + Input: model.MeasurementInput(target.Input()), MeasurementStartTime: utctimenow.Format(model.MeasurementDateFormat), MeasurementStartTimeSaved: utctimenow, ProbeIP: model.DefaultProbeIP, @@ -204,10 +205,15 @@ func (e *experiment) MeasureWithContext( ctx = bytecounter.WithExperimentByteCounter(ctx, e.byteCounter) // Create a new measurement that the experiment measurer will finish filling - // by adding the test keys etc. Please, note that, as of 2024-06-05, we're using - // the measurement Input to provide input to an experiment. We'll probably - // change this, when we'll have finished implementing richer input. - measurement := e.newMeasurement(target.Input()) + // by adding the test keys etc. Please, note that, as of 2024-06-06: + // + // 1. experiments using richer input receive input via the Target field; + // + // 2. other experiments use (*Measurement).Input. + // + // Here we're passing the whole target to newMeasurement such that we're able + // to record options values in addition to the input value. + measurement := e.newMeasurement(target) // Record when we started the experiment, to compute the runtime. start := time.Now() @@ -217,6 +223,7 @@ func (e *experiment) MeasureWithContext( Callbacks: e.callbacks, Measurement: measurement, Session: e.session, + Target: target, } // Invoke the measurer. Conventionally, an error being returned here diff --git a/internal/engine/experiment_test.go b/internal/engine/experiment_test.go index e2444306af..ceab79d7bb 100644 --- a/internal/engine/experiment_test.go +++ b/internal/engine/experiment_test.go @@ -17,7 +17,7 @@ func TestExperimentHonoursSharingDefaults(t *testing.T) { t.Fatal(err) } exp := builder.NewExperiment().(*experiment) - return exp.newMeasurement("") + return exp.newMeasurement(model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")) } type spec struct { name string diff --git a/internal/experiment/dnscheck/dnscheck.go b/internal/experiment/dnscheck/dnscheck.go index 4c9d5fd17c..3b6c93787b 100644 --- a/internal/experiment/dnscheck/dnscheck.go +++ b/internal/experiment/dnscheck/dnscheck.go @@ -96,7 +96,6 @@ type TestKeys struct { // Measurer performs the measurement. type Measurer struct { - Config Endpoints *Endpoints } @@ -125,6 +124,10 @@ 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 + target := args.Target.(*Target) + config, input := target.options, target.input + // 1. fill the measurement with test keys tk := new(TestKeys) tk.Lookups = make(map[string]urlgetter.TestKeys) @@ -133,20 +136,19 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { // 2. select the domain to resolve or use default and, while there, also // ensure that we register all the other options we're using. - domain := m.Config.Domain + domain := config.Domain if domain == "" { domain = defaultDomain } - tk.DefaultAddrs = m.Config.DefaultAddrs + tk.DefaultAddrs = config.DefaultAddrs tk.Domain = domain - tk.HTTP3Enabled = m.Config.HTTP3Enabled - tk.HTTPHost = m.Config.HTTPHost - tk.TLSServerName = m.Config.TLSServerName - tk.TLSVersion = m.Config.TLSVersion + tk.HTTP3Enabled = config.HTTP3Enabled + tk.HTTPHost = config.HTTPHost + tk.TLSServerName = config.TLSServerName + tk.TLSVersion = config.TLSVersion tk.Residual = m.Endpoints != nil // 3. parse the input URL describing the resolver to use - input := string(measurement.Input) if input == "" { return ErrInputRequired } @@ -191,7 +193,7 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { for _, addr := range addrs { allAddrs[addr] = true } - for _, addr := range strings.Split(m.Config.DefaultAddrs, " ") { + for _, addr := range strings.Split(config.DefaultAddrs, " ") { if addr != "" { allAddrs[addr] = true } @@ -208,10 +210,10 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { for addr := range allAddrs { inputs = append(inputs, urlgetter.MultiInput{ Config: urlgetter.Config{ - DNSHTTPHost: m.httpHost(URL.Host), - DNSTLSServerName: m.tlsServerName(URL.Hostname()), - DNSTLSVersion: m.Config.TLSVersion, - HTTP3Enabled: m.Config.HTTP3Enabled, + DNSHTTPHost: config.httpHost(URL.Host), + DNSTLSServerName: config.tlsServerName(URL.Hostname()), + DNSTLSVersion: config.TLSVersion, + HTTP3Enabled: config.HTTP3Enabled, RejectDNSBogons: true, // bogons are errors in this context ResolverURL: makeResolverURL(URL, addr), Timeout: 15 * time.Second, @@ -244,17 +246,17 @@ func (m *Measurer) lookupHost(ctx context.Context, hostname string, r model.Reso // httpHost returns the configured HTTP host, if set, otherwise // it will return the host provide as argument. -func (m *Measurer) httpHost(httpHost string) string { - if m.Config.HTTPHost != "" { - return m.Config.HTTPHost +func (c *Config) httpHost(httpHost string) string { + if c.HTTPHost != "" { + return c.HTTPHost } return httpHost } // tlsServerName is like httpHost for the TLS server name. -func (m *Measurer) tlsServerName(tlsServerName string) string { - if m.Config.TLSServerName != "" { - return m.Config.TLSServerName +func (c *Config) tlsServerName(tlsServerName string) string { + if c.TLSServerName != "" { + return c.TLSServerName } return tlsServerName } @@ -311,9 +313,8 @@ func makeResolverURL(URL *url.URL, addr string) string { } // NewExperimentMeasurer creates a new ExperimentMeasurer. -func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { +func NewExperimentMeasurer() model.ExperimentMeasurer { return &Measurer{ - Config: config, Endpoints: nil, // disabled by default } } diff --git a/internal/experiment/dnscheck/dnscheck_test.go b/internal/experiment/dnscheck/dnscheck_test.go index 2fd2c295c8..bfe91eb767 100644 --- a/internal/experiment/dnscheck/dnscheck_test.go +++ b/internal/experiment/dnscheck/dnscheck_test.go @@ -8,44 +8,40 @@ import ( "time" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/legacy/mockable" + "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" ) func TestHTTPHostWithOverride(t *testing.T) { - m := Measurer{Config: Config{HTTPHost: "antani"}} - result := m.httpHost("mascetti") - if result != "antani" { + c := &Config{HTTPHost: "antani"} + if result := c.httpHost("mascetti"); result != "antani" { t.Fatal("not the result we expected") } } func TestHTTPHostWithoutOverride(t *testing.T) { - m := Measurer{Config: Config{}} - result := m.httpHost("mascetti") - if result != "mascetti" { + c := &Config{} + if result := c.httpHost("mascetti"); result != "mascetti" { t.Fatal("not the result we expected") } } func TestTLSServerNameWithOverride(t *testing.T) { - m := Measurer{Config: Config{TLSServerName: "antani"}} - result := m.tlsServerName("mascetti") - if result != "antani" { + c := &Config{TLSServerName: "antani"} + if result := c.tlsServerName("mascetti"); result != "antani" { t.Fatal("not the result we expected") } } func TestTLSServerNameWithoutOverride(t *testing.T) { - m := Measurer{Config: Config{}} - result := m.tlsServerName("mascetti") - if result != "mascetti" { + c := &Config{} + if result := c.tlsServerName("mascetti"); result != "mascetti" { t.Fatal("not the result we expected") } } func TestExperimentNameAndVersion(t *testing.T) { - measurer := NewExperimentMeasurer(Config{Domain: "example.com"}) + measurer := NewExperimentMeasurer() if measurer.ExperimentName() != "dnscheck" { t.Error("unexpected experiment name") } @@ -55,11 +51,17 @@ func TestExperimentNameAndVersion(t *testing.T) { } func TestDNSCheckFailsWithoutInput(t *testing.T) { - measurer := NewExperimentMeasurer(Config{Domain: "example.com"}) + measurer := NewExperimentMeasurer() args := &model.ExperimentArgs{ Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: new(model.Measurement), Session: newsession(), + Target: &Target{ + input: "", // explicitly empty + options: &Config{ + Domain: "example.com", + }, + }, } err := measurer.Run(context.Background(), args) if !errors.Is(err, ErrInputRequired) { @@ -68,11 +70,15 @@ func TestDNSCheckFailsWithoutInput(t *testing.T) { } func TestDNSCheckFailsWithInvalidURL(t *testing.T) { - measurer := NewExperimentMeasurer(Config{}) + measurer := NewExperimentMeasurer() args := &model.ExperimentArgs{ Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: &model.Measurement{Input: "Not a valid URL \x7f"}, Session: newsession(), + Target: &Target{ + input: "Not a valid URL \x7f", + options: &Config{}, + }, } err := measurer.Run(context.Background(), args) if !errors.Is(err, ErrInvalidURL) { @@ -81,11 +87,15 @@ func TestDNSCheckFailsWithInvalidURL(t *testing.T) { } func TestDNSCheckFailsWithUnsupportedProtocol(t *testing.T) { - measurer := NewExperimentMeasurer(Config{}) + measurer := NewExperimentMeasurer() args := &model.ExperimentArgs{ Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: &model.Measurement{Input: "file://1.1.1.1"}, Session: newsession(), + Target: &Target{ + input: "file://1.1.1.1", + options: &Config{}, + }, } err := measurer.Run(context.Background(), args) if !errors.Is(err, ErrUnsupportedURLScheme) { @@ -96,14 +106,18 @@ func TestDNSCheckFailsWithUnsupportedProtocol(t *testing.T) { func TestWithCancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // immediately cancel the context - measurer := NewExperimentMeasurer(Config{ - DefaultAddrs: "1.1.1.1 1.0.0.1", - }) + measurer := NewExperimentMeasurer() measurement := &model.Measurement{Input: "dot://one.one.one.one"} args := &model.ExperimentArgs{ Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: measurement, Session: newsession(), + Target: &Target{ + input: "dot://one.one.one.one", + options: &Config{ + DefaultAddrs: "1.1.1.1 1.0.0.1", + }, + }, } err := measurer.Run(ctx, args) if err != nil { @@ -140,14 +154,18 @@ func TestDNSCheckValid(t *testing.T) { t.Skip("skip test in short mode") } - measurer := NewExperimentMeasurer(Config{ - DefaultAddrs: "1.1.1.1 1.0.0.1", - }) + measurer := NewExperimentMeasurer() measurement := model.Measurement{Input: "dot://one.one.one.one:853"} args := &model.ExperimentArgs{ Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: &measurement, Session: newsession(), + Target: &Target{ + input: "dot://one.one.one.one:853", + options: &Config{ + DefaultAddrs: "1.1.1.1 1.0.0.1", + }, + }, } err := measurer.Run(context.Background(), args) if err != nil { @@ -169,7 +187,11 @@ func TestDNSCheckValid(t *testing.T) { } func newsession() model.ExperimentSession { - return &mockable.Session{MockableLogger: log.Log} + return &mocks.Session{ + MockLogger: func() model.Logger { + return log.Log + }, + } } func TestDNSCheckWait(t *testing.T) { @@ -187,6 +209,10 @@ func TestDNSCheckWait(t *testing.T) { Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: &measurement, Session: newsession(), + Target: &Target{ + input: input, + options: &Config{}, + }, } err := measurer.Run(context.Background(), args) if err != nil { diff --git a/internal/experiment/dnscheck/richerinput.go b/internal/experiment/dnscheck/richerinput.go new file mode 100644 index 0000000000..758b014319 --- /dev/null +++ b/internal/experiment/dnscheck/richerinput.go @@ -0,0 +1,120 @@ +package dnscheck + +import ( + "context" + + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" +) + +// Target is a richer-input target that this experiment should measure. +type Target struct { + // input is the input. + input string + + // options is the configuration. + options *Config +} + +var _ model.ExperimentTarget = &Target{} + +// Category implements [model.ExperimentTarget]. +func (t *Target) Category() string { + return model.DefaultCategoryCode +} + +// Country implements [model.ExperimentTarget]. +func (t *Target) Country() string { + return model.DefaultCountryCode +} + +// Input implements [model.ExperimentTarget]. +func (t *Target) Input() string { + return t.input +} + +// String implements [model.ExperimentTarget]. +func (t *Target) String() string { + return t.input +} + +// NewLoader constructs a new [model.ExperimentTargerLoader] instance. +// +// This function PANICS if options is not an instance of [*dnscheck.Config]. +func NewLoader(loader *targetloading.Loader, gopts any) model.ExperimentTargetLoader { + // Panic if we cannot convert the options to the expected type. + // + // We do not expect a panic here because the type is managed by the registry package. + options := gopts.(*Config) + + // Construct the proper loader instance. + return &targetLoader{ + loader: loader, + options: options, + } +} + +type targetLoader struct { + loader *targetloading.Loader + options *Config +} + +// Load implements model.ExperimentTargetLoader. +func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { + // TODO(bassosimone): we need a way to know whether the options are empty!!! + + // If there's nothing to statically load fallback to the API + if len(tl.loader.StaticInputs) <= 0 && len(tl.loader.SourceFiles) <= 0 { + return tl.loadFromBackend(ctx) + } + + // Otherwise, attempt to load the static inputs from CLI and files + inputs, err := targetloading.LoadStatic(tl.loader) + + // Handle the case where we couldn't + if err != nil { + return nil, err + } + + // Build the list of targets that we should measure. + var targets []model.ExperimentTarget + for _, input := range inputs { + targets = append(targets, &Target{ + options: tl.options, + input: input, + }) + } + return targets, nil +} + +var defaultInput = []model.ExperimentTarget{ + // + // https://dns.google/dns-query + // + // Measure HTTP/3 first and then HTTP/2 (see https://github.com/ooni/probe/issues/2675). + // + // Make sure we include the typical IP addresses for the domain. + // + &Target{ + input: "https://dns.google/dns-query", + options: &Config{ + HTTP3Enabled: true, + DefaultAddrs: "8.8.8.8 8.8.4.4", + }, + }, + &Target{ + input: "https://dns.google/dns-query", + options: &Config{ + DefaultAddrs: "8.8.8.8 8.8.4.4", + }, + }, + + // TODO(bassosimone): before merging, we need to reinstate the + // whole list that we previously had in tree +} + +func (tl *targetLoader) loadFromBackend(_ context.Context) ([]model.ExperimentTarget, error) { + // TODO(https://github.com/ooni/probe/issues/1390): serve DNSCheck + // inputs using richer input (aka check-in v2). + return defaultInput, nil +} diff --git a/internal/model/experiment.go b/internal/model/experiment.go index 8bd309f326..4aac4a0147 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -123,6 +123,9 @@ type ExperimentArgs struct { // Session is the MANDATORY session the experiment can use. Session ExperimentSession + + // Target is the MANDATORY target we're measuring. + Target ExperimentTarget } // ExperimentMeasurer is the interface that allows to run a diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 758db9e0c5..3136e32c66 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -82,14 +82,19 @@ func (ed *Experiment) Run(ctx context.Context) error { return err } - // 2. create input loader and load input for this experiment + // 2. configure experiment's options + if err := builder.SetOptionsAny(ed.ExtraOptions); err != nil { + return err + } + + // 3. create input loader and load input for this experiment targetLoader := ed.newTargetLoader(builder) targetList, err := targetLoader.Load(ctx) if err != nil { return err } - // 3. randomize input, if needed + // 4. randomize input, if needed if ed.Random { rnd := rand.New(rand.NewSource(time.Now().UnixNano())) // #nosec G404 -- not really important rnd.Shuffle(len(targetList), func(i, j int) { @@ -98,11 +103,6 @@ func (ed *Experiment) Run(ctx context.Context) error { experimentShuffledInputs.Add(1) } - // 4. configure experiment's options - if err := builder.SetOptionsAny(ed.ExtraOptions); err != nil { - return err - } - // 5. construct the experiment instance experiment := builder.NewExperiment() logger := ed.Session.Logger() diff --git a/internal/registry/dnscheck.go b/internal/registry/dnscheck.go index 2890ca19a5..2ed902c38e 100644 --- a/internal/registry/dnscheck.go +++ b/internal/registry/dnscheck.go @@ -12,16 +12,17 @@ import ( func init() { const canonicalName = "dnscheck" AllExperiments[canonicalName] = func() *Factory { + // TODO(bassosimone): for now, we MUST keep the InputOrStaticDefault + // policy because otherwise ./pkg/oonimkall should break. return &Factory{ build: func(config interface{}) model.ExperimentMeasurer { - return dnscheck.NewExperimentMeasurer( - *config.(*dnscheck.Config), - ) + return dnscheck.NewExperimentMeasurer() }, canonicalName: canonicalName, config: &dnscheck.Config{}, enabledByDefault: true, inputPolicy: model.InputOrStaticDefault, + newLoader: dnscheck.NewLoader, } } } diff --git a/internal/registry/factory.go b/internal/registry/factory.go index 0b37a79ee2..2bb6dd3fe2 100644 --- a/internal/registry/factory.go +++ b/internal/registry/factory.go @@ -37,6 +37,9 @@ type Factory struct { // interruptible indicates whether the experiment is interruptible. interruptible bool + + // newLoader is the OPTIONAL function to create a new loader. + newLoader func(config *targetloading.Loader, options any) model.ExperimentTargetLoader } // Session is the session definition according to this package. @@ -44,7 +47,7 @@ type Session = model.ExperimentTargetLoaderSession // NewTargetLoader creates a new [model.ExperimentTargetLoader] instance. func (b *Factory) NewTargetLoader(config *model.ExperimentTargetLoaderConfig) model.ExperimentTargetLoader { - return &targetloading.Loader{ + loader := &targetloading.Loader{ CheckInConfig: config.CheckInConfig, // OPTIONAL ExperimentName: b.canonicalName, InputPolicy: b.inputPolicy, @@ -53,6 +56,10 @@ func (b *Factory) NewTargetLoader(config *model.ExperimentTargetLoaderConfig) mo StaticInputs: config.StaticInputs, SourceFiles: config.SourceFiles, } + if b.newLoader != nil { + return b.newLoader(loader, b.config) + } + return loader } // Interruptible returns whether the experiment is interruptible. diff --git a/internal/targetloading/targetloading.go b/internal/targetloading/targetloading.go index 92e6a3b99c..c6c91162c5 100644 --- a/internal/targetloading/targetloading.go +++ b/internal/targetloading/targetloading.go @@ -220,16 +220,14 @@ 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. - // - // TODO(https://github.com/ooni/probe/issues/1390): serve DNSCheck - // inputs using richer input (aka check-in v2). - // - // TODO(https://github.com/ooni/probe/issues/2557): server STUNReachability - // inputs using richer input (aka check-in v2). switch experimentname.Canonicalize(name) { case "dnscheck": + // TODO(https://github.com/ooni/probe/issues/1390): serve DNSCheck + // inputs using richer input (aka check-in v2). return dnsCheckDefaultInput, nil case "stunreachability": + // TODO(https://github.com/ooni/probe/issues/2557): server STUNReachability + // inputs using richer input (aka check-in v2). return stunReachabilityDefaultInput, nil default: return nil, ErrNoStaticInput @@ -251,24 +249,17 @@ func (il *Loader) loadOrStaticDefault(_ context.Context) ([]model.ExperimentTarg return staticInputForExperiment(il.ExperimentName) } -// loadLocal loads inputs from StaticInputs and SourceFiles. +// loadLocal loads inputs from the [*Loader] StaticInputs and SourceFiles. func (il *Loader) loadLocal() ([]model.ExperimentTarget, error) { - inputs := []model.ExperimentTarget{} - for _, input := range il.StaticInputs { - inputs = append(inputs, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input)) + inputs, err := LoadStatic(il) + if err != nil { + return nil, err } - for _, filepath := range il.SourceFiles { - extra, err := il.readfile(filepath, fsx.OpenFile) - if err != nil { - return nil, err - } - // See https://github.com/ooni/probe-engine/issues/1123. - if len(extra) <= 0 { - return nil, fmt.Errorf("%w: %s", ErrDetectedEmptyFile, filepath) - } - inputs = append(inputs, extra...) + var targets []model.ExperimentTarget + for _, input := range inputs { + targets = append(targets, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input)) } - return inputs, nil + return targets, nil } // openFunc is the type of the function to open a file. @@ -276,8 +267,8 @@ type openFunc func(filepath string) (fs.File, error) // readfile reads inputs from the specified file. The open argument should be // compatible with stdlib's fs.Open and helps us with unit testing. -func (il *Loader) readfile(filepath string, open openFunc) ([]model.ExperimentTarget, error) { - inputs := []model.ExperimentTarget{} +func readfile(filepath string, open openFunc) ([]string, error) { + inputs := []string{} filep, err := open(filepath) if err != nil { return nil, err @@ -290,7 +281,7 @@ func (il *Loader) readfile(filepath string, open openFunc) ([]model.ExperimentTa for scanner.Scan() { line := scanner.Text() if line != "" { - inputs = append(inputs, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(line)) + inputs = append(inputs, line) } } if scanner.Err() != nil { @@ -299,6 +290,23 @@ func (il *Loader) readfile(filepath string, open openFunc) ([]model.ExperimentTa return inputs, nil } +// LoadStatic loads inputs from the [*Loader] StaticInputs and SourceFiles. +func LoadStatic(config *Loader) ([]string, error) { + inputs := append([]string{}, config.StaticInputs...) + for _, filepath := range config.SourceFiles { + extra, err := readfile(filepath, fsx.OpenFile) + if err != nil { + return nil, err + } + // See https://github.com/ooni/probe-engine/issues/1123. + if len(extra) <= 0 { + return nil, fmt.Errorf("%w: %s", ErrDetectedEmptyFile, filepath) + } + inputs = append(inputs, extra...) + } + return inputs, nil +} + // loadRemote loads inputs from a remote source. func (il *Loader) loadRemote(ctx context.Context) ([]model.ExperimentTarget, error) { switch experimentname.Canonicalize(il.ExperimentName) { diff --git a/internal/targetloading/targetloading_test.go b/internal/targetloading/targetloading_test.go index 82684a8205..0147ddb5ba 100644 --- a/internal/targetloading/targetloading_test.go +++ b/internal/targetloading/targetloading_test.go @@ -509,9 +509,8 @@ func (TargetLoaderBrokenFile) Close() error { return nil } -func TestTargetLoaderReadfileScannerFailure(t *testing.T) { - il := &Loader{} - out, err := il.readfile("", TargetLoaderBrokenFS{}.Open) +func TestReadfileScannerFailure(t *testing.T) { + out, err := readfile("", TargetLoaderBrokenFS{}.Open) if !errors.Is(err, syscall.EFAULT) { t.Fatal("not the error we expected") }