This implements the bare minimum needed to have circumvention proxy support in OONI Probe CLI.
The reference issue is https://github.com/ooni/probe/issues/1955. (Most of that issue is implemented, save for the fact that currently we do not have support for `http` and `https` proxies.)
While there, add to `Makefile` a rule for correctly building `ooniprobe` and `miniooni` for "this system" (i.e., the default `GOOS` and `GOARCH` on a system), because we needed this for testing this patch and we needed to figure out the commands instead.
This diff modifies the engine package to make Experiment and
ExperimentBuilder interfaces rather than structs.
The previosuly existing structs are now named experiment{,Builder}.
This diff helps https://github.com/ooni/probe/issues/2184
because it allows us to write unit tests more easily.
There should be no functional change.
While there, I removed a bunch of deprecated functions, which were
unnecessarily complicate the implementation and could be easily
replaced by passing them a context.Context or context.Background().
The objective is to make PR checks run much faster.
See https://github.com/ooni/probe/issues/2113 for context.
Regarding netxlite's tests:
Checking for every commit on master or on a release branch is
good enough and makes pull requests faster than one minute since
netxlite for windows is now 1m slower than coverage.
We're losing some coverage but coverage from integration tests
is not so good anyway, so I'm not super sad about this loss.
This diff fixes the way in which we print JSON results inside
`ooniprobe show <ID>` by using the "%s" fmt specifier rather than
using the JSON string itself as the format string.
See https://github.com/ooni/probe/issues/2082
FWIW, `ooniprobe show --batch <ID>` was already WAI.
* Upgrade to github.com/upper/db/v4
* fix(oonitest): repair imports after merge
Oops, okay, it seems the merge did not preserve all the import
changes, so let's ensure we use the right imports here!
* cleanup(go.mod): don't refer to upper.io/db/v3
These lines didn't disappear previously because the merge commit
failed to remove all references to upper.io/db/v3.
Co-authored-by: stergem <sgemelas@protonmail.com>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
This diff changes the software name used by unattended runs for which
we did not override the default software name (`ooniprobe-cli`).
It will become `ooniprobe-cli-unattended`. This software name is in line
with the one we use for Android, iOS, and desktop unattended runs.
While working in this diff, I introduced string constants for the run
types and a string constant for the default software name.
See https://github.com/ooni/probe/issues/2081.
This commit forward ports 59c63ee0b2249c803c40f1eb19c0f6c062838bf1,
whose original log message follows:
- - -
While doing QA in https://github.com/ooni/probe/issues/1845, I
noticed we're not using the correct directory.
Results are written in the current directory inside of the OONI_HOME,
which is quite not what we want to happen.
This diff WILL require forward porting to master.
Reference issue: https://github.com/ooni/probe/issues/1917.
I needed to change the summary key type returned by `torsf` to be a value. It seems the DB layer assumes that. If we pass it a pointer, it panics because it's experiment a value rather than a pointer 🤷.
* Refactor the list measurements function to make use of nested queries
With a dataset of 1489 test, the ooniprobe list command went from
taking 17.27s to run, to requiring 0.17s or a 100x speed boost
See https://github.com/ooni/probe/issues/1966
* Remove dead code from actions
* Improve the tests for the ListResults function
* Add test for AnomalyCount
* Add more documentation about the merging of the test_keys
## Checklist
- [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request: https://github.com/ooni/probe/issues/1885
- [x] related ooni/spec pull request: N/A
Location of the issue tracker: https://github.com/ooni/probe
## Description
This PR contains a set of changes to move important interfaces and data types into the `./internal/model` package.
The criteria for including an interface or data type in here is roughly that the type should be important and used by several packages. We are especially interested to move more interfaces here to increase modularity.
An additional side effect is that, by reading this package, one should be able to understand more quickly how different parts of the codebase interact with each other.
This is what I want to move in `internal/model`:
- [x] most important interfaces from `internal/netxlite`
- [x] everything that was previously part of `internal/engine/model`
- [x] mocks from `internal/netxlite/mocks` should also be moved in here as a subpackage
This diff is part of https://github.com/ooni/probe/issues/1814 and
teaches `ooniprobe` to run dnscheck and stunreachability by using the
default static input feature of the `InputLoader`.
I've manually tested that we can still run `websites` like
we did before (including category filtering).
I've also manually tested that now we can run `experimental` and
get parseable results for dnscheck and stunreachability.
With this diff in, we have fixed the original problem highlighted in
the https://github.com/ooni/probe/issues/1814 issue.
Yet, because of the way in which I solved the problem, there is
more work to do. My changes have broken stunreachability for
mobile and now it's time I apply fixes to make it work again.
This diff was extracted from https://github.com/ooni/probe-cli/pull/539,
which at this point only basically contains the remaining fixes to
ensure we can run stunreachability on mobile.
Co-authored-by: Arturo Filastò <arturo@filasto.net>
Co-authored-by: Arturo Filastò <arturo@filasto.net>
See https://github.com/ooni/probe/issues/1816
This diff addresses the most immediate issue but there is probably
extra work to do, including testing and making sure experiments
do not return an error when they should not.
When preparing a tutorial for netxlite, I figured it is easier
to tell people "hey, this is the package you should use for all
low-level networking stuff" rather than introducing people to
a set of packages working together where some piece of functionality
is here and some other piece is there.
Part of https://github.com/ooni/probe/issues/1591
## Description
This PR continues the refactoring of `netx` under the following principles:
1. do not break the rest of the tree and do not engage in extensive tree-wide refactoring yet
2. move under `netxlite` clearly related subpackages (e.g., `iox`, `netxmocks`)
3. move into `internal/netxlite/internal` stuff that is clearly private of `netxlite`
4. hide implementation details in `netxlite` pending new factories
5. refactor `tls` code in `netxlite` to clearly separate `crypto/tls` code from `utls` code
After each commit, I run `go test -short -race ./...` locally. Each individual commit explains what it does. I will squash, but this operation will preserve the original commit titles, so this will give further insight on each step.
## Commits
* refactor: rename netxmocks -> netxlite/mocks
Part of https://github.com/ooni/probe/issues/1591
* refactor: rename quicx -> netxlite/quicx
See https://github.com/ooni/probe/issues/1591
* refactor: rename iox -> netxlite/iox
Regenerate sources and make sure the tests pass.
See https://github.com/ooni/probe/issues/1591.
* refactor(iox): move MockableReader to netxlite/mocks
See https://github.com/ooni/probe/issues/1591
* refactor(netxlite): generator is an implementation detail
See https://github.com/ooni/probe/issues/1591
* refactor(netxlite): separate tls and utls code
See https://github.com/ooni/probe/issues/1591
* refactor(netxlite): hide most types but keep old names as legacy
With this change we avoid breaking the rest of the tree, but we start
hiding some implementation details a bit. Factories will follow.
See https://github.com/ooni/probe/issues/1591
* fix(all): introduce and use iox.CopyContext
This PR is part of https://github.com/ooni/probe/issues/1417.
In https://github.com/ooni/probe-cli/pull/379 we introduced a context
aware wrapper for io.ReadAll (formerly ioutil.ReadAll).
Here we introduce a context aware wrapper for io.Copy.
* fix(humanize): more significant digits
* fix: rename humanize files to follow the common pattern
* fix aligment
* fix test
* fix(all): introduce and use iox.ReadAllContext
This improvement over the ioutil.ReadAll utility returns early
if the context expires. This enables us to unblock stuck code in
case there's censorship confounding the TCP stack.
See https://github.com/ooni/probe/issues/1417.
Compared to the functionality postulated in the above mentioned
issue, I choose to be more generic and separate limiting the
maximum body size (not implemented here) from using the context
to return early when reading a body (or any other reader).
After implementing iox.ReadAllContext, I made sure we always
use it everywhere in the tree instead of ioutil.ReadAll.
This includes many parts of the codebase where in theory we don't
need iox.ReadAllContext. Though, changing all the places makes
checking whether we're not using ioutil.ReadAll where we should
not be using it easy: `git grep` should return no lines.
* Update internal/iox/iox_test.go
* fix(ndt7): treat context errors as non-errors
The rationale is explained by the comment documenting reduceErr.
* Update internal/engine/experiment/ndt7/download.go
* refactor(atomicx): move outside the engine package
After merging probe-engine into probe-cli, my impression is that we have
too much unnecessary nesting of packages in this repository.
The idea of this commit and of a bunch of following commits will instead
be to reduce the nesting and simplify the structure.
While there, improve the documentation.
* fix: always use the atomicx package
For consistency, never use sync/atomic and always use ./internal/atomicx
so we can just grep and make sure we're not risking to crash if we make
a subtle mistake on a 32 bit platform.
While there, mention in the contributing guidelines that we want to
always prefer the ./internal/atomicx package over sync/atomic.
* fix(atomicx): remove unnecessary constructor
We don't need a constructor here. The default constructed `&Int64{}`
instance is already usable and the constructor does not add anything to
what we are doing, rather it just creates extra confusion.
* cleanup(atomicx): we are not using Float64
Because atomicx.Float64 is unused, we can safely zap it.
* cleanup(atomicx): simplify impl and improve tests
We can simplify the implementation by using defer and by letting
the Load() method call Add(0).
We can improve tests by making many goroutines updated the
atomic int64 value concurrently.
* refactor(fsx): can live in the ./internal pkg
Let us reduce the amount of nesting. While there, ensure that the
package only exports the bare minimum, and improve the documentation
of the tests, to ease reading the code.
* refactor: move runtimex to ./internal
* refactor: move shellx into the ./internal package
While there, remove unnecessary dependency between packages.
While there, specify in the contributing guidelines that
one should use x/sys/execabs instead of os/exec.
* refactor: move ooapi into the ./internal pkg
* refactor(humanize): move to ./internal and better docs
* refactor: move platform to ./internal
* refactor(randx): move to ./internal
* refactor(multierror): move into the ./internal pkg
* refactor(kvstore): all kvstores in ./internal
Rather than having part of the kvstore inside ./internal/engine/kvstore
and part in ./internal/engine/kvstore.go, let us put every piece of code
that is kvstore related into the ./internal/kvstore package.
* fix(kvstore): always return ErrNoSuchKey on Get() error
It should help to use the kvstore everywhere removing all the
copies that are lingering around the tree.
* sessionresolver: make KVStore mandatory
Simplifies implementation. While there, use the ./internal/kvstore
package rather than having our private implementation.
* fix(ooapi): use the ./internal/kvstore package
* fix(platform): better documentation
* Add support for tracking the is_uploaded status in the results table
* Add unit tests for measurement upload status
This implements: https://github.com/ooni/probe/issues/1457
* Update cmd/ooniprobe/internal/database/actions.go
Co-authored-by: Simone Basso <bassosimone@gmail.com>
When this happens, the user is expressing the intention of
explicitly testing all the input they provided.
So, disable maxRuntime in these cases.
Part of https://github.com/ooni/probe/issues/1436.
This fixes an issue where URLs provided with --input are not
accepted by the preventMistakes filter.
The filter itself needs to execute _only_ on URLs returned
by the checkIn API, rather than on URLs returned by the
InputLoader, which may instead be user provided.
Reference issue: https://github.com/ooni/probe/issues/1435
* feat: create tunnel inside NewSession
We want to create the tunnel when we create the session. This change
allows us to nicely ignore the problem of creating a tunnel when we
already have a proxy, as well as the problem of locking. Everything is
happening, in fact, inside of the NewSession factory.
Modify miniooni such that --tunnel is just syntactic sugar for
--proxy, at least for now. We want, in the future, to teach the
tunnel to possibly use a socks5 proxy.
Because starting a tunnel is a slow operation, we need a context in
NewSession. This causes a bunch of places to change. Not really a big
deal except we need to propagate the changes.
Make sure that the mobile code can create a new session using a
proxy for all the APIs we support.
Make sure all tests are still green and we don't loose coverage of
the various ways in which this code could be used.
This change is part of https://github.com/ooni/probe/issues/985.
* changes after merge
* fix: only keep tests that can hopefully work
While there, identify other places where we should add more
tests or fix integration tests.
Part of https://github.com/ooni/probe/issues/985
* feat(tunnel): introduce persistent tunnel state dir
This diff introduces a persistent state directory for tunnels, so that
we can bootstrap them more quickly after the first time.
Part of https://github.com/ooni/probe/issues/985
* fix: make tunnel dir optional
We have many tests where it does not make sense to explicitly
provide a tunnel dir because we're not using tunnels.
This should simplify setting up a session.
* fix(tunnel): repair tests
* final changes
* more cleanups
* fix(pkg.go.dev): import a subpackage containing the assets
We're trying to fix this issue that pkg.go.dev does not build.
Thanks to @hellais for this very neat idea! Let's keep our
fingers crossed and see whether it fixes!
* feat: use embedded geoip databases
Closes https://github.com/ooni/probe/issues/1372.
Work done as part of https://github.com/ooni/probe/issues/1369.
* fix(assetsx): add tests
* feat: simplify and just vendor uncompressed DBs
* remove tests that seems not necessary anymore
* fix: run go mod tidy
* Address https://github.com/ooni/probe-cli/pull/260/files#r605181364
* rewrite a test in a better way
* fix: gently cleanup the legacy assetsdir
Do not remove the whole directory with brute force. Just zap the
files whose name we know. Then attempt to delete the legacy directory
as well. If not empty, just fail. This is fine because it means the
user has stored other files inside the directory.
* fix: create .miniooni if missing
* feat(ooniprobe): discard lists not in selected categories
One day we may make an integration mistake and for any reason
we may end up with URLs that do not belong to the categories
originally selected by the user. If that happens, it's nice to
have a safety net where we remove URLs that do not belong to
the right category before proceeding with testing.
This diff was conceived while discussing the robustness of
https://github.com/ooni/probe/issues/1299 with @hellais.
* fix behavior and add unit test
* more robust
We cannot control anymore the maximum number of URLs using the API
because now we are using check-in, that has no such limit.
We could theoretically clamp the number of URLs to measure after
the call to check-in, and still honour the setting.
Yet, the right thing to do seems to introduce a max runtime variable
because that is what desktop and mobile do.
Thus, introduce code that warns the user about the change in the
settings, should they have set the URL limit to nonzero.
We are going to do a best effort conversion from the URL limit to
the maximum runtime for the rest of 2021.
Since then, we will silently ignore the URL limit.
This work is part of https://github.com/ooni/probe/issues/1299.
* ongoing work
* reduce diff with master
* feat(inputloader): use the check-in API
Part of https://github.com/ooni/probe/issues/1299
* fix: better naming for a variable
* chore: add more tests
* fix: add one more TODO
* Add signal to the im test group
* fix(ipconfig_test.go): disable when running in CI
Reference issue: https://github.com/ooni/probe/issues/1418
* fix(geolocate): remove unused variable
Came across this while looking into this issue with the CI that
is now failing. Guess fixing it here comes across as leaving the
camp slightly less in a bad shape than how I found it.
Co-authored-by: Simone Basso <bassosimone@gmail.com>
