diff --git a/internal/enginenetx/httpsdialerstats_internal_test.go b/internal/enginenetx/httpsdialerstats_internal_test.go deleted file mode 100644 index c40b0e1f02..0000000000 --- a/internal/enginenetx/httpsdialerstats_internal_test.go +++ /dev/null @@ -1,416 +0,0 @@ -package enginenetx - -import ( - "context" - "encoding/json" - "errors" - "testing" - "time" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ooni/probe-cli/v3/internal/kvstore" - "github.com/ooni/probe-cli/v3/internal/mocks" - "github.com/ooni/probe-cli/v3/internal/runtimex" -) - -func TestLoadHTTPSDialerStatsRootContainer(t *testing.T) { - type testcase struct { - // name is the test case name - name string - - // input returns the bytes we should Set into the key-value store - input func() []byte - - // expectedErr is the expected error string or an empty string - expectErr string - - // expectRoot is the expected root container content - expectRoot *HTTPSDialerStatsRootContainer - } - - cases := []testcase{{ - name: "when the key-value store does not contain any data", - input: func() []byte { - // Note that returning nil causes the code to NOT set anything into the kvstore - return nil - }, - expectErr: "no such key", - expectRoot: nil, - }, { - name: "when we cannot parse the serialized JSON", - input: func() []byte { - return []byte(`{`) - }, - expectErr: "unexpected end of JSON input", - expectRoot: nil, - }, { - name: "with invalid version", - input: func() []byte { - return []byte(`{"Version":1}`) - }, - expectErr: "httpsdialerstats.state: wrong stats container version: expected=2 got=1", - expectRoot: nil, - }, { - name: "on success", - input: func() []byte { - root := &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{ - "api.ooni.io": { - Tactics: map[string]*HTTPSDialerStatsTacticRecord{ - "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { - CountStarted: 4, - CountTCPConnectError: 1, - CountTLSHandshakeError: 1, - CountTLSVerificationError: 1, - CountSuccess: 1, - HistoTCPConnectError: map[string]int64{ - "connection_refused": 1, - }, - HistoTLSHandshakeError: map[string]int64{ - "generic_timeout_error": 1, - }, - HistoTLSVerificationError: map[string]int64{ - "ssl_invalid_hostname": 1, - }, - LastUpdated: time.Date(2023, 9, 25, 0, 0, 0, 0, time.UTC), - Tactic: &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, - }, - }, - }, - }, - Version: HTTPSDialerStatsContainerVersion, - } - return runtimex.Try1(json.Marshal(root)) - }, - expectErr: "", - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{ - "api.ooni.io": { - Tactics: map[string]*HTTPSDialerStatsTacticRecord{ - "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { - CountStarted: 4, - CountTCPConnectError: 1, - CountTLSHandshakeError: 1, - CountTLSVerificationError: 1, - CountSuccess: 1, - HistoTCPConnectError: map[string]int64{ - "connection_refused": 1, - }, - HistoTLSHandshakeError: map[string]int64{ - "generic_timeout_error": 1, - }, - HistoTLSVerificationError: map[string]int64{ - "ssl_invalid_hostname": 1, - }, - LastUpdated: time.Date(2023, 9, 25, 0, 0, 0, 0, time.UTC), - Tactic: &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, - }, - }, - }, - }, - Version: HTTPSDialerStatsContainerVersion, - }, - }} - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - kvStore := &kvstore.Memory{} - if input := tc.input(); len(input) > 0 { - if err := kvStore.Set(HTTPSDialerStatsKey, input); err != nil { - t.Fatal(err) - } - } - - root, err := loadHTTPSDialerStatsRootContainer(kvStore) - - switch { - case err == nil && tc.expectErr == "": - // all good - - case err != nil && tc.expectErr == "": - t.Fatal("expected", tc.expectErr, "but got", err.Error()) - - case err == nil && tc.expectErr != "": - t.Fatal("expected", tc.expectErr, "but got", err) - - case err != nil && tc.expectErr != "": - if tc.expectErr != err.Error() { - t.Fatal("expected", tc.expectErr, "but got", err.Error()) - } - } - - if diff := cmp.Diff(tc.expectRoot, root); diff != "" { - t.Fatal(diff) - } - }) - } -} - -func TestHTTPSDialerStatsManagerCallbacks(t *testing.T) { - type testcase struct { - name string - initialRoot *HTTPSDialerStatsRootContainer - do func(stats *HTTPSDialerStatsManager) - expectWarnf int - expectRoot *HTTPSDialerStatsRootContainer - } - - cases := []testcase{ - - // When TCP connect fails and the reason is a canceled context - { - name: "OnTCPConnectError with ctx.Error() != nil", - initialRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{ - "api.ooni.io": { - Tactics: map[string]*HTTPSDialerStatsTacticRecord{ - "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { - CountStarted: 1, - }, - }, - }, - }, - Version: HTTPSDialerStatsContainerVersion, - }, - do: func(stats *HTTPSDialerStatsManager) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() // immediately! - - tactic := &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - err := errors.New("generic_timeout_error") - - stats.OnTCPConnectError(ctx, tactic, err) - }, - expectWarnf: 0, - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{ - "api.ooni.io": { - Tactics: map[string]*HTTPSDialerStatsTacticRecord{ - "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { - CountStarted: 1, - CountTCPConnectInterrupt: 1, - }, - }, - }, - }, - Version: HTTPSDialerStatsContainerVersion, - }, - }, - - // When TCP connect fails and we don't already have a policy record - { - name: "OnTCPConnectError when we are missing the stats record for the domain", - initialRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - do: func(stats *HTTPSDialerStatsManager) { - ctx := context.Background() - - tactic := &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - err := errors.New("generic_timeout_error") - - stats.OnTCPConnectError(ctx, tactic, err) - }, - expectWarnf: 1, - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - }, - - // When TLS handshake fails and the reason is a canceled context - { - name: "OnTLSHandshakeError with ctx.Error() != nil", - initialRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{ - "api.ooni.io": { - Tactics: map[string]*HTTPSDialerStatsTacticRecord{ - "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { - CountStarted: 1, - }, - }, - }, - }, - Version: HTTPSDialerStatsContainerVersion, - }, - do: func(stats *HTTPSDialerStatsManager) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() // immediately! - - tactic := &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - err := errors.New("generic_timeout_error") - - stats.OnTLSHandshakeError(ctx, tactic, err) - }, - expectWarnf: 0, - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{ - "api.ooni.io": { - Tactics: map[string]*HTTPSDialerStatsTacticRecord{ - "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { - CountStarted: 1, - CountTLSHandshakeInterrupt: 1, - }, - }, - }, - }, - Version: HTTPSDialerStatsContainerVersion, - }, - }, - - // When TLS handshake fails and we don't already have a policy record - { - name: "OnTLSHandshakeError when we are missing the stats record for the domain", - initialRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - do: func(stats *HTTPSDialerStatsManager) { - ctx := context.Background() - - tactic := &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - err := errors.New("generic_timeout_error") - - stats.OnTLSHandshakeError(ctx, tactic, err) - }, - expectWarnf: 1, - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - }, - - // When TLS verification fails and we don't already have a policy record - { - name: "OnTLSVerifyError when we are missing the stats record for the domain", - initialRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - do: func(stats *HTTPSDialerStatsManager) { - tactic := &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - err := errors.New("generic_timeout_error") - - stats.OnTLSVerifyError(tactic, err) - }, - expectWarnf: 1, - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - }, - - // With success when we don't already have a policy record - { - name: "OnSuccess when we are missing the stats record for the domain", - initialRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - do: func(stats *HTTPSDialerStatsManager) { - tactic := &HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - - stats.OnSuccess(tactic) - }, - expectWarnf: 1, - expectRoot: &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, - }, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - // configure the initial value of the stats - kvStore := &kvstore.Memory{} - if err := kvStore.Set(HTTPSDialerStatsKey, runtimex.Try1(json.Marshal(tc.initialRoot))); err != nil { - t.Fatal(err) - } - - // create logger counting the number Warnf invocations - var warnfCount int - logger := &mocks.Logger{ - MockWarnf: func(format string, v ...any) { - warnfCount++ - }, - } - - // create the stats manager - stats := NewHTTPSDialerStatsManager(kvStore, logger) - - // invoke the proper stats callback - tc.do(stats) - - // close the stats to trigger a kvstore write - if err := stats.Close(); err != nil { - t.Fatal(err) - } - - // extract the possibly modified stats from the kvstore - var root *HTTPSDialerStatsRootContainer - rawRoot, err := kvStore.Get(HTTPSDialerStatsKey) - if err != nil { - t.Fatal(err) - } - if err := json.Unmarshal(rawRoot, &root); err != nil { - t.Fatal(err) - } - - // make sure the stats are the ones we expect - diffOptions := []cmp.Option{ - cmpopts.IgnoreFields(HTTPSDialerStatsTacticRecord{}, "LastUpdated"), - } - if diff := cmp.Diff(tc.expectRoot, root, diffOptions...); diff != "" { - t.Fatal(diff) - } - - // make sure we logged if necessary - if tc.expectWarnf != warnfCount { - t.Fatal("expected", tc.expectWarnf, "got", warnfCount) - } - }) - } -} diff --git a/internal/enginenetx/httpsdialerstats_test.go b/internal/enginenetx/httpsdialerstats_test.go deleted file mode 100644 index 3ff5fdb5b2..0000000000 --- a/internal/enginenetx/httpsdialerstats_test.go +++ /dev/null @@ -1,279 +0,0 @@ -package enginenetx_test - -import ( - "encoding/json" - "net" - "testing" - "time" - - "github.com/apex/log" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ooni/netem" - "github.com/ooni/probe-cli/v3/internal/bytecounter" - "github.com/ooni/probe-cli/v3/internal/enginenetx" - "github.com/ooni/probe-cli/v3/internal/kvstore" - "github.com/ooni/probe-cli/v3/internal/netemx" - "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/runtimex" -) - -func TestHTTPSDialerCollectStats(t *testing.T) { - // testcase is a test case run by this function - type testcase struct { - // name is the test case name - name string - - // URL is the URL to GET - URL string - - // initialPolicy is the initial policy to configure into the key-value store - initialPolicy func() []byte - - // configureDPI is the function to configure DPI - configureDPI func(dpi *netem.DPIEngine) - - // expectErr is the expected error string - expectErr string - - // statsDomain is the domain to lookup inside the stats - statsDomain string - - // statsTacticsSummary is the summary to lookup inside the stats - // once we have used the statsDomain to get a record - statsTacticsSummary string - - // expectStats contains the expected record containing tactics stats - expectStats *enginenetx.HTTPSDialerStatsTacticRecord - } - - cases := []testcase{ - - { - name: "with TCP connect failure", - URL: "https://api.ooni.io/", - initialPolicy: func() []byte { - p0 := &enginenetx.HTTPSDialerStaticPolicyRoot{ - Domains: map[string][]*enginenetx.HTTPSDialerTactic{ - // This policy has a different SNI and VerifyHostname, which gives - // us confidence that the stats are using the latter - "api.ooni.io": {{ - Endpoint: net.JoinHostPort(netemx.AddressApiOONIIo, "443"), - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }}, - }, - Version: enginenetx.HTTPSDialerStaticPolicyVersion, - } - return runtimex.Try1(json.Marshal(p0)) - }, - configureDPI: func(dpi *netem.DPIEngine) { - dpi.AddRule(&netem.DPICloseConnectionForServerEndpoint{ - Logger: log.Log, - ServerIPAddress: netemx.AddressApiOONIIo, - ServerPort: 443, - }) - }, - expectErr: `Get "https://api.ooni.io/": connection_refused`, - statsDomain: "api.ooni.io", - statsTacticsSummary: "162.55.247.208:443 sni=www.example.com verify=api.ooni.io", - expectStats: &enginenetx.HTTPSDialerStatsTacticRecord{ - CountStarted: 1, - CountTCPConnectError: 1, - CountTLSHandshakeError: 0, - CountTLSVerificationError: 0, - CountSuccess: 0, - HistoTCPConnectError: map[string]int64{ - "connection_refused": 1, - }, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: time.Time{}, - Tactic: &enginenetx.HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, - }, - }, - - { - name: "with TLS handshake failure", - URL: "https://api.ooni.io/", - initialPolicy: func() []byte { - p0 := &enginenetx.HTTPSDialerStaticPolicyRoot{ - Domains: map[string][]*enginenetx.HTTPSDialerTactic{ - // This policy has a different SNI and VerifyHostname, which gives - // us confidence that the stats are using the latter - "api.ooni.io": {{ - Endpoint: net.JoinHostPort(netemx.AddressApiOONIIo, "443"), - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }}, - }, - Version: enginenetx.HTTPSDialerStaticPolicyVersion, - } - return runtimex.Try1(json.Marshal(p0)) - }, - configureDPI: func(dpi *netem.DPIEngine) { - dpi.AddRule(&netem.DPIResetTrafficForTLSSNI{ - Logger: log.Log, - SNI: "www.example.com", - }) - }, - expectErr: `Get "https://api.ooni.io/": connection_reset`, - statsDomain: "api.ooni.io", - statsTacticsSummary: "162.55.247.208:443 sni=www.example.com verify=api.ooni.io", - expectStats: &enginenetx.HTTPSDialerStatsTacticRecord{ - CountStarted: 1, - CountTCPConnectError: 0, - CountTLSHandshakeError: 1, - CountTLSVerificationError: 0, - CountSuccess: 0, - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{ - "connection_reset": 1, - }, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: time.Time{}, - Tactic: &enginenetx.HTTPSDialerTactic{ - Endpoint: "162.55.247.208:443", - InitialDelay: 0, - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, - }, - }, - - { - name: "with TLS verification failure", - URL: "https://api.ooni.io/", - initialPolicy: func() []byte { - p0 := &enginenetx.HTTPSDialerStaticPolicyRoot{ - Domains: map[string][]*enginenetx.HTTPSDialerTactic{ - // This policy has a different SNI and VerifyHostname, which gives - // us confidence that the stats are using the latter - "api.ooni.io": {{ - Endpoint: net.JoinHostPort(netemx.AddressBadSSLCom, "443"), - InitialDelay: 0, - SNI: "untrusted-root.badssl.com", - VerifyHostname: "api.ooni.io", - }}, - }, - Version: enginenetx.HTTPSDialerStaticPolicyVersion, - } - return runtimex.Try1(json.Marshal(p0)) - }, - configureDPI: func(dpi *netem.DPIEngine) { - // nothing - }, - expectErr: `Get "https://api.ooni.io/": ssl_invalid_hostname`, - statsDomain: "api.ooni.io", - statsTacticsSummary: "104.154.89.105:443 sni=untrusted-root.badssl.com verify=api.ooni.io", - expectStats: &enginenetx.HTTPSDialerStatsTacticRecord{ - CountStarted: 1, - CountTCPConnectError: 0, - CountTLSHandshakeError: 0, - CountTLSVerificationError: 1, - CountSuccess: 0, - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{ - "ssl_invalid_hostname": 1, - }, - LastUpdated: time.Time{}, - Tactic: &enginenetx.HTTPSDialerTactic{ - Endpoint: "104.154.89.105:443", - InitialDelay: 0, - SNI: "untrusted-root.badssl.com", - VerifyHostname: "api.ooni.io", - }, - }, - }} - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - qa := netemx.MustNewScenario(netemx.InternetScenario) - defer qa.Close() - - // make sure we apply specific DPI rules - tc.configureDPI(qa.DPIEngine()) - - // create a memory key-value store where the engine will write stats that later we - // would be able to read to confirm we're collecting stats - kvStore := &kvstore.Memory{} - - initialPolicy := tc.initialPolicy() - t.Logf("initialPolicy: %s", string(initialPolicy)) - if err := kvStore.Set(enginenetx.HTTPSDialerStaticPolicyKey, initialPolicy); err != nil { - t.Fatal(err) - } - - qa.Do(func() { - byteCounter := bytecounter.New() - resolver := netxlite.NewStdlibResolver(log.Log) - - netx := enginenetx.NewNetwork(byteCounter, kvStore, log.Log, nil, resolver) - defer netx.Close() - - client := netx.NewHTTPClient() - - resp, err := client.Get(tc.URL) - - switch { - case err == nil && tc.expectErr == "": - // all good - - case err != nil && tc.expectErr == "": - t.Fatal("expected", tc.expectErr, "but got", err.Error()) - - case err == nil && tc.expectErr != "": - t.Fatal("expected", tc.expectErr, "but got", err) - - case err != nil && tc.expectErr != "": - if tc.expectErr != err.Error() { - t.Fatal("expected", tc.expectErr, "but got", err.Error()) - } - } - - if resp != nil { - defer resp.Body.Close() - } - }) - - // obtain the tactics container for the proper domain - rawStats, err := kvStore.Get(enginenetx.HTTPSDialerStatsKey) - if err != nil { - t.Fatal(err) - } - var rootStats enginenetx.HTTPSDialerStatsRootContainer - if err := json.Unmarshal(rawStats, &rootStats); err != nil { - t.Fatal(err) - } - tactics, good := rootStats.Domains[tc.statsDomain] - if !good { - t.Fatalf("no such record for `%s`", tc.statsDomain) - } - t.Logf("%+v", tactics) - - // we expect to see a single record - if len(tactics.Tactics) != 1 { - t.Fatal("expected a single tactic") - } - tactic, good := tactics.Tactics[tc.statsTacticsSummary] - if !good { - t.Fatalf("no such record for: %s", tc.statsTacticsSummary) - } - - diffOptions := []cmp.Option{ - cmpopts.IgnoreFields(enginenetx.HTTPSDialerStatsTacticRecord{}, "LastUpdated"), - } - if diff := cmp.Diff(tc.expectStats, tactic, diffOptions...); diff != "" { - t.Fatal(diff) - } - }) - } -} diff --git a/internal/enginenetx/network.go b/internal/enginenetx/network.go index dc5f51a737..1669a8bc38 100644 --- a/internal/enginenetx/network.go +++ b/internal/enginenetx/network.go @@ -15,7 +15,7 @@ import ( // Network is the network abstraction used by the OONI engine. type Network struct { reso model.Resolver - stats *HTTPSDialerStatsManager + stats *statsManager txp model.HTTPTransport } @@ -84,7 +84,7 @@ func NewNetwork( dialer := netxlite.NewDialerWithResolver(logger, resolver) // Create manager for keeping track of statistics - stats := NewHTTPSDialerStatsManager(kvStore, logger) + stats := newStatsManager(kvStore, logger) // Create a TLS dialer ONLY used for dialing TLS connections. This dialer will use // happy-eyeballs and possibly custom policies for dialing TLS connections. diff --git a/internal/enginenetx/network_internal_test.go b/internal/enginenetx/network_internal_test.go index 550add880b..741f183ed4 100644 --- a/internal/enginenetx/network_internal_test.go +++ b/internal/enginenetx/network_internal_test.go @@ -31,11 +31,11 @@ func TestNetworkUnit(t *testing.T) { // nothing }, }, - stats: &HTTPSDialerStatsManager{ + stats: &statsManager{ kvStore: &kvstore.Memory{}, logger: model.DiscardLogger, mu: sync.Mutex{}, - root: &HTTPSDialerStatsRootContainer{}, + root: &statsContainer{}, }, txp: expected, } @@ -56,11 +56,11 @@ func TestNetworkUnit(t *testing.T) { } netx := &Network{ reso: expected, - stats: &HTTPSDialerStatsManager{ + stats: &statsManager{ kvStore: &kvstore.Memory{}, logger: model.DiscardLogger, mu: sync.Mutex{}, - root: &HTTPSDialerStatsRootContainer{}, + root: &statsContainer{}, }, txp: &mocks.HTTPTransport{ MockCloseIdleConnections: func() { diff --git a/internal/enginenetx/httpsdialerstats.go b/internal/enginenetx/stats.go similarity index 60% rename from internal/enginenetx/httpsdialerstats.go rename to internal/enginenetx/stats.go index cbf16ad2d4..56fd97a335 100644 --- a/internal/enginenetx/httpsdialerstats.go +++ b/internal/enginenetx/stats.go @@ -12,8 +12,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/runtimex" ) -// HTTPSDialerStatsTacticRecord keeps stats about an [HTTPSDialerTactic]. -type HTTPSDialerStatsTacticRecord struct { +// statsTactic keeps stats about an [*HTTPSDialerTactic]. +type statsTactic struct { // CountStarted counts the number of operations we started. CountStarted int64 @@ -51,30 +51,29 @@ type HTTPSDialerStatsTacticRecord struct { Tactic *HTTPSDialerTactic } -// HTTPSDialerStatsTacticsContainer contains tactics. -type HTTPSDialerStatsTacticsContainer struct { - // Tactic maps the summary of a tactic to the tactic record. - Tactics map[string]*HTTPSDialerStatsTacticRecord +// statsDomain contains stats associated with a domain. +type statsDomain struct { + Tactics map[string]*statsTactic } -// HTTPSDialerStatsContainerVersion is the current version of [HTTPSDialerStatsContainer]. -const HTTPSDialerStatsContainerVersion = 2 +// statsContainerVersion is the current version of [statsContainer]. +const statsContainerVersion = 2 -// HTTPSDialerStatsRootContainer is the root container for stats. +// statsContainer is the root container for stats. // -// The zero value is invalid; construct using [NewHTTPSDialerStatsRootContainer]. -type HTTPSDialerStatsRootContainer struct { +// The zero value is invalid; construct using [newStatsContainer]. +type statsContainer struct { // Domains maps a domain name to its tactics - Domains map[string]*HTTPSDialerStatsTacticsContainer + Domains map[string]*statsDomain // Version is the version of the container data format. Version int } -// Get returns the tactic record for the given [*HTTPSDialerTactic] instance. +// Get returns the tactic record for the given [*statsTactic] instance. // -// At the name implies, this function MUST be called while holding the [HTTPSDialerStatsManager] mutex. -func (c *HTTPSDialerStatsRootContainer) GetLocked(tactic *HTTPSDialerTactic) (*HTTPSDialerStatsTacticRecord, bool) { +// At the name implies, this function MUST be called while holding the [*statsManager] mutex. +func (c *statsContainer) GetLocked(tactic *HTTPSDialerTactic) (*statsTactic, bool) { domainRecord, found := c.Domains[tactic.VerifyHostname] if !found { return nil, false @@ -83,19 +82,19 @@ func (c *HTTPSDialerStatsRootContainer) GetLocked(tactic *HTTPSDialerTactic) (*H return tacticRecord, found } -// Set sets the tactic record for the given the given [*HTTPSDialerTactic] instance. +// Set sets the tactic record for the given the given [*statsTactic] instance. // -// At the name implies, this function MUST be called while holding the [HTTPSDialerStatsManager] mutex. -func (c *HTTPSDialerStatsRootContainer) SetLocked(tactic *HTTPSDialerTactic, record *HTTPSDialerStatsTacticRecord) { +// At the name implies, this function MUST be called while holding the [*statsManager] mutex. +func (c *statsContainer) SetLocked(tactic *HTTPSDialerTactic, record *statsTactic) { domainRecord, found := c.Domains[tactic.VerifyHostname] if !found { - domainRecord = &HTTPSDialerStatsTacticsContainer{ - Tactics: map[string]*HTTPSDialerStatsTacticRecord{}, + domainRecord = &statsDomain{ + Tactics: map[string]*statsTactic{}, } // make sure the map is initialized if len(c.Domains) <= 0 { - c.Domains = make(map[string]*HTTPSDialerStatsTacticsContainer) + c.Domains = make(map[string]*statsDomain) } c.Domains[tactic.VerifyHostname] = domainRecord @@ -104,20 +103,20 @@ func (c *HTTPSDialerStatsRootContainer) SetLocked(tactic *HTTPSDialerTactic, rec domainRecord.Tactics[tactic.Summary()] = record } -// NewHTTPSDialerStatsRootContainer creates a new empty [*HTTPSDialerStatsRootContainer]. -func NewHTTPSDialerStatsRootContainer() *HTTPSDialerStatsRootContainer { - return &HTTPSDialerStatsRootContainer{ - Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, - Version: HTTPSDialerStatsContainerVersion, +// newStatsContainer creates a new empty [*statsContainer]. +func newStatsContainer() *statsContainer { + return &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, } } -// HTTPSDialerStatsManager implements [HTTPSDialerStatsTracker] by storing +// statsManager implements [HTTPSDialerStatsTracker] by storing // the relevant statistics in a [model.KeyValueStore]. // // The zero value of this structure is not ready to use; please, use the -// [NewHTTPSDialerStatsManager] factory to create a new instance. -type HTTPSDialerStatsManager struct { +// [newStatsManager] factory to create a new instance. +type statsManager struct { // kvStore is the key-value store we're using kvStore model.KeyValueStore @@ -128,36 +127,36 @@ type HTTPSDialerStatsManager struct { mu sync.Mutex // root is the root container for stats - root *HTTPSDialerStatsRootContainer + root *statsContainer } -// HTTPSDialerStatsKey is the key used in the key-value store to access the state. -const HTTPSDialerStatsKey = "httpsdialerstats.state" +// statsKey is the key used in the key-value store to access the state. +const statsKey = "httpsdialerstats.state" -// errDialerStatsContainerWrongVersion means that the stats container document has the wrong version number. -var errDialerStatsContainerWrongVersion = errors.New("wrong stats container version") +// errStatsContainerWrongVersion means that the stats container document has the wrong version number. +var errStatsContainerWrongVersion = errors.New("wrong stats container version") -// loadHTTPSDialerStatsRootContainer loads a state container from the given key-value store. -func loadHTTPSDialerStatsRootContainer(kvStore model.KeyValueStore) (*HTTPSDialerStatsRootContainer, error) { +// loadStatsContainer loads a state container from the given key-value store. +func loadStatsContainer(kvStore model.KeyValueStore) (*statsContainer, error) { // load data from the kvstore - data, err := kvStore.Get(HTTPSDialerStatsKey) + data, err := kvStore.Get(statsKey) if err != nil { return nil, err } // parse as JSON - var container HTTPSDialerStatsRootContainer + var container statsContainer if err := json.Unmarshal(data, &container); err != nil { return nil, err } // make sure the version is OK - if container.Version != HTTPSDialerStatsContainerVersion { + if container.Version != statsContainerVersion { err := fmt.Errorf( "%s: %w: expected=%d got=%d", - HTTPSDialerStatsKey, - errDialerStatsContainerWrongVersion, - HTTPSDialerStatsContainerVersion, + statsKey, + errStatsContainerWrongVersion, + statsContainerVersion, container.Version, ) return nil, err @@ -166,14 +165,14 @@ func loadHTTPSDialerStatsRootContainer(kvStore model.KeyValueStore) (*HTTPSDiale return &container, nil } -// NewHTTPSDialerStatsManager constructs a new instance of [*HTTPSDialerStatsManager]. -func NewHTTPSDialerStatsManager(kvStore model.KeyValueStore, logger model.Logger) *HTTPSDialerStatsManager { - root, err := loadHTTPSDialerStatsRootContainer(kvStore) +// newStatsManager constructs a new instance of [*statsManager]. +func newStatsManager(kvStore model.KeyValueStore, logger model.Logger) *statsManager { + root, err := loadStatsContainer(kvStore) if err != nil { - root = NewHTTPSDialerStatsRootContainer() + root = newStatsContainer() } - return &HTTPSDialerStatsManager{ + return &statsManager{ root: root, kvStore: kvStore, logger: logger, @@ -181,10 +180,10 @@ func NewHTTPSDialerStatsManager(kvStore model.KeyValueStore, logger model.Logger } } -var _ HTTPSDialerStatsTracker = &HTTPSDialerStatsManager{} +var _ HTTPSDialerStatsTracker = &statsManager{} // OnStarting implements HTTPSDialerStatsManager. -func (mt *HTTPSDialerStatsManager) OnStarting(tactic *HTTPSDialerTactic) { +func (mt *statsManager) OnStarting(tactic *HTTPSDialerTactic) { // get exclusive access defer mt.mu.Unlock() mt.mu.Lock() @@ -192,7 +191,7 @@ func (mt *HTTPSDialerStatsManager) OnStarting(tactic *HTTPSDialerTactic) { // get the record record, found := mt.root.GetLocked(tactic) if !found { - record = &HTTPSDialerStatsTacticRecord{ + record = &statsTactic{ CountStarted: 0, CountTCPConnectError: 0, CountTCPConnectInterrupt: 0, @@ -215,7 +214,7 @@ func (mt *HTTPSDialerStatsManager) OnStarting(tactic *HTTPSDialerTactic) { } // OnTCPConnectError implements HTTPSDialerStatsManager. -func (mt *HTTPSDialerStatsManager) OnTCPConnectError(ctx context.Context, tactic *HTTPSDialerTactic, err error) { +func (mt *statsManager) OnTCPConnectError(ctx context.Context, tactic *HTTPSDialerTactic, err error) { // get exclusive access defer mt.mu.Unlock() mt.mu.Lock() @@ -238,7 +237,7 @@ func (mt *HTTPSDialerStatsManager) OnTCPConnectError(ctx context.Context, tactic } // OnTLSHandshakeError implements HTTPSDialerStatsManager. -func (mt *HTTPSDialerStatsManager) OnTLSHandshakeError(ctx context.Context, tactic *HTTPSDialerTactic, err error) { +func (mt *statsManager) OnTLSHandshakeError(ctx context.Context, tactic *HTTPSDialerTactic, err error) { // get exclusive access defer mt.mu.Unlock() mt.mu.Lock() @@ -261,7 +260,7 @@ func (mt *HTTPSDialerStatsManager) OnTLSHandshakeError(ctx context.Context, tact } // OnTLSVerifyError implements HTTPSDialerStatsManager. -func (mt *HTTPSDialerStatsManager) OnTLSVerifyError(tactic *HTTPSDialerTactic, err error) { +func (mt *statsManager) OnTLSVerifyError(tactic *HTTPSDialerTactic, err error) { // get exclusive access defer mt.mu.Unlock() mt.mu.Lock() @@ -280,7 +279,7 @@ func (mt *HTTPSDialerStatsManager) OnTLSVerifyError(tactic *HTTPSDialerTactic, e } // OnSuccess implements HTTPSDialerStatsManager. -func (mt *HTTPSDialerStatsManager) OnSuccess(tactic *HTTPSDialerTactic) { +func (mt *statsManager) OnSuccess(tactic *HTTPSDialerTactic) { // get exclusive access defer mt.mu.Unlock() mt.mu.Lock() @@ -298,7 +297,7 @@ func (mt *HTTPSDialerStatsManager) OnSuccess(tactic *HTTPSDialerTactic) { } // Close implements io.Closer -func (mt *HTTPSDialerStatsManager) Close() error { +func (mt *statsManager) Close() error { // TODO(bassosimone): do we need to apply a "once" semantics to this method? // get exclusive access @@ -306,5 +305,5 @@ func (mt *HTTPSDialerStatsManager) Close() error { mt.mu.Lock() // write updated stats into the underlying key-value store - return mt.kvStore.Set(HTTPSDialerStatsKey, runtimex.Try1(json.Marshal(mt.root))) + return mt.kvStore.Set(statsKey, runtimex.Try1(json.Marshal(mt.root))) } diff --git a/internal/enginenetx/stats_test.go b/internal/enginenetx/stats_test.go new file mode 100644 index 0000000000..053c3136da --- /dev/null +++ b/internal/enginenetx/stats_test.go @@ -0,0 +1,683 @@ +package enginenetx + +import ( + "context" + "encoding/json" + "errors" + "net" + "testing" + "time" + + "github.com/apex/log" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/netemx" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// This test ensures that a [*Network] created with [NewNetwork] collects stats. +func TestNetworkCollectsStats(t *testing.T) { + // testcase is a test case run by this function + type testcase struct { + // name is the test case name + name string + + // URL is the URL to GET + URL string + + // initialPolicy is the initial policy to configure into the key-value store + initialPolicy func() []byte + + // configureDPI is the function to configure DPI + configureDPI func(dpi *netem.DPIEngine) + + // expectErr is the expected error string + expectErr string + + // statsDomain is the domain to lookup inside the stats + statsDomain string + + // statsTacticsSummary is the summary to lookup inside the stats + // once we have used the statsDomain to get a record + statsTacticsSummary string + + // expectStats contains the expected record containing tactics stats + expectStats *statsTactic + } + + cases := []testcase{ + + { + name: "with TCP connect failure", + URL: "https://api.ooni.io/", + initialPolicy: func() []byte { + p0 := &HTTPSDialerStaticPolicyRoot{ + Domains: map[string][]*HTTPSDialerTactic{ + // This policy has a different SNI and VerifyHostname, which gives + // us confidence that the stats are using the latter + "api.ooni.io": {{ + Endpoint: net.JoinHostPort(netemx.AddressApiOONIIo, "443"), + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }}, + }, + Version: HTTPSDialerStaticPolicyVersion, + } + return runtimex.Try1(json.Marshal(p0)) + }, + configureDPI: func(dpi *netem.DPIEngine) { + dpi.AddRule(&netem.DPICloseConnectionForServerEndpoint{ + Logger: log.Log, + ServerIPAddress: netemx.AddressApiOONIIo, + ServerPort: 443, + }) + }, + expectErr: `Get "https://api.ooni.io/": connection_refused`, + statsDomain: "api.ooni.io", + statsTacticsSummary: "162.55.247.208:443 sni=www.example.com verify=api.ooni.io", + expectStats: &statsTactic{ + CountStarted: 1, + CountTCPConnectError: 1, + CountTLSHandshakeError: 0, + CountTLSVerificationError: 0, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{ + "connection_refused": 1, + }, + HistoTLSHandshakeError: map[string]int64{}, + HistoTLSVerificationError: map[string]int64{}, + LastUpdated: time.Time{}, + Tactic: &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + + { + name: "with TLS handshake failure", + URL: "https://api.ooni.io/", + initialPolicy: func() []byte { + p0 := &HTTPSDialerStaticPolicyRoot{ + Domains: map[string][]*HTTPSDialerTactic{ + // This policy has a different SNI and VerifyHostname, which gives + // us confidence that the stats are using the latter + "api.ooni.io": {{ + Endpoint: net.JoinHostPort(netemx.AddressApiOONIIo, "443"), + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }}, + }, + Version: HTTPSDialerStaticPolicyVersion, + } + return runtimex.Try1(json.Marshal(p0)) + }, + configureDPI: func(dpi *netem.DPIEngine) { + dpi.AddRule(&netem.DPIResetTrafficForTLSSNI{ + Logger: log.Log, + SNI: "www.example.com", + }) + }, + expectErr: `Get "https://api.ooni.io/": connection_reset`, + statsDomain: "api.ooni.io", + statsTacticsSummary: "162.55.247.208:443 sni=www.example.com verify=api.ooni.io", + expectStats: &statsTactic{ + CountStarted: 1, + CountTCPConnectError: 0, + CountTLSHandshakeError: 1, + CountTLSVerificationError: 0, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{}, + HistoTLSHandshakeError: map[string]int64{ + "connection_reset": 1, + }, + HistoTLSVerificationError: map[string]int64{}, + LastUpdated: time.Time{}, + Tactic: &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + + { + name: "with TLS verification failure", + URL: "https://api.ooni.io/", + initialPolicy: func() []byte { + p0 := &HTTPSDialerStaticPolicyRoot{ + Domains: map[string][]*HTTPSDialerTactic{ + // This policy has a different SNI and VerifyHostname, which gives + // us confidence that the stats are using the latter + "api.ooni.io": {{ + Endpoint: net.JoinHostPort(netemx.AddressBadSSLCom, "443"), + InitialDelay: 0, + SNI: "untrusted-root.badssl.com", + VerifyHostname: "api.ooni.io", + }}, + }, + Version: HTTPSDialerStaticPolicyVersion, + } + return runtimex.Try1(json.Marshal(p0)) + }, + configureDPI: func(dpi *netem.DPIEngine) { + // nothing + }, + expectErr: `Get "https://api.ooni.io/": ssl_invalid_hostname`, + statsDomain: "api.ooni.io", + statsTacticsSummary: "104.154.89.105:443 sni=untrusted-root.badssl.com verify=api.ooni.io", + expectStats: &statsTactic{ + CountStarted: 1, + CountTCPConnectError: 0, + CountTLSHandshakeError: 0, + CountTLSVerificationError: 1, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{}, + HistoTLSHandshakeError: map[string]int64{}, + HistoTLSVerificationError: map[string]int64{ + "ssl_invalid_hostname": 1, + }, + LastUpdated: time.Time{}, + Tactic: &HTTPSDialerTactic{ + Endpoint: "104.154.89.105:443", + InitialDelay: 0, + SNI: "untrusted-root.badssl.com", + VerifyHostname: "api.ooni.io", + }, + }, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + qa := netemx.MustNewScenario(netemx.InternetScenario) + defer qa.Close() + + // make sure we apply specific DPI rules + tc.configureDPI(qa.DPIEngine()) + + // create a memory key-value store where the engine will write stats that later we + // would be able to read to confirm we're collecting stats + kvStore := &kvstore.Memory{} + + initialPolicy := tc.initialPolicy() + t.Logf("initialPolicy: %s", string(initialPolicy)) + if err := kvStore.Set(HTTPSDialerStaticPolicyKey, initialPolicy); err != nil { + t.Fatal(err) + } + + qa.Do(func() { + byteCounter := bytecounter.New() + resolver := netxlite.NewStdlibResolver(log.Log) + + netx := NewNetwork(byteCounter, kvStore, log.Log, nil, resolver) + defer netx.Close() + + client := netx.NewHTTPClient() + + resp, err := client.Get(tc.URL) + + switch { + case err == nil && tc.expectErr == "": + // all good + + case err != nil && tc.expectErr == "": + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + + case err == nil && tc.expectErr != "": + t.Fatal("expected", tc.expectErr, "but got", err) + + case err != nil && tc.expectErr != "": + if tc.expectErr != err.Error() { + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + } + } + + if resp != nil { + defer resp.Body.Close() + } + }) + + // obtain the tactics container for the proper domain + rawStats, err := kvStore.Get(statsKey) + if err != nil { + t.Fatal(err) + } + var rootStats statsContainer + if err := json.Unmarshal(rawStats, &rootStats); err != nil { + t.Fatal(err) + } + tactics, good := rootStats.Domains[tc.statsDomain] + if !good { + t.Fatalf("no such record for `%s`", tc.statsDomain) + } + t.Logf("%+v", tactics) + + // we expect to see a single record + if len(tactics.Tactics) != 1 { + t.Fatal("expected a single tactic") + } + tactic, good := tactics.Tactics[tc.statsTacticsSummary] + if !good { + t.Fatalf("no such record for: %s", tc.statsTacticsSummary) + } + + diffOptions := []cmp.Option{ + cmpopts.IgnoreFields(statsTactic{}, "LastUpdated"), + } + if diff := cmp.Diff(tc.expectStats, tactic, diffOptions...); diff != "" { + t.Fatal(diff) + } + }) + } +} + +func TestLoadStatsContainer(t *testing.T) { + type testcase struct { + // name is the test case name + name string + + // input returns the bytes we should Set into the key-value store + input func() []byte + + // expectedErr is the expected error string or an empty string + expectErr string + + // expectRoot is the expected root container content + expectRoot *statsContainer + } + + cases := []testcase{{ + name: "when the key-value store does not contain any data", + input: func() []byte { + // Note that returning nil causes the code to NOT set anything into the kvstore + return nil + }, + expectErr: "no such key", + expectRoot: nil, + }, { + name: "when we cannot parse the serialized JSON", + input: func() []byte { + return []byte(`{`) + }, + expectErr: "unexpected end of JSON input", + expectRoot: nil, + }, { + name: "with invalid version", + input: func() []byte { + return []byte(`{"Version":1}`) + }, + expectErr: "httpsdialerstats.state: wrong stats container version: expected=2 got=1", + expectRoot: nil, + }, { + name: "on success", + input: func() []byte { + root := &statsContainer{ + Domains: map[string]*statsDomain{ + "api.ooni.io": { + Tactics: map[string]*statsTactic{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 4, + CountTCPConnectError: 1, + CountTLSHandshakeError: 1, + CountTLSVerificationError: 1, + CountSuccess: 1, + HistoTCPConnectError: map[string]int64{ + "connection_refused": 1, + }, + HistoTLSHandshakeError: map[string]int64{ + "generic_timeout_error": 1, + }, + HistoTLSVerificationError: map[string]int64{ + "ssl_invalid_hostname": 1, + }, + LastUpdated: time.Date(2023, 9, 25, 0, 0, 0, 0, time.UTC), + Tactic: &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + }, + }, + Version: statsContainerVersion, + } + return runtimex.Try1(json.Marshal(root)) + }, + expectErr: "", + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{ + "api.ooni.io": { + Tactics: map[string]*statsTactic{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 4, + CountTCPConnectError: 1, + CountTLSHandshakeError: 1, + CountTLSVerificationError: 1, + CountSuccess: 1, + HistoTCPConnectError: map[string]int64{ + "connection_refused": 1, + }, + HistoTLSHandshakeError: map[string]int64{ + "generic_timeout_error": 1, + }, + HistoTLSVerificationError: map[string]int64{ + "ssl_invalid_hostname": 1, + }, + LastUpdated: time.Date(2023, 9, 25, 0, 0, 0, 0, time.UTC), + Tactic: &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + }, + }, + Version: statsContainerVersion, + }, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + kvStore := &kvstore.Memory{} + if input := tc.input(); len(input) > 0 { + if err := kvStore.Set(statsKey, input); err != nil { + t.Fatal(err) + } + } + + root, err := loadStatsContainer(kvStore) + + switch { + case err == nil && tc.expectErr == "": + // all good + + case err != nil && tc.expectErr == "": + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + + case err == nil && tc.expectErr != "": + t.Fatal("expected", tc.expectErr, "but got", err) + + case err != nil && tc.expectErr != "": + if tc.expectErr != err.Error() { + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + } + } + + if diff := cmp.Diff(tc.expectRoot, root); diff != "" { + t.Fatal(diff) + } + }) + } +} + +func TestStatsManagerCallbacks(t *testing.T) { + type testcase struct { + name string + initialRoot *statsContainer + do func(stats *statsManager) + expectWarnf int + expectRoot *statsContainer + } + + cases := []testcase{ + + // When TCP connect fails and the reason is a canceled context + { + name: "OnTCPConnectError with ctx.Error() != nil", + initialRoot: &statsContainer{ + Domains: map[string]*statsDomain{ + "api.ooni.io": { + Tactics: map[string]*statsTactic{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + }, + }, + }, + }, + Version: statsContainerVersion, + }, + do: func(stats *statsManager) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // immediately! + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTCPConnectError(ctx, tactic, err) + }, + expectWarnf: 0, + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{ + "api.ooni.io": { + Tactics: map[string]*statsTactic{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + CountTCPConnectInterrupt: 1, + }, + }, + }, + }, + Version: statsContainerVersion, + }, + }, + + // When TCP connect fails and we don't already have a policy record + { + name: "OnTCPConnectError when we are missing the stats record for the domain", + initialRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + do: func(stats *statsManager) { + ctx := context.Background() + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTCPConnectError(ctx, tactic, err) + }, + expectWarnf: 1, + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + }, + + // When TLS handshake fails and the reason is a canceled context + { + name: "OnTLSHandshakeError with ctx.Error() != nil", + initialRoot: &statsContainer{ + Domains: map[string]*statsDomain{ + "api.ooni.io": { + Tactics: map[string]*statsTactic{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + }, + }, + }, + }, + Version: statsContainerVersion, + }, + do: func(stats *statsManager) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // immediately! + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTLSHandshakeError(ctx, tactic, err) + }, + expectWarnf: 0, + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{ + "api.ooni.io": { + Tactics: map[string]*statsTactic{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + CountTLSHandshakeInterrupt: 1, + }, + }, + }, + }, + Version: statsContainerVersion, + }, + }, + + // When TLS handshake fails and we don't already have a policy record + { + name: "OnTLSHandshakeError when we are missing the stats record for the domain", + initialRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + do: func(stats *statsManager) { + ctx := context.Background() + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTLSHandshakeError(ctx, tactic, err) + }, + expectWarnf: 1, + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + }, + + // When TLS verification fails and we don't already have a policy record + { + name: "OnTLSVerifyError when we are missing the stats record for the domain", + initialRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + do: func(stats *statsManager) { + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTLSVerifyError(tactic, err) + }, + expectWarnf: 1, + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + }, + + // With success when we don't already have a policy record + { + name: "OnSuccess when we are missing the stats record for the domain", + initialRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + do: func(stats *statsManager) { + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + + stats.OnSuccess(tactic) + }, + expectWarnf: 1, + expectRoot: &statsContainer{ + Domains: map[string]*statsDomain{}, + Version: statsContainerVersion, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // configure the initial value of the stats + kvStore := &kvstore.Memory{} + if err := kvStore.Set(statsKey, runtimex.Try1(json.Marshal(tc.initialRoot))); err != nil { + t.Fatal(err) + } + + // create logger counting the number Warnf invocations + var warnfCount int + logger := &mocks.Logger{ + MockWarnf: func(format string, v ...any) { + warnfCount++ + }, + } + + // create the stats manager + stats := newStatsManager(kvStore, logger) + + // invoke the proper stats callback + tc.do(stats) + + // close the stats to trigger a kvstore write + if err := stats.Close(); err != nil { + t.Fatal(err) + } + + // extract the possibly modified stats from the kvstore + var root *statsContainer + rawRoot, err := kvStore.Get(statsKey) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(rawRoot, &root); err != nil { + t.Fatal(err) + } + + // make sure the stats are the ones we expect + diffOptions := []cmp.Option{ + cmpopts.IgnoreFields(statsTactic{}, "LastUpdated"), + } + if diff := cmp.Diff(tc.expectRoot, root, diffOptions...); diff != "" { + t.Fatal(diff) + } + + // make sure we logged if necessary + if tc.expectWarnf != warnfCount { + t.Fatal("expected", tc.expectWarnf, "got", warnfCount) + } + }) + } +}