* chore(netxlite): add currently failing test case
This diff introduces a test cases that will fail because of the reason
explained in https://github.com/ooni/probe/issues/1965.
* chore(netxlite/iox_test.go): add failing unit tests
These tests directly show how the Go implementation of ReadAll
and Copy has the issue of checking for io.EOF equality.
* fix(netxlite): make {ReadAll,Copy}Context robust to wrapped io.EOF
The fix is simple: we just need to check for `errors.Is(err, io.EOF)`
after either io.ReadAll or io.Copy has returned. When this condition is
true, we need to convert the error back to `nil` as it ought to be.
While there, observe that the unit tests I committed in the previous
commit are wrongly asserting that the error must be wrapped. This
assertion is not correct, because in both cases we have just ensured
that the returned error is `nil` (i.e., success).
See https://github.com/ooni/probe/issues/1965.
* cleanup: remove previous workaround for wrapped io.EOF
These workarounds were partial, meaning that they would cover some
cases in which the issue occurred but not all of them.
Handling the problem in `netxlite.{ReadAll,Copy}Context` is the
right thing to do _as long as_ we always use these functions instead
of `io.{ReadAll,Copy}`.
This is why it's now important to ensure we clearly mention that
inside of the `CONTRIBUTING.md` guide and to also ensure that we're
not using these functions in the code base.
* fix(urlgetter): repair tests who assumed to see EOF error
Now that we have established that we should normalize EOF when
reading bodies like the stdlib does and now that it's clear why
our behavior diverged from the stdlib, we also need to repair
all the tests that assumed this incorrect behavior.
* fix(all): don't use io{,util}.{Copy,ReadAll}
* feat: add checks to ensure we don't use io.{Copy,ReadAll}
* doc(netxlite): document we know how to deal w/ wrapped io.EOF
* fix(nocopyreadall.bash): add exception for i/n/iox.go
This diff addresses another point of https://github.com/ooni/probe/issues/1956:
> - [ ] observe that we're still using a bunch of private interfaces for common interfaces such as the `Dialer`, so we can get rid of these private interfaces and always use the ones in `model`, which allows us to remove a bunch of legacy wrappers
Additional cleanups may still be possible. The more I cleanup, the more I see
there's extra legacy code we can dispose of (which seems good?).
This diff implements the first two cleanups defined at https://github.com/ooni/probe/issues/1956:
> - [ ] observe that `netxlite` and `netx` differ in error wrapping only in the way in which we set `ErrWrapper.Operation`. Observe that the code using `netxlite` does not care about such a field. Therefore, we can modify `netxlite` to set such a field using the code of `netx` and we can remove `netx` specific code for errors (which currently lives inside of the `./internal/engine/legacy/errorsx` package
>
> - [ ] after we've done the previous cleanup, we can make all the classifiers code private, since there's no code outside `netxlite` that needs them
A subsequent diff will address the remaining cleanup.
While there, notice that there are failing, unrelated obfs4 tests, so disable them in short mode. (I am confident these tests are unrelated because they fail for me when running test locally from the `master` branch.)
1. we want optionally to log the body (we don't want to log the body
when we're fetching psiphon secrets or tor targets)
2. we want body logging to _also_ happen on error since this is quite
useful to debug possible errors when accessing the API
This diff adds the above functionality, which were previously
described in https://github.com/ooni/probe/issues/1951.
This diff also adds comprehensive testing.
As mentioned in https://github.com/ooni/probe/issues/1951, one of
the main issues I did see with httpx.APIClient is that in some cases
it's used in a very fragile way by probeservices.Client.
This happens in psiphon.go and tor.go, where we create a copy of
the APIClient and then modify it's Authorization field.
If we ever refactor probeservices.Client to take a pointer to
httpx.Client, we are now mutating the httpx.Client.
Of course, we don't want that to happen.
This diff attempts to address such a problem as follows:
1. we create a new APIClientTemplate type that holds the same
fields of an APIClient and allows to build an APIClient
2. we modify every user of APIClient to use APIClientTemplate
3. when we need an APIClient, we build it from the corresponding
template and, when we need to use a specific Authorization, we
use a build factory that sets APIClient.Authorization
4. we hide APIClient by renaming it apiClient and by defining
an interface called APIClient that allows to use it
So, now the codebase always uses the opaque APIClient interface to
issue API calls and always uses the APIClientTemplate to build an
opaque APIClient.
Boom! We have separated construction from usage and we are not
mutating in weird ways the APIClient anymore.
This PR starts to implement the refactoring described at https://github.com/ooni/probe/issues/1951. I originally wrote more patches than the ones in this PR, but overall they were not readable. Since I want to squash and merge, here's a reasonable subset of the original patches that will still be readable and understandable in the future.
## 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
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
We need still to add similar wrappers to internal/netxlite but we
will adopt a saner approach to error wrapping this time.
See https://github.com/ooni/probe/issues/1591
The legacy part for now is internal/errorsx. It will stay there until
I figure out whether it also needs some extra bug fixing.
The good part is now in internal/netxlite/errorsx and contains all the
logic for mapping errors. We need to further improve upon this logic
by writing more thorough integration tests for QUIC.
We also need to copy the various dialer, conn, etc adapters that set
errors. We will put them inside netxlite and we will generate errors in
a way that is less crazy with respect to the major operation. (The
idea is to always wrap, given that now we measure in an incremental way
and we don't measure every operation together.)
Part of https://github.com/ooni/probe/issues/1591
Like before, do not touch the rest of the tree. Rather create
compatibility types declared as legacy.
We will soon be able to close idle connections for an HTTP3
transport using any kind of resolvers more easily.
See https://github.com/ooni/probe/issues/1591
This basically adapts already existing code inside websteps to
instead be into the netxlite package, where it belongs.
In the process, abstract the TLSDialer but keep a reference to the
previous name to avoid refactoring existing code (just for now).
While there, notice that the right name is CloseIdleConnections (i.e.,
plural not singular) and change the name.
While there, since we abstracted TLSDialer to be an interface, create
suitable factories for making a TLSDialer type from a Dialer and a
TLSHandshaker.
See https://github.com/ooni/probe/issues/1591
Like we did before for the resolver, a dialer should propagate the
request to close idle connections to underlying types.
See https://github.com/ooni/probe/issues/1591
We would like to refactor the code so that a DoH resolver owns the
connections of its underlying HTTP client.
To do that, we need first to incorporate CloseIdleConnections
into the Resolver model. Then, we need to add the same function
to all netxlite types that wrap a Resolver type.
At the same time, we want the rest of the code for now to continue
with the simpler definition of a Resolver, now called ResolverLegacy.
We will eventually propagate this change to the rest of the tree
and simplify the way in which we manage Resolvers.
To make this possible, we introduce a new factory function that
adapts a ResolverLegacy to become a Resolver.
See 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
The quic-go library does not support it anymore. So, let us be consistent
and remove any reference to h3-29 from our codebase.
Closes https://github.com/ooni/probe/issues/1740.
This diff enables `websteps` to use uTLS for TLS parroting. It integrates the `oohttp.StdlibTransport` wrapper which uses the `ooni/oohttp` fork. `oohttp` supports TLS-like connections like `utls.Conn`.
As a prototype, the testhelper and `websteps` code now uses the `utls.HelloChrome_Auto` fingerprint, i.e. the simulated TLS fingerprint of the Google Chrome browser.
It is a further contribution for my GSoC project.
Reference issue: https://github.com/ooni/probe/issues/1733
This is the extension of https://github.com/ooni/probe-cli/pull/431, and my final deliverable for GSoC 2021.
The diff introduces:
1) The new `testhelper` which supports testing multiple IP endpoints per domain and introduces HTTP/3 control measurements. The specification of the `testhelper` can be found at https://github.com/ooni/spec/pull/219. The `testhelper` algorithm consists of three main steps:
* `InitialChecks` verifies that the input URL can be parsed, has an expected scheme, and contains a valid domain name.
* `Explore` enumerates all the URLs that it discovers by redirection from the original URL, or by detecting h3 support at the target host.
* `Generate` performs a step-by-step measurement of each discovered URL.
2) A prototype of the corresponding new experiment `websteps` which uses the control measurement of the `testhelper` to know which URLs to measure, and what to expect. The prototype does not yet have:
* unit and integration tests,
* an analysis tool to compare the control and the probe measurement.
This PR is my final deliverable as it is the outcome of the trials, considerations and efforts of my GSoC weeks at OONI.
It fully integrates HTTP/3 (QUIC) support which has been only used in the `urlgetter` experiment until now.
Related issues: https://github.com/ooni/probe/issues/1729 and https://github.com/ooni/probe/issues/1733.
