diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 9933c84..5c30899 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -3,7 +3,8 @@ on: push: branches: - mobile-staging - - 'release/**' + - "release/**" + - "**android**" jobs: test: runs-on: macos-latest @@ -14,7 +15,7 @@ jobs: - uses: actions/checkout@v2 - run: brew install --cask android-sdk - run: echo y | sdkmanager --install "platforms;android-29" - - run: echo y | sdkmanager --install "ndk;21.3.6528147" + - run: echo y | sdkmanager --install "ndk-bundle" - run: ./build-android.bash env: ANDROID_HOME: /usr/local/Caskroom/android-sdk/4333796 diff --git a/build-android.bash b/build-android.bash index a96d759..2b5e9ab 100755 --- a/build-android.bash +++ b/build-android.bash @@ -11,7 +11,7 @@ if [ -z "$ANDROID_HOME" -o "$1" = "--help" ]; then echo "" echo "Then make sure you install the required packages:" echo "" - echo "sdkmanager --install 'build-tools;29.0.3' 'ndk;21.3.6528147'" + echo "sdkmanager --install 'build-tools;29.0.3' 'ndk-bundle'" echo "" echo "or, if you already installed, that you're up to date:" echo "" @@ -22,28 +22,6 @@ if [ -z "$ANDROID_HOME" -o "$1" = "--help" ]; then echo "" exit 1 fi -if [ -d $ANDROID_HOME/ndk-bundle ]; then - echo "" - echo "FATAL: currently we need 'ndk;21.3.6528147' instead of ndk-bundle" - echo "" - echo "See https://github.com/ooni/probe-engine/issues/1179." - echo "" - echo "To fix: sdkmanager --uninstall ndk-bundle" - echo "" - exit 1 -fi -export ANDROID_NDK_HOME=$ANDROID_HOME/ndk/21.3.6528147 -if [ ! -d $ANDROID_NDK_HOME ]; then - echo "" - echo "FATAL: currently we need 'ndk;21.3.6528147'" - echo "" - echo "See https://github.com/ooni/probe-engine/issues/1179." - echo "" - echo "To fix: sdkmanager --install 'ndk;21.3.6528147'" - echo "" - exit 1 -fi - topdir=$(cd $(dirname $0) && pwd -P) set -x export PATH=$(go env GOPATH)/bin:$PATH diff --git a/cmd/ooniprobe/internal/nettests/stunreachability.go b/cmd/ooniprobe/internal/nettests/stunreachability.go index 68fb40e..14aa46c 100644 --- a/cmd/ooniprobe/internal/nettests/stunreachability.go +++ b/cmd/ooniprobe/internal/nettests/stunreachability.go @@ -5,7 +5,7 @@ type STUNReachability struct{} // Run starts the nettest. func (n STUNReachability) Run(ctl *Controller) error { - builder, err := ctl.Session.NewExperimentBuilder("stun_reachability") + builder, err := ctl.Session.NewExperimentBuilder("stunreachability") if err != nil { return err } diff --git a/internal/engine/allexperiments.go b/internal/engine/allexperiments.go index c937add..5b6ebfb 100644 --- a/internal/engine/allexperiments.go +++ b/internal/engine/allexperiments.go @@ -241,7 +241,7 @@ var experimentsByName = map[string]func(*Session) *ExperimentBuilder{ } }, - "stun_reachability": func(session *Session) *ExperimentBuilder { + "stunreachability": func(session *Session) *ExperimentBuilder { return &ExperimentBuilder{ build: func(config interface{}) *Experiment { return NewExperiment(session, stunreachability.NewExperimentMeasurer( diff --git a/internal/engine/experiment/stunreachability/stunreachability.go b/internal/engine/experiment/stunreachability/stunreachability.go index 8bd2845..4d5ba5b 100644 --- a/internal/engine/experiment/stunreachability/stunreachability.go +++ b/internal/engine/experiment/stunreachability/stunreachability.go @@ -19,8 +19,8 @@ import ( ) const ( - testName = "stun_reachability" - testVersion = "0.1.0" + testName = "stunreachability" + testVersion = "0.2.0" ) // Config contains the experiment config. @@ -122,15 +122,15 @@ func (tk *TestKeys) do( if err != nil { return err } - defer conn.Close() newClient := stun.NewClient if config.newClient != nil { newClient = config.newClient } - client, err := newClient(conn, stun.WithNoConnClose) + client, err := newClient(conn) if err != nil { return err } + defer client.Close() message := stun.MustBuild(stun.TransactionID, stun.BindingRequest) ch := make(chan error) err = client.Start(message, func(ev stun.Event) { diff --git a/internal/engine/experiment/stunreachability/stunreachability_test.go b/internal/engine/experiment/stunreachability/stunreachability_test.go index 3f9ffc5..777bb4f 100644 --- a/internal/engine/experiment/stunreachability/stunreachability_test.go +++ b/internal/engine/experiment/stunreachability/stunreachability_test.go @@ -18,10 +18,10 @@ import ( func TestMeasurerExperimentNameVersion(t *testing.T) { measurer := stunreachability.NewExperimentMeasurer(stunreachability.Config{}) - if measurer.ExperimentName() != "stun_reachability" { + if measurer.ExperimentName() != "stunreachability" { t.Fatal("unexpected ExperimentName") } - if measurer.ExperimentVersion() != "0.1.0" { + if measurer.ExperimentVersion() != "0.2.0" { t.Fatal("unexpected ExperimentVersion") } } diff --git a/internal/engine/geolocate/geolocate.go b/internal/engine/geolocate/geolocate.go index d1371ae..15378cb 100644 --- a/internal/engine/geolocate/geolocate.go +++ b/internal/engine/geolocate/geolocate.go @@ -5,9 +5,9 @@ import ( "context" "errors" "fmt" - "net/http" "github.com/ooni/probe-cli/v3/internal/engine/model" + "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/runtimex" "github.com/ooni/probe-cli/v3/internal/version" ) @@ -51,7 +51,12 @@ var ( // Logger is the definition of Logger used by this package. type Logger interface { + Debug(msg string) Debugf(format string, v ...interface{}) + Info(msg string) + Infof(format string, v ...interface{}) + Warn(msg string) + Warnf(format string, v ...interface{}) } // Results contains geolocate results @@ -115,15 +120,23 @@ type ResourcesManager interface { MaybeUpdateResources(ctx context.Context) error } +// Resolver is a DNS resolver. +type Resolver interface { + LookupHost(ctx context.Context, domain string) ([]string, error) + Network() string + Address() string +} + // Config contains configuration for a geolocate Task. type Config struct { // EnableResolverLookup indicates whether we want to // perform the optional resolver lookup. EnableResolverLookup bool - // HTTPClient is the HTTP client to use. If not set, then - // we will use the http.DefaultClient. - HTTPClient *http.Client + // Resolver is the resolver we should use when + // making requests for discovering the IP. When + // this field is not set, we use the stdlib. + Resolver Resolver // Logger is the logger to use. If not set, then we will // use a logger that discards all messages. @@ -146,9 +159,6 @@ func Must(task *Task, err error) *Task { // NewTask creates a new instance of Task from config. func NewTask(config Config) (*Task, error) { - if config.HTTPClient == nil { - config.HTTPClient = http.DefaultClient - } if config.Logger == nil { config.Logger = model.DiscardLogger } @@ -158,13 +168,17 @@ func NewTask(config Config) (*Task, error) { if config.UserAgent == "" { config.UserAgent = fmt.Sprintf("ooniprobe-engine/%s", version.Version) } + if config.Resolver == nil { + config.Resolver = netx.NewResolver( + netx.Config{Logger: config.Logger}) + } return &Task{ countryLookupper: mmdbLookupper{}, enableResolverLookup: config.EnableResolverLookup, probeIPLookupper: ipLookupClient{ - HTTPClient: config.HTTPClient, - Logger: config.Logger, - UserAgent: config.UserAgent, + Resolver: config.Resolver, + Logger: config.Logger, + UserAgent: config.UserAgent, }, probeASNLookupper: mmdbLookupper{}, resolverASNLookupper: mmdbLookupper{}, diff --git a/internal/engine/geolocate/geolocate_test.go b/internal/engine/geolocate/geolocate_test.go index 99091ee..cabf2e2 100644 --- a/internal/engine/geolocate/geolocate_test.go +++ b/internal/engine/geolocate/geolocate_test.go @@ -393,3 +393,10 @@ func TestNewTaskWithNoResourcesManager(t *testing.T) { t.Fatal("expected nil task here") } } + +func TestASNStringWorks(t *testing.T) { + r := Results{ASN: 1234} + if r.ASNString() != "AS1234" { + t.Fatal("unexpected result") + } +} diff --git a/internal/engine/geolocate/iplookup.go b/internal/engine/geolocate/iplookup.go index 7bb6a92..acf0222 100644 --- a/internal/engine/geolocate/iplookup.go +++ b/internal/engine/geolocate/iplookup.go @@ -11,6 +11,7 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/internal/multierror" + "github.com/ooni/probe-cli/v3/internal/engine/netx" ) var ( @@ -65,8 +66,8 @@ var ( ) type ipLookupClient struct { - // HTTPClient is the HTTP client to use - HTTPClient *http.Client + // Resolver is the resolver to use for HTTP. + Resolver Resolver // Logger is the logger to use Logger Logger @@ -88,7 +89,15 @@ func makeSlice() []method { func (c ipLookupClient) doWithCustomFunc( ctx context.Context, fn lookupFunc, ) (string, error) { - ip, err := fn(ctx, c.HTTPClient, c.Logger, c.UserAgent) + // Implementation note: we MUST use an HTTP client that we're + // sure IS NOT using any proxy. To this end, we construct a + // client ourself that we know is not proxied. + clnt := &http.Client{Transport: netx.NewHTTPTransport(netx.Config{ + Logger: c.Logger, + FullResolver: c.Resolver, + })} + defer clnt.CloseIdleConnections() + ip, err := fn(ctx, clnt, c.Logger, c.UserAgent) if err != nil { return DefaultProbeIP, err } @@ -102,7 +111,7 @@ func (c ipLookupClient) doWithCustomFunc( func (c ipLookupClient) LookupProbeIP(ctx context.Context) (string, error) { union := multierror.New(ErrAllIPLookuppersFailed) for _, method := range makeSlice() { - c.Logger.Debugf("iplookup: using %s", method.name) + c.Logger.Infof("iplookup: using %s", method.name) ip, err := c.doWithCustomFunc(ctx, method.fn) if err == nil { return ip, nil diff --git a/internal/engine/geolocate/iplookup_test.go b/internal/engine/geolocate/iplookup_test.go index 6d53a9c..b26ba9e 100644 --- a/internal/engine/geolocate/iplookup_test.go +++ b/internal/engine/geolocate/iplookup_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "net" - "net/http" "testing" "github.com/apex/log" @@ -12,9 +11,8 @@ import ( func TestIPLookupGood(t *testing.T) { ip, err := (ipLookupClient{ - HTTPClient: http.DefaultClient, - Logger: log.Log, - UserAgent: "ooniprobe-engine/0.1.0", + Logger: log.Log, + UserAgent: "ooniprobe-engine/0.1.0", }).LookupProbeIP(context.Background()) if err != nil { t.Fatal(err) @@ -28,9 +26,8 @@ func TestIPLookupAllFailed(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // immediately cancel to cause Do() to fail ip, err := (ipLookupClient{ - HTTPClient: http.DefaultClient, - Logger: log.Log, - UserAgent: "ooniprobe-engine/0.1.0", + Logger: log.Log, + UserAgent: "ooniprobe-engine/0.1.0", }).LookupProbeIP(ctx) if !errors.Is(err, context.Canceled) { t.Fatal("expected an error here") @@ -43,9 +40,8 @@ func TestIPLookupAllFailed(t *testing.T) { func TestIPLookupInvalidIP(t *testing.T) { ctx := context.Background() ip, err := (ipLookupClient{ - HTTPClient: http.DefaultClient, - Logger: log.Log, - UserAgent: "ooniprobe-engine/0.1.0", + Logger: log.Log, + UserAgent: "ooniprobe-engine/0.1.0", }).doWithCustomFunc(ctx, invalidIPLookup) if !errors.Is(err, ErrInvalidIPAddress) { t.Fatal("expected an error here") diff --git a/internal/engine/geolocate/stun.go b/internal/engine/geolocate/stun.go index 34e9abe..90332db 100644 --- a/internal/engine/geolocate/stun.go +++ b/internal/engine/geolocate/stun.go @@ -7,6 +7,11 @@ import ( "github.com/pion/stun" ) +// TODO(bassosimone): we should modify the stun code to use +// the session resolver rather than using its own. +// +// See https://github.com/ooni/probe/issues/1383. + type stunClient interface { Close() error Start(m *stun.Message, h stun.Handler) error diff --git a/internal/engine/internal/sessionresolver/resolvermaker.go b/internal/engine/internal/sessionresolver/resolvermaker.go index 95e30f6..1719c6e 100644 --- a/internal/engine/internal/sessionresolver/resolvermaker.go +++ b/internal/engine/internal/sessionresolver/resolvermaker.go @@ -88,6 +88,7 @@ func (r *Resolver) newresolver(URL string) (childResolver, error) { ByteCounter: r.byteCounter(), HTTP3Enabled: h3, Logger: r.logger(), + ProxyURL: r.ProxyURL, }, URL) } diff --git a/internal/engine/internal/sessionresolver/sessionresolver.go b/internal/engine/internal/sessionresolver/sessionresolver.go index 8613a21..9b9af01 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver.go +++ b/internal/engine/internal/sessionresolver/sessionresolver.go @@ -25,6 +25,7 @@ import ( "errors" "fmt" "math/rand" + "net/url" "sync" "time" @@ -39,6 +40,7 @@ type Resolver struct { ByteCounter *bytecounter.Counter // optional KVStore KVStore // optional Logger Logger // optional + ProxyURL *url.URL // optional codec codec dnsClientMaker dnsclientmaker mu sync.Mutex @@ -71,6 +73,9 @@ func (r *Resolver) LookupHost(ctx context.Context, hostname string) ([]string, e defer r.writestate(state) me := multierror.New(ErrLookupHost) for _, e := range state { + if r.shouldSkipWithProxy(e) { + continue // we cannot proxy this URL so ignore it + } addrs, err := r.lookupHost(ctx, e, hostname) if err == nil { return addrs, nil @@ -80,6 +85,19 @@ func (r *Resolver) LookupHost(ctx context.Context, hostname string) ([]string, e return nil, me } +func (r *Resolver) shouldSkipWithProxy(e *resolverinfo) bool { + URL, err := url.Parse(e.URL) + if err != nil { + return true // please skip + } + switch URL.Scheme { + case "https", "dot", "tcp": + return false // we can handle this + default: + return true // please skip + } +} + func (r *Resolver) lookupHost(ctx context.Context, ri *resolverinfo, hostname string) ([]string, error) { const ewma = 0.9 // the last sample is very important re, err := r.getresolver(ri.URL) diff --git a/internal/engine/internal/sessionresolver/sessionresolver_test.go b/internal/engine/internal/sessionresolver/sessionresolver_test.go index b83cde8..b2b28d3 100644 --- a/internal/engine/internal/sessionresolver/sessionresolver_test.go +++ b/internal/engine/internal/sessionresolver/sessionresolver_test.go @@ -4,7 +4,9 @@ import ( "context" "errors" "net" + "net/url" "strings" + "sync/atomic" "testing" "github.com/google/go-cmp/cmp" @@ -247,3 +249,92 @@ func TestMaybeConfusionManyEntries(t *testing.T) { t.Fatal("unexpected state[3].URL") } } + +func TestResolverWorksWithProxy(t *testing.T) { + var ( + works int32 + startuperr = make(chan error) + listench = make(chan net.Listener) + done = make(chan interface{}) + ) + // proxy implementation + go func() { + defer close(done) + lconn, err := net.Listen("tcp", "127.0.0.1:0") + startuperr <- err + if err != nil { + return + } + listench <- lconn + for { + conn, err := lconn.Accept() + if err != nil { + // We assume this is when we were told to + // shutdown by the main goroutine. + return + } + atomic.AddInt32(&works, 1) + conn.Close() + } + }() + // make sure we could start the proxy + if err := <-startuperr; err != nil { + t.Fatal(err) + } + listener := <-listench + // use the proxy + reso := &Resolver{ProxyURL: &url.URL{ + Scheme: "socks5", + Host: listener.Addr().String(), + }} + ctx := context.Background() + addrs, err := reso.LookupHost(ctx, "ooni.org") + // cleanly shutdown the listener + listener.Close() + <-done + // check results + if !errors.Is(err, ErrLookupHost) { + t.Fatal("not the error we expected") + } + if addrs != nil { + t.Fatal("expected nil addrs") + } + if works < 1 { + t.Fatal("expected to see a positive number of entries here") + } +} + +func TestShouldSkipWithProxyWorks(t *testing.T) { + expect := []struct { + url string + result bool + }{{ + url: "\t", + result: true, + }, { + url: "https://dns.google/dns-query", + result: false, + }, { + url: "dot://dns.google/", + result: false, + }, { + url: "http3://dns.google/dns-query", + result: true, + }, { + url: "tcp://dns.google/", + result: false, + }, { + url: "udp://dns.google/", + result: true, + }, { + url: "system:///", + result: true, + }} + reso := &Resolver{} + for _, e := range expect { + out := reso.shouldSkipWithProxy(&resolverinfo{URL: e.url}) + if out != e.result { + t.Fatal("unexpected result for", e) + } + } +} diff --git a/internal/engine/session.go b/internal/engine/session.go index f6c8e69..e80d99d 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -108,14 +108,15 @@ func NewSession(config SessionConfig) (*Session, error) { ByteCounter: sess.byteCounter, BogonIsError: true, Logger: sess.logger, + ProxyURL: config.ProxyURL, } sess.resolver = &sessionresolver.Resolver{ ByteCounter: sess.byteCounter, KVStore: config.KVStore, Logger: sess.logger, + ProxyURL: config.ProxyURL, } httpConfig.FullResolver = sess.resolver - httpConfig.ProxyURL = config.ProxyURL // no need to proxy the resolver sess.httpDefaultTransport = netx.NewHTTPTransport(httpConfig) return sess, nil } @@ -490,8 +491,8 @@ func (s *Session) LookupLocationContext(ctx context.Context) (*geolocate.Results // when we are using a proxy because that might leak information. task := geolocate.Must(geolocate.NewTask(geolocate.Config{ EnableResolverLookup: s.proxyURL == nil, - HTTPClient: s.DefaultHTTPClient(), Logger: s.Logger(), + Resolver: s.resolver, ResourcesManager: s, UserAgent: s.UserAgent(), })) diff --git a/internal/libminiooni/libminiooni.go b/internal/libminiooni/libminiooni.go index 3360215..df6c540 100644 --- a/internal/libminiooni/libminiooni.go +++ b/internal/libminiooni/libminiooni.go @@ -43,6 +43,7 @@ type Options struct { HomeDir string Inputs []string InputFilePaths []string + Limit int64 NoJSON bool NoCollector bool ProbeServicesURL string @@ -54,6 +55,7 @@ type Options struct { TorBinary string Tunnel string Verbose bool + Version bool Yes bool } @@ -87,6 +89,10 @@ func init() { &globalOptions.Inputs, "input", 'i', "Add test-dependent input to the test input", "INPUT", ) + getopt.FlagLong( + &globalOptions.Limit, "limit", 0, + "Limit the number of URLs tested by Web Connectivity", "N", + ) getopt.FlagLong( &globalOptions.NoJSON, "no-json", 'N', "Disable writing to disk", ) @@ -126,6 +132,9 @@ func init() { getopt.FlagLong( &globalOptions.Verbose, "verbose", 'v', "Increase verbosity", ) + getopt.FlagLong( + &globalOptions.Version, "version", 0, "Print version and exit", + ) getopt.FlagLong( &globalOptions.Yes, "yes", 0, "I accept the risk of running OONI", ) @@ -149,6 +158,10 @@ func fatalIfFalse(cond bool, msg string) { // integrate this function to either handle the panic of ignore it. func Main() { getopt.Parse() + if globalOptions.Version { + fmt.Printf("%s\n", version.Version) + os.Exit(0) + } fatalIfFalse(len(getopt.Args()) == 1, "Missing experiment name") MainWithConfiguration(getopt.Arg(0), globalOptions) } @@ -362,7 +375,7 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { SourceFiles: currentOptions.InputFilePaths, InputPolicy: builder.InputPolicy(), Session: sess, - URLLimit: 17, + URLLimit: currentOptions.Limit, }) inputs, err := inputLoader.Load(context.Background()) fatalOnError(err, "cannot load inputs") diff --git a/internal/version/version.go b/internal/version/version.go index 9f7674a..0b5eeb3 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -3,5 +3,5 @@ package version const ( // Version is the software version - Version = "3.7.0" + Version = "3.8.0" ) diff --git a/pkg/oonimkall/session.go b/pkg/oonimkall/session.go index ad47fc8..3a8f624 100644 --- a/pkg/oonimkall/session.go +++ b/pkg/oonimkall/session.go @@ -4,11 +4,10 @@ import ( "context" "encoding/json" "errors" - "fmt" "runtime" "sync" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/engine/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" @@ -21,7 +20,7 @@ type AtomicInt64 struct { *atomicx.Int64 } -// The following two variables contain metrics pertaining to the number +// These two variables contain metrics pertaining to the number // of Sessions and Contexts that are currently being used. var ( ActiveSessions = &AtomicInt64{atomicx.NewInt64()} @@ -33,11 +32,22 @@ var ( // to this instance in the SessionConfig object. All log messages that // the Session will generate will be routed to this Logger. type Logger interface { + // Debug handles debug messages. Debug(msg string) + + // Info handles informational messages. Info(msg string) + + // Warn handles warning messages. Warn(msg string) } +// ExperimentCallbacks contains experiment callbacks. +type ExperimentCallbacks interface { + // OnProgress provides information about an experiment progress. + OnProgress(percentage float64, message string) +} + // SessionConfig contains configuration for a Session. You should // fill all the mandatory fields and could also optionally fill some of // the optional fields. Then pass this struct to NewSession. @@ -83,24 +93,18 @@ type SessionConfig struct { // OONI related task (e.g. geolocation). Note that the Session isn't // mean to be a long living object. The workflow is to create a Session, // do the operations you need to do with it now, then make sure it is -// not referenced by other variables, so the Go GC can finalize it. -// -// Future directions -// -// We will eventually rewrite the code for running new experiments such -// that a Task will be created from a Session, such that experiments -// could share the same Session and save geolookups, etc. For now, we -// are in the suboptimal situations where Tasks create, use, and close -// their own session, thus running more lookups than needed. +// not referenced by other variables, so the Go GC can finalize it. This +// is what you would normally done with Java/ObjC. type Session struct { + // Hooks for testing (should not appear in Java/ObjC, because they + // cannot be automatically transformed to Java/ObjC code.) + TestingCheckInBeforeNewProbeServicesClient func(ctx *Context) + TestingCheckInBeforeCheckIn func(ctx *Context) + cl []context.CancelFunc mtx sync.Mutex submitter *probeservices.Submitter sessp *engine.Session - - // Hooks for testing (should not appear in Java/ObjC) - TestingCheckInBeforeNewProbeServicesClient func(ctx *Context) - TestingCheckInBeforeCheckIn func(ctx *Context) } // NewSession creates a new session. You should use a session for running @@ -133,6 +137,8 @@ func NewSession(config *SessionConfig) (*Session, error) { return nil, err } sess := &Session{sessp: sessp} + // We use finalizers to reduce the burden of managing the + // session from languages with a garbage collector. runtime.SetFinalizer(sess, sessionFinalizer) ActiveSessions.Add(1) return sess, nil @@ -159,7 +165,9 @@ type Context struct { ctx context.Context } -// Cancel cancels pending operations using this context. +// Cancel cancels pending operations using this context. This method +// is idempotent. Calling it more than once is fine. The first invocation +// cancels the context. Subsequent invocations are no-operations. func (ctx *Context) Cancel() { ctx.cancel() } @@ -171,7 +179,8 @@ func (sess *Session) NewContext() *Context { // NewContextWithTimeout creates an new interruptible Context that will automatically // cancel itself after the given timeout. Setting a zero or negative timeout implies -// there is no actual timeout configured for the Context. +// there is no actual timeout configured for the Context, making this invocation +// equivalent to calling NewContext(). func (sess *Session) NewContextWithTimeout(timeout int64) *Context { sess.mtx.Lock() defer sess.mtx.Unlock() @@ -188,7 +197,7 @@ func (sess *Session) NewContextWithTimeout(timeout int64) *Context { return &Context{cancel: cancel, ctx: ctx} } -// GeolocateResults contains the GeolocateTask results. +// GeolocateResults contains the results of session.Geolocate. type GeolocateResults struct { // ASN is the autonomous system number. ASN string @@ -203,15 +212,21 @@ type GeolocateResults struct { Org string } -// MaybeUpdateResources ensures that resources are up to date. +// MaybeUpdateResources ensures that resources are up to date. This function +// could perform network activity when we need to update resources. +// +// This function locks the session until it's done. That is, no other operation +// can be performed as long as this function is pending. func (sess *Session) MaybeUpdateResources(ctx *Context) error { sess.mtx.Lock() defer sess.mtx.Unlock() return sess.sessp.MaybeUpdateResources(ctx.ctx) } -// Geolocate performs a geolocate operation and returns the results. This method -// is (in Java terminology) synchronized with the session instance. +// Geolocate performs a geolocate operation and returns the results. +// +// This function locks the session until it's done. That is, no other operation +// can be performed as long as this function is pending. func (sess *Session) Geolocate(ctx *Context) (*GeolocateResults, error) { sess.mtx.Lock() defer sess.mtx.Unlock() @@ -220,7 +235,7 @@ func (sess *Session) Geolocate(ctx *Context) (*GeolocateResults, error) { return nil, err } return &GeolocateResults{ - ASN: fmt.Sprintf("AS%d", info.ASN), + ASN: info.ASNString(), Country: info.CountryCode, IP: info.ProbeIP, Org: info.NetworkName, @@ -230,12 +245,17 @@ func (sess *Session) Geolocate(ctx *Context) (*GeolocateResults, error) { // SubmitMeasurementResults contains the results of a single measurement submission // to the OONI backends using the OONI collector API. type SubmitMeasurementResults struct { + // UpdateMeasurement is the measurement with updated report ID. UpdatedMeasurement string - UpdatedReportID string + + // UpdatedReportID is the report ID used for the measurement. + UpdatedReportID string } -// Submit submits the given measurement and returns the results. This method is (in -// Java terminology) synchronized with the Session instance. +// Submit submits the given measurement and returns the results. +// +// This function locks the session until it's done. That is, no other operation +// can be performed as long as this function is pending. func (sess *Session) Submit(ctx *Context, measurement string) (*SubmitMeasurementResults, error) { sess.mtx.Lock() defer sess.mtx.Unlock() @@ -261,12 +281,15 @@ func (sess *Session) Submit(ctx *Context, measurement string) (*SubmitMeasuremen }, nil } -// CheckInConfigWebConnectivity is the configuration for the WebConnectivity test +// CheckInConfigWebConnectivity contains WebConnectivity +// configuration for the check-in API. type CheckInConfigWebConnectivity struct { - CategoryCodes []string // CategoryCodes is an array of category codes + // CategoryCodes contains zero or more category codes (e.g. "HUMR"). + CategoryCodes []string } -// Add a category code to the array in CheckInConfigWebConnectivity +// Add adds a category code to ckw.CategoryCode. This method allows you to +// edit ckw.CategoryCodes, which is inaccessible from Java/ObjC. func (ckw *CheckInConfigWebConnectivity) Add(cat string) { ckw.CategoryCodes = append(ckw.CategoryCodes, cat) } @@ -277,36 +300,62 @@ func (ckw *CheckInConfigWebConnectivity) toModel() model.CheckInConfigWebConnect } } -// CheckInConfig contains configuration for calling the checkin API. +// CheckInConfig contains configuration for the check-in API. type CheckInConfig struct { - Charging bool // Charging indicate if the phone is actually charging - OnWiFi bool // OnWiFi indicate if the phone is actually connected to a WiFi network - Platform string // Platform of the probe - RunType string // RunType - SoftwareName string // SoftwareName of the probe - SoftwareVersion string // SoftwareVersion of the probe - WebConnectivity *CheckInConfigWebConnectivity // WebConnectivity class contain an array of categories + // Charging indicates whether the phone is charging. + Charging bool + + // OnWiFi indicates whether the phone is using the Wi-Fi. + OnWiFi bool + + // Platform is the mobile platform (e.g. "android") + Platform string + + // RunType indicates whether this is an automated ("timed") run + // or otherwise a manual run initiated by the user. + RunType string + + // SoftwareName is the name of the application. + SoftwareName string + + // SoftwareVersion is the version of the application. + SoftwareVersion string + + // WebConnectivity contains configuration items specific of + // the WebConnectivity experiment. + WebConnectivity *CheckInConfigWebConnectivity } -// CheckInInfoWebConnectivity contains the array of URLs returned by the checkin API +// CheckInInfoWebConnectivity contains the WebConnectivity +// specific results of the check-in API call. type CheckInInfoWebConnectivity struct { + // ReportID is the report ID we should be using. ReportID string - URLs []model.URLInfo + + // URLs contains the list of URLs to measure. + URLs []model.URLInfo } -// URLInfo contains info on a test lists URL +// URLInfo contains info on a specific URL to measure. type URLInfo struct { + // CategoryCode is the URL's category code (e.g. "HUMR"). CategoryCode string - CountryCode string - URL string + + // CountryCode is the test list from which this URL + // comes from (e.g. "IT", "FR"). + CountryCode string + + // URL is the URL itself. + URL string } -// Size returns the number of URLs. +// Size returns the number of URLs included into the result. func (ckw *CheckInInfoWebConnectivity) Size() int64 { return int64(len(ckw.URLs)) } -// At gets the URLInfo at position idx from CheckInInfoWebConnectivity.URLs +// At returns the URLInfo at index idx. Note that this function will +// return nil/null if the index is out of bounds. func (ckw *CheckInInfoWebConnectivity) At(idx int64) *URLInfo { if idx < 0 || int(idx) >= len(ckw.URLs) { return nil @@ -323,20 +372,27 @@ func newCheckInInfoWebConnectivity(ckw *model.CheckInInfoWebConnectivity) *Check if ckw == nil { return nil } - out := new(CheckInInfoWebConnectivity) - out.ReportID = ckw.ReportID - out.URLs = ckw.URLs - return out + return &CheckInInfoWebConnectivity{ + ReportID: ckw.ReportID, + URLs: ckw.URLs, + } } -// CheckInInfo contains the return test objects from the checkin API +// CheckInInfo contains the result of the check-in API. type CheckInInfo struct { + // WebConnectivity contains results that are specific to + // the WebConnectivity experiment. This field MAY be null + // if the server's response did not contain any info. WebConnectivity *CheckInInfoWebConnectivity } -// CheckIn function is called by probes asking if there are tests to be run -// The config argument contains the mandatory settings. -// Returns the list of tests to run and the URLs, on success, or an explanatory error, in case of failure. +// CheckIn calls the check-in API. Both ctx and config MUST NOT be nil. This +// function will fail if config is missing required settings. The return value +// is either an error or a valid CheckInInfo instance. Beware that the returned +// object MAY still contain nil fields depending on the server's response. +// +// This function locks the session until it's done. That is, no other operation +// can be performed as long as this function is pending. func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo, error) { sess.mtx.Lock() defer sess.mtx.Unlock() @@ -348,14 +404,14 @@ func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo, return nil, err } if sess.TestingCheckInBeforeNewProbeServicesClient != nil { - sess.TestingCheckInBeforeNewProbeServicesClient(ctx) + sess.TestingCheckInBeforeNewProbeServicesClient(ctx) // for testing } psc, err := sess.sessp.NewProbeServicesClient(ctx.ctx) if err != nil { return nil, err } if sess.TestingCheckInBeforeCheckIn != nil { - sess.TestingCheckInBeforeCheckIn(ctx) + sess.TestingCheckInBeforeCheckIn(ctx) // for testing } cfg := model.CheckInConfig{ Charging: config.Charging,