From beb1fc7223e77f8a43ff643165181fbf8a91d3f4 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 26 Apr 2024 16:51:16 +0200 Subject: [PATCH] chore(probeservices): better tests for psiphon and tor (#1568) Part of https://github.com/ooni/probe/issues/2718. --- internal/probeservices/psiphon_test.go | 220 +++++++++++++++--- internal/probeservices/tor_test.go | 301 +++++++++++++++++++------ 2 files changed, 418 insertions(+), 103 deletions(-) diff --git a/internal/probeservices/psiphon_test.go b/internal/probeservices/psiphon_test.go index 3bfacd2de2..47348c6873 100644 --- a/internal/probeservices/psiphon_test.go +++ b/internal/probeservices/psiphon_test.go @@ -4,44 +4,198 @@ import ( "context" "encoding/json" "errors" + "net/http" + "net/url" "testing" + "time" + + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" ) func TestFetchPsiphonConfig(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - clnt := newclient() - if err := clnt.MaybeRegister(context.Background(), MetadataFixture()); err != nil { - t.Fatal(err) - } - if err := clnt.MaybeLogin(context.Background()); err != nil { - t.Fatal(err) - } - data, err := clnt.FetchPsiphonConfig(context.Background()) - if err != nil { - t.Fatal(err) - } - var config interface{} - if err := json.Unmarshal(data, &config); err != nil { - t.Fatal(err) - } -} + // psiphonflow is the flow with which we invoke the psiphon API + psiphonflow := func(t *testing.T, client *Client) ([]byte, error) { + // we need to make sure we're registered and logged in + if err := client.MaybeRegister(context.Background(), MetadataFixture()); err != nil { + t.Fatal(err) + } + if err := client.MaybeLogin(context.Background()); err != nil { + t.Fatal(err) + } -func TestFetchPsiphonConfigNotRegistered(t *testing.T) { - clnt := newclient() - state := State{ - // Explicitly empty so the test is more clear - } - if err := clnt.StateFile.Set(state); err != nil { - t.Fatal(err) - } - data, err := clnt.FetchPsiphonConfig(context.Background()) - if !errors.Is(err, ErrNotRegistered) { - t.Fatal("expected an error here") - } - if data != nil { - t.Fatal("expected nil data here") + // then we can try to fetch the config + return client.FetchPsiphonConfig(context.Background()) } + + // First, let's check whether we can get a response from the real OONI backend. + t.Run("is working as intended with the real backend", func(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + + clnt := newclient() + + // run the psiphon flow + data, err := psiphonflow(t, clnt) + + // we do not expect an error here + if err != nil { + t.Fatal(err) + } + + // the config is bytes but we want to make sure we can parse it + var config interface{} + if err := json.Unmarshal(data, &config); err != nil { + t.Fatal(err) + } + }) + + // Now let's construct a test server that returns a valid response and try + // to communicate with such a test server successfully and with errors + + t.Run("is working as intended with a local test server", func(t *testing.T) { + // create state for emulating the OONI backend + state := &testingx.OONIBackendWithLoginFlow{} + + // make sure we return something that is JSON parseable + state.SetPsiphonConfig([]byte(`{}`)) + + // expose the state via HTTP + srv := testingx.MustNewHTTPServer(state.NewMux()) + defer srv.Close() + + // create a probeservices client + client := newclient() + + // override the HTTP client so we speak with our local server rather than the true backend + client.HTTPClient = &mocks.HTTPClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + URL := runtimex.Try1(url.Parse(srv.URL)) + req.URL.Scheme = URL.Scheme + req.URL.Host = URL.Host + return http.DefaultClient.Do(req) + }, + MockCloseIdleConnections: func() { + http.DefaultClient.CloseIdleConnections() + }, + } + + // then we can try to fetch the config + data, err := psiphonflow(t, client) + + // we do not expect an error here + if err != nil { + t.Fatal(err) + } + + // the config is bytes but we want to make sure we can parse it + var config interface{} + if err := json.Unmarshal(data, &config); err != nil { + t.Fatal(err) + } + }) + + t.Run("reports an error when the connection is reset", func(t *testing.T) { + // create quick and dirty server to serve the response + srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset()) + defer srv.Close() + + // create a probeservices client + client := newclient() + + // override the HTTP client + client.HTTPClient = &mocks.HTTPClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + URL := runtimex.Try1(url.Parse(srv.URL)) + req.URL.Scheme = URL.Scheme + req.URL.Host = URL.Host + return http.DefaultClient.Do(req) + }, + MockCloseIdleConnections: func() { + http.DefaultClient.CloseIdleConnections() + }, + } + + // we need to convince the client that we're logged in first otherwise it will + // refuse to send a request to the server and we won't be testing networking + runtimex.Try0(client.StateFile.Set(State{ + ClientID: "ttt-uuu-iii", + Expire: time.Now().Add(30 * time.Hour), + Password: "xxx-xxx-xxx", + Token: "abc-yyy-zzz", + })) + + // issue the call directly: no register or login otherwise we're testing + // the register or login implementation LOL + data, err := client.FetchPsiphonConfig(context.Background()) + + // we do expect an error + if !errors.Is(err, netxlite.ECONNRESET) { + t.Fatal("unexpected error", err) + } + + // we expect to see zero-length data + if len(data) != 0 { + t.Fatal("expected result lenght to be zero") + } + }) + + t.Run("when we're not registered", func(t *testing.T) { + clnt := newclient() + + // With explicitly empty state so it's pretty obvioust there's no state + state := State{} + + // force the state to be empty + if err := clnt.StateFile.Set(state); err != nil { + t.Fatal(err) + } + + // attempt to fetch the config + data, err := clnt.FetchPsiphonConfig(context.Background()) + + // ensure that the error says we're not registered + if !errors.Is(err, ErrNotRegistered) { + t.Fatal("expected an error here") + } + + // obviously the data should be empty as well + if len(data) != 0 { + t.Fatal("expected nil data here") + } + }) + + t.Run("correctly handles the case where the URL is unparseable", func(t *testing.T) { + // create a probeservices client + client := newclient() + + // override the URL to be unparseable + client.BaseURL = "\t\t\t" + + // we need to convince the client that we're logged in first otherwise it will + // refuse to send a request to the server and we won't be testing networking + runtimex.Try0(client.StateFile.Set(State{ + ClientID: "ttt-uuu-iii", + Expire: time.Now().Add(30 * time.Hour), + Password: "xxx-xxx-xxx", + Token: "abc-yyy-zzz", + })) + + // issue the API call proper + data, err := client.FetchPsiphonConfig(context.Background()) + + // we do expect an error + if err == nil || err.Error() != `parse "\t\t\t": net/url: invalid control character in URL` { + t.Fatal("unexpected error", err) + } + + // we expect data to be zero length + if len(data) != 0 { + t.Fatal("expected zero length data") + } + }) } diff --git a/internal/probeservices/tor_test.go b/internal/probeservices/tor_test.go index 62f7065143..01ed6e6b91 100644 --- a/internal/probeservices/tor_test.go +++ b/internal/probeservices/tor_test.go @@ -2,85 +2,246 @@ package probeservices import ( "context" + "errors" "net/http" + "net/url" "testing" + "time" + + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" ) func TestFetchTorTargets(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - clnt := newclient() - if err := clnt.MaybeRegister(context.Background(), MetadataFixture()); err != nil { - t.Fatal(err) - } - if err := clnt.MaybeLogin(context.Background()); err != nil { - t.Fatal(err) - } - data, err := clnt.FetchTorTargets(context.Background(), "ZZ") - if err != nil { - t.Fatal(err) - } - if data == nil || len(data) <= 0 { - t.Fatal("invalid data") - } -} + // Implementation note: OONIBackendWithLoginFlow ensures that tor sends a query + // string and there are also tests making sure of that. We use to have a test that + // checked for the query string here, but now it seems unnecessary. -func TestFetchTorTargetsNotRegistered(t *testing.T) { - clnt := newclient() - state := State{ - // Explicitly empty so the test is more clear - } - if err := clnt.StateFile.Set(state); err != nil { - t.Fatal(err) - } - data, err := clnt.FetchTorTargets(context.Background(), "ZZ") - if err == nil { - t.Fatal("expected an error here") - } - if data != nil { - t.Fatal("expected nil data here") + // torflow is the flow with which we invoke the tor API + torflow := func(t *testing.T, client *Client) (map[string]model.OOAPITorTarget, error) { + // we need to make sure we're registered and logged in + if err := client.MaybeRegister(context.Background(), MetadataFixture()); err != nil { + t.Fatal(err) + } + if err := client.MaybeLogin(context.Background()); err != nil { + t.Fatal(err) + } + + // then we can try to fetch the config + return client.FetchTorTargets(context.Background(), "ZZ") } -} -type FetchTorTargetsHTTPTransport struct { - Response *http.Response -} + // First, let's check whether we can get a response from the real OONI backend. + t.Run("is working as intended with the real backend", func(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } -func (clnt *FetchTorTargetsHTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { - resp, err := http.DefaultTransport.RoundTrip(req) - if err != nil { - return nil, err - } - if req.URL.Path == "/api/v1/test-list/tor-targets" { - clnt.Response = resp - } - return resp, err -} + clnt := newclient() -func TestFetchTorTargetsSetsQueryString(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } + // run the tor flow + targets, err := torflow(t, clnt) + + // we do not expect an error here + if err != nil { + t.Fatal(err) + } + + // we expect non-zero length targets + if len(targets) <= 0 { + t.Fatal("expected non-zero-length targets") + } + }) + + // Now let's construct a test server that returns a valid response and try + // to communicate with such a test server successfully and with errors + + t.Run("is working as intended with a local test server", func(t *testing.T) { + // create state for emulating the OONI backend + state := &testingx.OONIBackendWithLoginFlow{} + + // make sure we return something that is JSON parseable and non-zero-length + state.SetTorTargets([]byte(`{"foo": {}}`)) + + // expose the state via HTTP + srv := testingx.MustNewHTTPServer(state.NewMux()) + defer srv.Close() + + // create a probeservices client + client := newclient() + + // override the HTTP client so we speak with out local server rather than the true backend + client.HTTPClient = &mocks.HTTPClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + URL := runtimex.Try1(url.Parse(srv.URL)) + req.URL.Scheme = URL.Scheme + req.URL.Host = URL.Host + return http.DefaultClient.Do(req) + }, + MockCloseIdleConnections: func() { + http.DefaultClient.CloseIdleConnections() + }, + } + + // run the tor flow + targets, err := torflow(t, client) + + // we do not expect an error here + if err != nil { + t.Fatal(err) + } + + // we expect non-zero length targets + if len(targets) <= 0 { + t.Fatal("expected non-zero-length targets") + } + }) + + t.Run("reports an error when the connection is reset", func(t *testing.T) { + // create quick and dirty server to serve the response + srv := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset()) + defer srv.Close() + + // create a probeservices client + client := newclient() + + // override the HTTP client + client.HTTPClient = &mocks.HTTPClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + URL := runtimex.Try1(url.Parse(srv.URL)) + req.URL.Scheme = URL.Scheme + req.URL.Host = URL.Host + return http.DefaultClient.Do(req) + }, + MockCloseIdleConnections: func() { + http.DefaultClient.CloseIdleConnections() + }, + } + + // we need to convince the client that we're logged in first otherwise it will + // refuse to send a request to the server and we won't be testing networking + runtimex.Try0(client.StateFile.Set(State{ + ClientID: "ttt-uuu-iii", + Expire: time.Now().Add(30 * time.Hour), + Password: "xxx-xxx-xxx", + Token: "abc-yyy-zzz", + })) + + // run the tor flow + targets, err := torflow(t, client) + + // we do expect an error + if !errors.Is(err, netxlite.ECONNRESET) { + t.Fatal("unexpected error", err) + } + + // we expect to see zero-length targets + if len(targets) != 0 { + t.Fatal("expected targets to be zero length") + } + }) + + t.Run("reports an error when the response is not JSON parsable", func(t *testing.T) { + // create quick and dirty server to serve the response + srv := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{`)) + })) + defer srv.Close() + + // create a probeservices client + client := newclient() + + // override the HTTP client + client.HTTPClient = &mocks.HTTPClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + URL := runtimex.Try1(url.Parse(srv.URL)) + req.URL.Scheme = URL.Scheme + req.URL.Host = URL.Host + return http.DefaultClient.Do(req) + }, + MockCloseIdleConnections: func() { + http.DefaultClient.CloseIdleConnections() + }, + } + + // we need to convince the client that we're logged in first otherwise it will + // refuse to send a request to the server and we won't be testing networking + runtimex.Try0(client.StateFile.Set(State{ + ClientID: "ttt-uuu-iii", + Expire: time.Now().Add(30 * time.Hour), + Password: "xxx-xxx-xxx", + Token: "abc-yyy-zzz", + })) + + // run the tor flow + targets, err := torflow(t, client) + + // we do expect an error + if err == nil || err.Error() != "unexpected end of JSON input" { + t.Fatal("unexpected error", err) + } + + // we expect to see zero-length targets + if len(targets) != 0 { + t.Fatal("expected targets to be zero length") + } + }) + + t.Run("when we're not registered", func(t *testing.T) { + clnt := newclient() + + // With explicitly empty state so it's pretty obvioust there's no state + state := State{} + + // force the state to be empty + if err := clnt.StateFile.Set(state); err != nil { + t.Fatal(err) + } + + // run the tor flow + targets, err := clnt.FetchTorTargets(context.Background(), "ZZ") + + // ensure that the error says we're not registered + if !errors.Is(err, ErrNotRegistered) { + t.Fatal("expected an error here") + } + + // we expect zero length targets + if len(targets) != 0 { + t.Fatal("expected zero-length targets") + } + }) + + t.Run("correctly handles the case where the URL is unparseable", func(t *testing.T) { + // create a probeservices client + client := newclient() + + // override the URL to be unparseable + client.BaseURL = "\t\t\t" + + // we need to convince the client that we're logged in first otherwise it will + // refuse to send a request to the server and we won't be testing networking + runtimex.Try0(client.StateFile.Set(State{ + ClientID: "ttt-uuu-iii", + Expire: time.Now().Add(30 * time.Hour), + Password: "xxx-xxx-xxx", + Token: "abc-yyy-zzz", + })) + + targets, err := client.FetchTorTargets(context.Background(), "ZZ") + + // we do expect an error + if err == nil || err.Error() != `parse "\t\t\t": net/url: invalid control character in URL` { + t.Fatal("unexpected error", err) + } + + // we expect zero length targets + if len(targets) != 0 { + t.Fatal("expected zero-length targets") + } + }) - clnt := newclient() - txp := new(FetchTorTargetsHTTPTransport) - clnt.HTTPClient = &http.Client{Transport: txp} - if err := clnt.MaybeRegister(context.Background(), MetadataFixture()); err != nil { - t.Fatal(err) - } - if err := clnt.MaybeLogin(context.Background()); err != nil { - t.Fatal(err) - } - data, err := clnt.FetchTorTargets(context.Background(), "ZZ") - if err != nil { - t.Fatal(err) - } - if data == nil || len(data) <= 0 { - t.Fatal("invalid data") - } - requestURL := txp.Response.Request.URL - if requestURL.Query().Get("country_code") != "ZZ" { - t.Fatal("invalid country code") - } }