Set of changes to unblock ooni/probe-cli#46 #37
Conversation
This is currently speculative, since MK v0.10.5 has not been released yet. Nonetheless, this functionality should be working when we have relased that version.
Psiphon is not using `go.mod`. Updating also psiphon should be done with extra care precisely because of that. For other dependencies, we can go YOLO.
bassosimone
force-pushed
the
feature/probe-cli-46
branch
from
96ffea7
to
56ac91d
Compare
bassosimone
added
enhancement
bassosimone
added this to In progress
in DO NOT USE (was: OONI Nettests and Engine)
via automation
|
At this point the build is failing because |
bassosimone
changed the title
|
@hellais I believe I have addressed your comments by refactoring the code. PTAL? |
That is half correct. We look it up but we don't pass it to MK. It seems actually the right thing to do is to pass that information to MK such that it case use it, as we do for the probe IP. |
Ok. Does probe-engine write the resolver_ip inside of the |
No, it does not. The main issue is that the |
|
@bassosimone wrote:
TBH, I'm not sure anymore of what could be the best approach here. The |
|
Yes it's fine we can do this as part of another PR. |
Okay, issue here #38 |
|
So, today I'll proceed with breaking fixing the build, breaking this down in commits and merging. |
|
Regarding the Coveralls error, it turns out yesterday it was down for maintenance. |
This diff introduces code to redact IP, ASN, and CC directly in the engine, rather than delegating doing that. When you initialise a Session, it's your responsibility to change the default privacy settings. The names of the collector.PrivacySettings structure fields are the same names used by ooni/probe-cli. After a measurement, the settings configured in the session are honoured with respect to redacting IP, ASN, and CC. We take an extra step of redacting the whole test keys string, _if_ it contains the IP. This is an extra safety net to make sure that we are really scrubbing the body. The nettest should do that. This commit is part of #28.
1. make sure we use a separate structure for the open report response such that the server cannot write our data structure 2. make sure symbols that can be private are actually private 3. make sure the code is compliant with the spec by checking that the server actually supports `json` reports
bassosimone
force-pushed
the
feature/probe-cli-46
branch
from
2550506
to
861ff03
Compare
|
I restructured the PR in a set of understandable commits and fixed a bug that prevented tests without input to run when doing an additional review. I am now going to merge as soon as Travis is green. |
Rationale
As identified in ooni/probe-cli#46 (comment), the remaining actions before merging ooni/probe-cli#46 are:
This PR implements enough functionality in the engine to satisfy the above requirements. We do not implement everything that is described in #28 but we implement enough of it such that the two above criteria are satisfied. This is needed to, among other things, unblock #52.
Scope
Each commit in this PR could be considered independently, if the overall diff looks too messy. Also, I have added more details to each commit message. The whole diff also has comments.