commit aa8314973df0f6e458aa1e0de786faaad8264dc3 Author: Arturo Filastò arturo@filasto.net Date: Fri Apr 15 15:45:03 2016 +0200

[refactoring] Change signature of Deck to be more robust

Do more sanity checks on bouncer address at instantiation time --- ooni/deck.py | 30 ++++++++++++++---------------- ooni/oonibclient.py | 15 +++++++++++++-- ooni/oonicli.py | 4 ++-- ooni/tests/mocks.py | 3 +++ ooni/tests/test_deck.py | 14 +++++++------- ooni/tests/test_templates.py | 3 +++ 6 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/ooni/deck.py b/ooni/deck.py index b13385d..4b2c502 100644 --- a/ooni/deck.py +++ b/ooni/deck.py @@ -95,28 +95,27 @@ def nettest_to_path(path, allow_arbitrary_paths=False):

class Deck(InputFile): + # this exists so we can mock it out in unittests + _OONIBClient = OONIBClient + def __init__(self, deck_hash=None, - deckFile=None, + bouncer=None, decks_directory=config.decks_directory, no_collector=False): self.id = deck_hash - self.requiresTor = False self.no_collector = no_collector - self.bouncer = '' + self.bouncer = bouncer + + self.requiresTor = False + self.netTestLoaders = [] self.inputs = []

- self.oonibclient = OONIBClient(self.bouncer) - self.decksDirectory = os.path.abspath(decks_directory) - self.deckHash = deck_hash - - if deckFile: - self.loadDeck(deckFile)

@property def cached_file(self): - return os.path.join(self.decksDirectory, self.deckHash) + return os.path.join(self.decksDirectory, self.id)

@property def cached_descriptor(self): @@ -124,7 +123,7 @@ class Deck(InputFile):

def loadDeck(self, deckFile): with open(deckFile) as f: - self.deckHash = sha256(f.read()).hexdigest() + self.id = sha256(f.read()).hexdigest() f.seek(0) test_deck = yaml.safe_load(f)

@@ -175,7 +174,7 @@ class Deck(InputFile):

@defer.inlineCallbacks def lookupCollectorAndTestHelpers(self): - self.oonibclient.address = self.bouncer + oonibclient = self._OONIBClient(self.bouncer)

required_nettests = []

@@ -201,8 +200,7 @@ class Deck(InputFile): if not requires_test_helpers and not requires_collector: defer.returnValue(None)

- log.debug("Looking up {}".format(required_nettests)) - response = yield self.oonibclient.lookupTestCollector(required_nettests) + response = yield oonibclient.lookupTestCollector(required_nettests) provided_net_tests = response['net-tests']

def find_collector_and_test_helpers(test_name, test_version, input_files): @@ -240,10 +238,10 @@ class Deck(InputFile): for i in net_test_loader.inputFiles: if i['url']: log.debug("Downloading %s" % i['url']) - self.oonibclient.address = i['address'] + oonibclient = self._OONIBClient(i['address'])

try: - input_file = yield self.oonibclient.downloadInput(i['hash']) + input_file = yield oonibclient.downloadInput(i['hash']) except: raise e.UnableToLoadDeckInput

diff --git a/ooni/oonibclient.py b/ooni/oonibclient.py index 388bb68..336ae4e 100644 --- a/ooni/oonibclient.py +++ b/ooni/oonibclient.py @@ -1,5 +1,7 @@ import json

+from urlparse import urljoin + from twisted.web.client import Agent from twisted.internet import defer, reactor from twisted.internet.endpoints import TCP4ClientEndpoint @@ -15,6 +17,15 @@ class OONIBClient(object): retries = 3

def __init__(self, address): + if address.startswith("https://"): + log.err("HTTPS bouncers are currently not supported!") + raise e.InvalidOONIBBouncerAddress + elif address.startswith("http://"): + log.msg("Warning using plaintext bouncer!") + elif address.startswith("httpo://"): + log.debug("Using Tor hidden service bouncer: {}".format(address)) + else: + raise e.InvalidOONIBBouncerAddress self.address = address

def _request(self, method, urn, genReceiver, bodyProducer=None): @@ -30,7 +41,7 @@ class OONIBClient(object): raise e.InvalidOONIBBouncerAddress

elif self.address.startswith('http://'): - log.msg("Warning using unencrypted backend") + log.msg("Warning using unencrypted bouncer") agent = Agent(reactor)

attempts = 0 @@ -38,7 +49,7 @@ class OONIBClient(object): finished = defer.Deferred()

def perform_request(attempts): - uri = address + urn + uri = urljoin(address, urn) d = agent.request(method, uri, bodyProducer=bodyProducer)

@d.addCallback diff --git a/ooni/oonicli.py b/ooni/oonicli.py index e2d78e1..281ff46 100644 --- a/ooni/oonicli.py +++ b/ooni/oonicli.py @@ -245,8 +245,8 @@ def createDeck(global_options, url=None): if global_options['no-yamloo']: log.msg("Will not write to a yamloo report file")

- deck = Deck(no_collector=global_options['no-collector']) - deck.bouncer = global_options['bouncer'] + deck = Deck(bouncer=global_options['bouncer'], + no_collector=global_options['no-collector'])

try: if global_options['testdeck']: diff --git a/ooni/tests/mocks.py b/ooni/tests/mocks.py index 4f51f32..19b4692 100644 --- a/ooni/tests/mocks.py +++ b/ooni/tests/mocks.py @@ -190,6 +190,9 @@ class MockTaskManager(TaskManager):

class MockOONIBClient(object): + def __init__(self, *args, **kw): + pass + def lookupTestHelpers(self, required_test_helpers): ret = { 'default': { diff --git a/ooni/tests/test_deck.py b/ooni/tests/test_deck.py index 2c3bc04..3e2a322 100644 --- a/ooni/tests/test_deck.py +++ b/ooni/tests/test_deck.py @@ -127,14 +127,14 @@ class TestDeck(BaseTestCase): os.remove(self.filename)

def test_open_deck(self): - deck = Deck(decks_directory=".") - deck.bouncer = "httpo://foo.onion" + deck = Deck(bouncer="httpo://foo.onion", + decks_directory=".") deck.loadDeck(self.deck_file) assert len(deck.netTestLoaders) == 1

def test_save_deck_descriptor(self): - deck = Deck(decks_directory=".") - deck.bouncer = "httpo://foo.onion" + deck = Deck(bouncer="httpo://foo.onion", + decks_directory=".") deck.loadDeck(self.deck_file) deck.load({'name': 'spam', 'id': 'spam', @@ -149,9 +149,9 @@ class TestDeck(BaseTestCase):

@defer.inlineCallbacks def test_lookup_test_helpers_and_collector(self): - deck = Deck(decks_directory=".") - deck.bouncer = "httpo://foo.onion" - deck.oonibclient = MockOONIBClient() + deck = Deck(bouncer="httpo://foo.onion", + decks_directory=".") + deck._OONIBClient = MockOONIBClient deck.loadDeck(self.deck_file)

self.assertEqual(len(deck.netTestLoaders[0].missingTestHelpers), 1) diff --git a/ooni/tests/test_templates.py b/ooni/tests/test_templates.py index e7452df..5b2c77a 100644 --- a/ooni/tests/test_templates.py +++ b/ooni/tests/test_templates.py @@ -1,6 +1,7 @@ from ooni.templates import httpt, dnst

from ooni.tests import is_internet_connected + from twisted.names import dns from twisted.internet.error import DNSLookupError from twisted.internet import reactor, defer, base @@ -92,6 +93,8 @@ class TestDNST(unittest.TestCase):

@defer.inlineCallbacks def test_perform_a_lookup(self): + if not is_internet_connected(): + self.skipTest("You must be connected to the internet to run this test") dns_test = dnst.DNSTest() dns_test._setUp() result = yield dns_test.performALookup('example.com', dns_server=('8.8.8.8', 53))