c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(oohelper): do not stop if DNS fails locally (#450)

Browse Source 
When a probe gets a local DNS failure, it will continue and nonetheless
query the test helper without any IP address, just an empty list.

This diff fixes the behavior of cmd/oohelper to do the same.

Work part of https://github.com/ooni/probe/issues/1707.
This commit is contained in:
Simone Basso 2021-09-05 12:06:02 +02:00 committed by GitHub
parent 619826ac34
commit 276beb8719
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
internal/cmd/oohelper/internal/client.go
View File

@ -91,12 +91,12 @@ func (oo OOClient) Do(ctx context.Context, config OOConfig) (*CtrlResponse, erro
return nil, fmt.Errorf("%w: %s", ErrInvalidURL, err.Error()) return nil, fmt.Errorf("%w: %s", ErrInvalidURL, err.Error())
} }
addrs, err := oo.Resolver.LookupHost(ctx, targetURL.Hostname()) addrs, err := oo.Resolver.LookupHost(ctx, targetURL.Hostname())
if err != nil { endpoints := []string{}
return nil, err if err == nil {
} endpoints, err = MakeTCPEndpoints(targetURL, addrs)
endpoints, err := MakeTCPEndpoints(targetURL, addrs) if err != nil {
if err != nil { return nil, err
return nil, err }
} }
creq := ctrlRequest{ creq := ctrlRequest{
HTTPRequest: config.TargetURL, HTTPRequest: config.TargetURL,

19
internal/cmd/oohelper/internal/client_test.go
View File

@ -104,14 +104,23 @@ func TestOOClientDoWithResolverFailure(t *testing.T) {
ServerURL: "https://wcth.ooni.io", ServerURL: "https://wcth.ooni.io",
} }
clnt := internal.OOClient{ clnt := internal.OOClient{
Resolver: internal.NewFakeResolverThatFails(), HTTPClient: http.DefaultClient,
Resolver: internal.NewFakeResolverThatFails(),
} }
cresp, err := clnt.Do(ctx, config) cresp, err := clnt.Do(ctx, config)
if !errors.Is(err, internal.ErrNotFound) { if err != nil {
t.Fatalf("not the error we expected: %+v", err) t.Fatal(err)
} }
if cresp != nil { if len(cresp.TCPConnect) > 0 {
t.Fatal("expected nil response") // The current implementation of the test helper (the legacy codebase)
// only follows the IP addresses returned by the client.
t.Fatal("expected empty TCPConnect here")
}
if cresp.HTTPRequest.StatusCode != 200 {
t.Fatal("expected 200 status code here")
}
if len(cresp.DNS.Addrs) < 1 {
t.Fatal("expected at least an IP address here")
} }
} }