Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#7664 closed defect (fixed)

ooniprobe decks do not appear to work

Reported by: ioerror Owned by: hellais
Priority: Medium Milestone:
Component: Archived/Ooni Version:
Severity: Keywords:
Cc: isis, hellias, aagbsn Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by hellais)

I've been trying to run various decks - only to have just a few yamloo files - generally in the form of foo.yamloo and foo.yamloo.old - even the before_i_commit.deck is outputting just http_host.yamloo.yamloo and http_host.yamloo.yamloo.old - the latter has dns tests in it. Something is totally wacked.

Child Tickets

Change History (8)

comment:1 Changed 8 years ago by hellais

Can you try with this latest commit: https://gitweb.torproject.org/ooni-probe.git/commit/b1e818aa18f997c174215e39e73fe53c2538cdc1

Also it is ideal to have ooniprobe set the filename for your reports. So you should set in your deck reportfile: null.

comment:2 Changed 7 years ago by ioerror

I've tried it against the tip of master with the following deck:

decks/burma.testdesk               
# - options:
#     collector: null
#     help: 0
#     logfile: null
#     pcapfile: null
#     reportfile: null
#     subargs: []
#     test: nettests/manipulation/captiveportal.py
- options:
    collector: null
    help: 0
    logfile: null
    pcapfile: null
    reportfile: null
    subargs: [-T, inputs/burma-yangon-hotel-dns-resolvers.txt, -f, 'inputs/list0-dnsnames.txt']
    test: nettests/blocking/dnstamper.py
- options:
    collector: null
    help: 0
    logfile: 
    pcapfile: null
    reportfile: null
    subargs: [-T, inputs/burma-yangon-hotel-dns-resolvers.txt, -f, inputs/list1-dns-list.txt]
    test: nettests/blocking/dnstamper.py
- options:
    collector: null
    help: 0
    logfile: null
    pcapfile: null
    reportfile: null
    subargs: [-T, inputs/burma-yangon-hotel-dns-resolvers.txt, -f, inputs/top-1000.txt]
    test: nettests/blocking/dnstamper.py
- options:
    collector: null
    help: 0
    logfile: null
    pcapfile: null
    reportfile: null 
    subargs: [-b, 'http://ooni.nu/test', -f, 'inputs/list0.txt']
    test: nettests/manipulation/http_host.py
- options:
    collector: null
    help: 0
    logfile: null
    pcapfile: null
    reportfile: null
    subargs: [-b, 'http://ooni.nu/test', -f, 'inputs/list1.txt']
    test: nettests/manipulation/http_host.py
- options:
    collector: null
    help: 1
    logfile: null
    pcapfile: null
    reportfile: http_host_global_list.yamloo
    subargs: [-b, 'http://ooni.nu/test', -f, 'inputs/top-1000.txt']
    test: nettests/manipulation/http_host.py
- options:
    collector: null
    help: 0
    logfile: null
    pcapfile: null
    reportfile: null
    subargs: [-b, 'http://46.166.147.154']
    test: nettests/manipulation/http_header_field_manipulation.py
- options:
    collector: null
    help: 0
    logfile: null
    pcapfile: null
    reportfile: null
    subargs: [-b, 'http://46.166.147.154']
    test: nettests/manipulation/http_invalid_request_line.py
- options:
    collector: null
    help: 0
    logfile: 
    pcapfile: null
    reportfile: null
    subargs: []
    test: nettests/third_party/netalyzr.py

It creates the following files:

report_dnstamper_2012-12-08T193908Z.yamloo
report_dnstamper_2012-12-08T193909Z.yamloo
report_dnstamper_2012-12-08T193909Z.yamloo.1
report_http_header_field_manipulation_2012-12-08T193909Z.yamloo
report_http_host_2012-12-08T193909Z.yamloo
report_http_host_2012-12-08T193909Z.yamloo.1
report_http_invalid_request_line_2012-12-08T193909Z.yamloo
report_netalyzr_2012-12-08T193909Z.yamloo

They aren't very large files:

-rw-r--r-- 1 io io 563 2012-12-09 02:09 http_host_global_list.yamloo
-rw-r--r-- 1 io io 594 2012-12-09 02:09 report_dnstamper_2012-12-08T193908Z.yamloo
-rw-r--r-- 1 io io 559 2012-12-09 02:09 report_dnstamper_2012-12-08T193909Z.yamloo
-rw-r--r-- 1 io io 586 2012-12-09 02:09 report_dnstamper_2012-12-08T193909Z.yamloo.1
-rw-r--r-- 1 io io 579 2012-12-09 02:09 report_http_header_field_manipulation_2012-12-08T193909Z.yamloo
-rw-r--r-- 1 io io 561 2012-12-09 02:09 report_http_host_2012-12-08T193909Z.yamloo
-rw-r--r-- 1 io io 553 2012-12-09 02:09 report_http_host_2012-12-08T193909Z.yamloo.1
-rw-r--r-- 1 io io 564 2012-12-09 02:09 report_http_invalid_request_line_2012-12-08T193909Z.yamloo
-rw-r--r-- 1 io io 626 2012-12-09 02:09 report_netalyzr_2012-12-08T193909Z.yamloo

Note that each one of them essentially contains only a header:

 % cat report_dnstamper_2012-12-08T193908Z.yamloo
###########################################
# OONI Probe Report for DNS tamper test
# Sun Dec  9 02:09:08 2012
###########################################
---
options:
  collector: null
  help: 0
  logfile: null
  pcapfile: null
  reportfile: null
  resume: false
  subargs: [-T, inputs/burma-yangon-hotel-dns-resolvers.txt, -f, inputs/list0-dnsnames.txt]
  test: nettests/blocking/dnstamper.py
probe_asn: AS0
probe_cc: null
probe_ip: 127.0.0.1
software_name: ooniprobe
software_version: 0.0.8
start_time: 1354972148.0
test_name: DNS tamper
test_version: '0.4'
...

The console has the correct output and oonprobe.log has a copy of the console output. The reports are however essentially empty.

comment:3 Changed 7 years ago by hellais

Status: newneeds_revision

The problem is with the netalyzr test.

You are calling in it reactor.stop, that will kill the event loop therefore stopping all the other tests that will never have the time to write to the config file.

This commit fixes it: https://gitweb.torproject.org/ooni-probe.git/commitdiff/fb9d25b80d761532c41034fffc255244b3175fab.

The starting and stopping of the twisted reactor *must* happen only once and *never* happen inside of tests.

comment:4 Changed 7 years ago by hellais

Also I see you are running this towards ooni.nu/test. That has been migrated to ooni.torproject.org/test/ though I believe that it does not work as expected.

To achieve what you are trying to do I would suggest running the http_requests test.

comment:5 Changed 7 years ago by ioerror

Ah - that was unclear - I thought that was the suggest way! I guess it isn't for threaded Twisted plugins... :(

I think we should overload that stop method in plugins such that we can't have this issue again. Really, I think we should have helper classes that ensure that people aren't calling Twisted directly but rather methods/functions that take these little details into account.

I'll give it a try and see how it goes.

comment:6 Changed 7 years ago by ioerror

Ok - it appears that the git tip of master works for decks again. I would say that this bug is likely to come back again - it will be some other thing that shouldn't be called. So how about we turn this bug into a bug where we document those things or implement helper classes, so people do not even call potentially dangerous methods or functions?

comment:7 in reply to:  5 Changed 7 years ago by hellais

Resolution: fixed
Status: needs_revisionclosed

Replying to ioerror:

Ah - that was unclear - I thought that was the suggest way! I guess it isn't for threaded Twisted plugins... :(

I think we should overload that stop method in plugins such that we can't have this issue again. Really, I think we should have helper classes that ensure that people aren't calling Twisted directly but rather methods/functions that take these little details into account.

I'll give it a try and see how it goes.

Well, you should always be using a test template, though there not a test template (yet) for command line scripts.

I opened a ticket on it here: https://trac.torproject.org/projects/tor/ticket/7685

Overriding the twisted reactor.stop method is not a clean solution as the reactor.stop and reactor.start are the only two functions you should not be calling.

Writing tests from scratch should only be done if you know what you are doing, for everything else there is a test template that exposes a simplified API.

comment:8 in reply to:  6 Changed 7 years ago by hellais

Description: modified (diff)

Replying to ioerror:

Ok - it appears that the git tip of master works for decks again. I would say that this bug is likely to come back again - it will be some other thing that shouldn't be called. So how about we turn this bug into a bug where we document those things or implement helper classes, so people do not even call potentially dangerous methods or functions?

The *only* two function that should not be called are reactor.stop and reactor.start, and I though it was quite obvious that this would have been the case. Since apparently it is not so obvious I added it to the tutorial on writing tests: https://gitweb.torproject.org/ooni-probe.git/commit/53ed7f69ff809e20eeaaaa4e3797e1da155de6a2

Note: See TracTickets for help on using tickets.