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.
Reducing the errors is not done in a perfect way.
We have documented the most striking differences inside
https://github.com/ooni/probe/issues/1707#issuecomment-942283746 and
some attempts to improve the situation further inside
https://github.com/ooni/probe/issues/1707#issuecomment-942341255.
A better strategy for the future would be to introduce more
specific timeout errors, such as dns_timeout_error, etc.
More testing may be needed to further validate and compare the
old and the new TH, but this requires Jafar improvements to
more precisely simulate more complex censorship.
This is the most immediate fix to the issue described by
https://github.com/ooni/probe/issues/1792.
So, the logic was actually miss the increment, which
would have been noticed with proper unit testing.
Anyway, I am not sure why the loop ensues in the first
time. By looking at the headers, it seems we're passing
the headers correctly.
So, even though this fix interrupts the loop, it still
remains the question of whether the loop is legit or
whether we're missing extra logic to properly redirect.
This diff adds the prototype websteps implementation that used
to live at https://github.com/ooni/probe-cli/pull/506.
The code is reasonably good already and it's pointing to a roaming
test helper that I've properly configured.
You can run websteps with:
```
./miniooni -n websteps
```
This will go over the test list for your country.
At this stage the mechanics of the experiment is set, but we
still need to have a conversation on the following topics:
1. whether we're okay with reusing the data format used by other
OONI experiments, or we would like to use a more compact data
format (which may either be a more compact JSON or we can choose
to always submit compressed measurements for websteps);
2. the extent to which we would like to keep the measurement as
a collection of "the experiment saw this" and "the test helper
saw that" and let the pipeline choose an overall score: this is
clearly an option, but there is also the opposite option to
build a summary of the measurement on the probe.
Compared to the previous prototype of websteps, the main
architectural change we have here is that we are following
the point of view of the probe and the test helper is
much more dumb. Basically, the probe will choose which
redirection to follow and ask the test helper every time
it discovers a new URL to measure it w/o redirections.
Reference issue: https://github.com/ooni/probe/issues/1733
This commit introduce a measurement library that consists of
refactored code from earlier websteps experiments.
I am not going to add tests for the time being, because this library
is still a bit in flux, as we finalize websteps.
I will soon though commit documentation explaining in detail how
to use it, which currrently is at https://github.com/ooni/probe-cli/pull/506
and adds a new directory to internal/tutorial.
The core idea of this measurement library is to allow two
measurement modes:
1. tracing, which is what we're currently doing now, and the
tutorial shows how we can rewrite the measurement part of web
connectivity with measurex using less code. Under a tracing
approach, we construct a normal http.Client that however has
tracing configured, we gather events for resolve, connect, TLS
handshake, QUIC handshake, HTTP round trip, etc. and then we
try to make sense of what happened from the events stream;
2. step-by-step, which is what websteps does, and basically
means that after each operation you immediately write into
a Measurement structure its results and immediately draw the
conclusions on what seems odd (which later may become an
anomaly if we see what the test helper measured).
This library is also such that it produces a data format
compatible with the current OONI spec.
This work is part of https://github.com/ooni/probe/issues/1733.
This is required to implement websteps, which is currently tracked
by https://github.com/ooni/probe/issues/1733.
We introduce the concept of async runner. An async runner will
post measurements on a channel until it is done. When it is done,
it will close the channel to notify the reader about that.
This change causes sync experiments now to strictly return either
a non-nil measurement or a non-nil error.
While this is a pretty much obvious situation in golang, we had
some parts of the codebase that were not robust to this assumption
and attempted to submit a measurement after the measure call
returned an error.
Luckily, we had enough tests to catch this change in our assumption
and this is why there are extra docs and tests changes.
At the moment ooapi is not used. It will eventually be used since
it's a better way of accessing the OONI backend API.
To fix these tests, we need to fix the swagger emitted by the
backend API, which is not a priority at the moment, since we are
working instead to integrate websteps in miniooni.
Issue https://github.com/ooni/probe/issues/1790 tracks the work
required to re-enabled the tests I'm skipping with this diff.
This work is part of https://github.com/ooni/probe/issues/1733.
* feat: run ~always netxlite integration tests
This diff ensures that we check on windows, linux, macos that our
fundamental networking library (netxlite) works.
We combine unit and integration tests.
This work is part of https://github.com/ooni/probe/issues/1733, where
I want to have more strong guarantees about the foundations.
* fix(filtering/tls_test.go): make portable on Windows
The trick here is to use the wrapped error so to normalize the
different errors messages we see on Windows.
* fix(netxlite/quic_test.go): make portable on windows
Rather than using the zero port, use the `x` port which fails
when the stdlib is parsing the address.
The zero port seems to work on Windows while it does not on Unix.
* fix(serialresolver_test.go): make error more timeout than before
This seems enough to convince Go on Windows about this error
being really a timeout timeouty timeouted thingie.
On Windows, GetAddrInfoW is a syscall and the Go resolver does
not attempt to map errors beyond WSA_HOST_NOT_FOUND, which becomes
"no such host", which we map to "dns_nxdomain_error".
See https://github.com/golang/go/blob/go1.17.1/src/net/lookup_windows.go#L16.
To map more GetAddrInfoW errors, thus, we need to enhance our
error classifier to have system specific errors.
Then, we need to filter for the WSA errors that are most likely
to pop up and map them to OONI failures. Those are three:
- WSANO_DATA which we have from our own UDP resolver as well
and which we can map to `dns_no_answer`
- WSANO_RECOVERY which we don't have but existed for MK so
we will use `dns_non_recoverable_failure`, which was an MK error
- WSATRY_AGAIN which likewise we map to the error that MK
used to emit, so `dns_temporary_failure`
This diff should address https://github.com/ooni/probe/issues/1467.
I need to run test on Windows and I just discovered that:
1. the `errno_unix.go` filename does not mean anything because
`unix` is not a valid platform, so we need a filename for
each platform that we care about;
2. on Windows we need to use WSA prefixed names;
3. `i/e/session_psiphon.go` was not building because of the
migration from `netxlite/iox` to `netxlite`.
This diff attempts to fix all three issues.
The reference issue is https://github.com/ooni/probe/issues/1733,
because I was working on such an issue.
* feat(oonimkall): instrument code to understand CI issue
It seems ~difficult to reproduce the problem locally and I could not
see it after five runs of
```
go test -race -count 1 ./pkg/oonimkall/...
```
So, here's some diagnostic code that could help understanding the
reference issue https://github.com/ooni/probe/issues/1785.
Also, it seems the issue pops up much more frequently when running
CI anyway. So, I am going to leave this diff around and when it
appears again I have more context to fix the issue.
* fix(oonimkall): skip flaky test in short mode
See https://github.com/ooni/probe/issues/1785
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
I have recently seen a data race related our way of
mutating the outgoing request to set the host header.
Unfortunately, I've lost track of the race output,
because I rebooted my Linux box before saving it.
Though, after inspecting why and and where we're mutating
outgoing requets, I've found that:
1. we add the host header when logging to have it logged,
which is not a big deal since we already emit the URL
rather than just the URL path when logging a request, and
so we can safely zap this piece of code;
2. as a result, in measurements we may omit the host header
but again this is pretty much obvious from the URL itself
and so it should not be very important (nonetheless,
avoid surprises and keep the existing behavior);
3. when the User-Agent header is not set, we default to
a `miniooni/0.1.0-dev` user agent, which is probably not
very useful anyway, so we can actually remove it.
Part of https://github.com/ooni/probe/issues/1733 (this diff
has been extracted from https://github.com/ooni/probe-cli/pull/506).
While there, modernize the way in which we run tests to avoid
depending on the fake files scattered around the tree and to
use some well defined mock structures instead.
Part of https://github.com/ooni/probe/issues/1591
There are a bunch of packages where we don't really need to depend
on netx but we can use local definitions that describe what we are
expecting from data structures we receive in input. This diff
addresses one of such cases.
Part of https://github.com/ooni/probe/issues/1591
I discovered which transport were used by apitool and made sure he gets the same transports now. While there, I discovered an issue with ooni/oohttp that has been fixed with cba9b1ce5e.
Part of https://github.com/ooni/probe/issues/1591
* netxlite: improve docs, tests, and code quality
* better documentation
* more strict testing of dialer (especially make sure we
document the quirk in https://github.com/ooni/probe/issues/1779
and we have tests to guarantee we don't screw up here)
* introduce NewErrWrapper factory for creating errors so we
have confidence we are creating them correctly
Part of https://github.com/ooni/probe/issues/1591
