[tor-commits] [ooni-probe/master] Start adding support for HTTPS, Cloudfronted test helpers and collectors

art at torproject.org art at torproject.org
Mon May 30 16:28:33 UTC 2016


commit d17873211da4bd6ec3c0d449ea1d62c2216d1996
Author: Arturo Filastò <arturo at filasto.net>
Date:   Wed May 11 15:11:13 2016 +0200

    Start adding support for HTTPS, Cloudfronted test helpers and collectors
    
    * Add routines to verify which collectors and test helpers are reachable
    
    * Merciless refactoring of the reporting logic
    
    * Remove dumb logic
---
 ooni/backend_client.py                     | 364 +++++++++++++++++++++++++++++
 ooni/deck.py                               | 164 +++++++++++--
 ooni/director.py                           |  24 +-
 ooni/errors.py                             |  49 +++-
 ooni/nettests/blocking/web_connectivity.py |  36 ++-
 ooni/oonibclient.py                        | 232 ------------------
 ooni/oonicli.py                            |  35 +--
 ooni/report/tool.py                        |   4 +-
 ooni/reporter.py                           | 220 +++++------------
 ooni/tests/mocks.py                        |  12 +-
 ooni/tests/test_deck.py                    |   9 +-
 ooni/tests/test_oonibclient.py             |  47 ++--
 ooni/tests/test_reporter.py                |  32 ++-
 ooni/tests/test_utils.py                   |  34 +--
 ooni/utils/__init__.py                     |  56 ++---
 15 files changed, 729 insertions(+), 589 deletions(-)

diff --git a/ooni/backend_client.py b/ooni/backend_client.py
new file mode 100644
index 0000000..c7de7f0
--- /dev/null
+++ b/ooni/backend_client.py
@@ -0,0 +1,364 @@
+import os
+import json
+
+from urlparse import urljoin, urlparse
+
+from twisted.web.error import Error
+from twisted.web.client import Agent, Headers
+from twisted.internet import defer, reactor
+from twisted.internet.endpoints import TCP4ClientEndpoint
+
+from twisted.python.versions import Version
+from twisted import version as _twisted_version
+_twisted_14_0_2_version = Version('twisted', 14, 0, 2)
+
+from ooni import errors as e
+from ooni.settings import config
+from ooni.utils import log
+from ooni.utils.net import BodyReceiver, StringProducer, Downloader
+from ooni.utils.trueheaders import TrueHeadersSOCKS5Agent
+
+
+class OONIBClient(object):
+    def __init__(self, address=None, settings={}):
+        self.base_headers = {}
+        self.backend_type = settings.get('type', None)
+        self.base_address = settings.get('address', address).encode('ascii')
+
+        if self.backend_type is None:
+            self._guessBackendType()
+        self.backend_type = self.backend_type.encode('ascii')
+
+        if self.backend_type == 'cloudfront':
+            self.base_headers['Host'] = settings['front'].encode('ascii')
+
+        self._setupBaseAddress()
+        self.settings = {
+            'type': self.backend_type,
+            'address': self.base_address,
+            'front': settings.get('front', '').encode('ascii')
+        }
+
+    def _guessBackendType(self):
+        if self.base_address is None:
+            raise e.InvalidAddress
+        if self.base_address.startswith('https://'):
+            self.backend_type = 'https'
+        elif self.base_address.startswith('httpo://'):
+            self.backend_type = 'onion'
+        elif self.base_address.startswith('http://'):
+            self.backend_type = 'http'
+        else:
+            raise e.InvalidAddress
+
+    def _setupBaseAddress(self):
+        parsed_address = urlparse(self.base_address)
+        if self.backend_type == 'onion':
+            if not parsed_address.netloc.endswith(".onion"):
+                log.err("Invalid onion address.")
+                raise e.InvalidAddress(self.base_address)
+            self.base_address = ("http://%s" % parsed_address.netloc)
+        elif self.backend_type == 'http':
+            self.base_address = ("http://%s" % parsed_address.netloc)
+        elif self.backend_type in ('https', 'cloudfront'):
+            self.base_address = ("https://%s" % parsed_address.netloc)
+
+    def isSupported(self):
+        if self.backend_type in ("https", "cloudfront"):
+            if _twisted_version < _twisted_14_0_2_version:
+                log.err("HTTPS and cloudfronted backends require "
+                        "twisted > 14.0.2.")
+                return False
+        elif self.backend_type == "http":
+            if config.advanced.insecure_collector is not True:
+                log.err("Plaintext backends are not supported. To "
+                        "enable at your own risk set "
+                        "advanced->insecure_collector to true")
+                return False
+        elif self.backend_type == "onion":
+            # XXX add an extra check to ensure tor is running
+            if not config.tor_state and config.tor.socks_port is None:
+                return False
+        return True
+
+    def isReachable(self):
+        raise NotImplemented
+
+    def _request(self, method, urn, genReceiver, bodyProducer=None, retries=3):
+        if self.backend_type == 'onion':
+            agent = TrueHeadersSOCKS5Agent(reactor,
+                                           proxyEndpoint=TCP4ClientEndpoint(reactor,
+                                                                            '127.0.0.1',
+                                                                            config.tor.socks_port))
+        else:
+            agent = Agent(reactor)
+
+        attempts = 0
+
+        finished = defer.Deferred()
+
+        def perform_request(attempts):
+            uri = urljoin(self.base_address, urn)
+            d = agent.request(method, uri, bodyProducer=bodyProducer,
+                              headers=Headers(self.base_headers))
+
+            @d.addCallback
+            def callback(response):
+                try:
+                    content_length = int(response.headers.getRawHeaders('content-length')[0])
+                except:
+                    content_length = None
+                response.deliverBody(genReceiver(finished, content_length))
+
+            def errback(err, attempts):
+                # We we will recursively keep trying to perform a request until
+                # we have reached the retry count.
+                if attempts < retries:
+                    log.err("Lookup failed. Retrying.")
+                    attempts += 1
+                    perform_request(attempts)
+                else:
+                    log.err("Failed. Giving up.")
+                    finished.errback(err)
+
+            d.addErrback(errback, attempts)
+
+        perform_request(attempts)
+
+        return finished
+
+    def queryBackend(self, method, urn, query=None, retries=3):
+        bodyProducer = None
+        if query:
+            bodyProducer = StringProducer(json.dumps(query))
+
+        def genReceiver(finished, content_length):
+            def process_response(s):
+                # If empty string then don't parse it.
+                if not s:
+                    return
+                try:
+                    response = json.loads(s)
+                except ValueError:
+                    raise e.get_error(None)
+                if 'error' in response:
+                    log.err("Got this backend error message %s" % response)
+                    raise e.get_error(response['error'])
+                return response
+
+            return BodyReceiver(finished, content_length, process_response)
+
+        return self._request(method, urn, genReceiver, bodyProducer, retries)
+
+    def download(self, urn, download_path):
+
+        def genReceiver(finished, content_length):
+            return Downloader(download_path, finished, content_length)
+
+        return self._request('GET', urn, genReceiver)
+
+class BouncerClient(OONIBClient):
+    def isReachable(self):
+        pass
+
+    @defer.inlineCallbacks
+    def lookupTestCollector(self, net_tests):
+        try:
+            test_collector = yield self.queryBackend('POST', '/bouncer/net-tests',
+                                                     query={'net-tests': net_tests})
+        except Exception as exc:
+            log.exception(exc)
+            raise e.CouldNotFindTestCollector
+
+        defer.returnValue(test_collector)
+
+    @defer.inlineCallbacks
+    def lookupTestHelpers(self, test_helper_names):
+        try:
+            test_helper = yield self.queryBackend('POST', '/bouncer/test-helpers',
+                                                  query={'test-helpers': test_helper_names})
+        except Exception as exc:
+            log.exception(exc)
+            raise e.CouldNotFindTestHelper
+
+        if not test_helper:
+            raise e.CouldNotFindTestHelper
+
+        defer.returnValue(test_helper)
+
+
+class CollectorClient(OONIBClient):
+    def isReachable(self):
+        # XXX maybe in the future we can have a dedicated API endpoint to
+        # test the reachability of the collector.
+        d = self.queryBackend('GET', '/invalidpath')
+
+        @d.addCallback
+        def cb(_):
+            # We should never be getting an acceptable response for a
+            # request to an invalid path.
+            raise e.CollectorUnreachable
+
+        @d.addErrback
+        def err(failure):
+            failure.trap(Error)
+            if failure.value.status == '404':
+                return True
+            raise e.CollectorUnreachable
+
+        return d
+
+    def getInput(self, input_hash):
+        from ooni.deck import InputFile
+
+        input_file = InputFile(input_hash)
+        if input_file.descriptorCached:
+            return defer.succeed(input_file)
+        else:
+            d = self.queryBackend('GET', '/input/' + input_hash)
+
+            @d.addCallback
+            def cb(descriptor):
+                input_file.load(descriptor)
+                input_file.save()
+                return input_file
+
+            @d.addErrback
+            def err(err):
+                log.err("Failed to get descriptor for input %s" % input_hash)
+                log.exception(err)
+
+            return d
+
+    def getInputList(self):
+        return self.queryBackend('GET', '/input')
+
+    def downloadInput(self, input_hash):
+        from ooni.deck import InputFile
+
+        input_file = InputFile(input_hash)
+
+        if input_file.fileCached:
+            return defer.succeed(input_file)
+        else:
+            d = self.download('/input/' + input_hash + '/file', input_file.cached_file)
+
+            @d.addCallback
+            def cb(res):
+                input_file.verify()
+                return input_file
+
+            @d.addErrback
+            def err(err):
+                log.err("Failed to download the input file %s" % input_hash)
+                log.exception(err)
+
+            return d
+
+    def getInputPolicy(self):
+        return self.queryBackend('GET', '/policy/input')
+
+    def getNettestPolicy(self):
+        return self.queryBackend('GET', '/policy/nettest')
+
+    def getDeckList(self):
+        return self.queryBackend('GET', '/deck')
+
+    def getDeck(self, deck_hash):
+        from ooni.deck import Deck
+
+        deck = Deck(deck_hash)
+        if deck.descriptorCached:
+            return defer.succeed(deck)
+        else:
+            d = self.queryBackend('GET', '/deck/' + deck_hash)
+
+            @d.addCallback
+            def cb(descriptor):
+                deck.load(descriptor)
+                deck.save()
+                return deck
+
+            @d.addErrback
+            def err(err):
+                log.err("Failed to get descriptor for deck %s" % deck_hash)
+                log.exception(err)
+
+            return d
+
+    def downloadDeck(self, deck_hash):
+        from ooni.deck import Deck
+
+        deck = Deck(deck_hash)
+        if deck.fileCached:
+            return defer.succeed(deck)
+        else:
+            d = self.download('/deck/' + deck_hash + '/file', deck.cached_file)
+
+            @d.addCallback
+            def cb(res):
+                deck.verify()
+                return deck
+
+            @d.addErrback
+            def err(err):
+                log.err("Failed to download the deck %s" % deck_hash)
+                log.exception(err)
+
+            return d
+
+    def createReport(self, test_details):
+        request = {
+            'software_name': test_details['software_name'],
+            'software_version': test_details['software_version'],
+            'probe_asn': test_details['probe_asn'],
+            'probe_cc': test_details['probe_cc'],
+            'test_name': test_details['test_name'],
+            'test_version': test_details['test_version'],
+            'test_start_time': test_details['test_start_time'],
+            'input_hashes': test_details['input_hashes'],
+            'data_format_version': test_details['data_format_version'],
+            'format': 'json'
+        }
+        # import values from the environment
+        request.update([(k.lower(),v) for (k,v) in os.environ.iteritems()
+                        if k.startswith('PROBE_')])
+
+        return self.queryBackend('POST', '/report', query=request)
+
+    def updateReport(self, report_id, serialization_format, entry_content):
+        request = {
+            'format': serialization_format,
+            'content': entry_content
+        }
+        return self.queryBackend('POST', '/report/%s' % report_id,
+                                 query=request)
+
+
+    def closeReport(self, report_id):
+        return self.queryBackend('POST', '/report/' + report_id + '/close')
+
+class WebConnectivityClient(OONIBClient):
+    def isReachable(self):
+        # XXX maybe in the future we can have a dedicated API endpoint to
+        # test the reachability of the collector.
+        d = self.queryBackend('GET', '/status')
+
+        @d.addCallback
+        def cb(result):
+            if result.get("status", None) is not "ok":
+                raise e.TestHelperUnreachable
+            return True
+
+        @d.addErrback
+        def err(_):
+            raise e.TestHelperUnreachable
+
+        return d
+
+    def control(self, http_request, tcp_connect):
+        request = {
+            'http_request': http_request,
+            'tcp_connect': tcp_connect
+        }
+        self.queryBackend('POST', '/', query=request)
diff --git a/ooni/deck.py b/ooni/deck.py
index 4b2c502..24c7904 100644
--- a/ooni/deck.py
+++ b/ooni/deck.py
@@ -1,14 +1,12 @@
 # -*- coding: utf-8 -*-
 
-from ooni.oonibclient import OONIBClient
+from ooni.backend_client import CollectorClient, BouncerClient
+from ooni.backend_client import WebConnectivityClient
 from ooni.nettest import NetTestLoader
 from ooni.settings import config
 from ooni.utils import log
 from ooni import errors as e
 
-from twisted import version as _twisted_version
-from twisted.python.versions import Version
-
 from twisted.python.filepath import FilePath
 from twisted.internet import defer
 
@@ -96,7 +94,8 @@ def nettest_to_path(path, allow_arbitrary_paths=False):
 
 class Deck(InputFile):
     # this exists so we can mock it out in unittests
-    _OONIBClient = OONIBClient
+    _BouncerClient = BouncerClient
+    _CollectorClient = CollectorClient
 
     def __init__(self, deck_hash=None,
                  bouncer=None,
@@ -138,7 +137,9 @@ class Deck(InputFile):
                                             annotations=test['options'].get('annotations', {}),
                                             test_file=nettest_path)
             if test['options']['collector']:
-                net_test_loader.collector = test['options']['collector']
+                net_test_loader.collector = CollectorClient(
+                    test['options']['collector']
+                )
             self.insert(net_test_loader)
 
     def insert(self, net_test_loader):
@@ -152,13 +153,6 @@ class Deck(InputFile):
                 raise
             self.requiresTor = True
 
-        if net_test_loader.collector and net_test_loader.collector.startswith('https://'):
-            _twisted_14_0_2_version = Version('twisted', 14, 0, 2)
-            if _twisted_version < _twisted_14_0_2_version:
-                raise e.HTTPCollectorUnsupported
-        elif net_test_loader.collector and net_test_loader.collector.startswith('http://'):
-            if config.advanced.insecure_collector is not True:
-                raise e.InsecureCollector
         self.netTestLoaders.append(net_test_loader)
 
     @defer.inlineCallbacks
@@ -172,9 +166,132 @@ class Deck(InputFile):
             log.msg("Looking up collector and test helpers")
             yield self.lookupCollectorAndTestHelpers()
 
+
+    def sortAddressesByPriority(self, priority_address, alternate_addresses):
+        onion_addresses= []
+        cloudfront_addresses= []
+        https_addresses = []
+        plaintext_addresses = []
+
+        if priority_address.startswith('httpo://'):
+            priority_address = {
+                'address': priority_address,
+                'type': 'onion'
+            }
+        elif priority_address.startswith('https://'):
+            priority_address = {
+                'address': priority_address,
+                'type': 'https'
+            }
+        elif priority_address.startswith('http://'):
+            if config.advanced.insecure_collector is True:
+                priority_address = {
+                    'address': priority_address,
+                    'type': 'http'
+                }
+        else:
+            raise e.InvalidOONIBCollectorAddress
+
+        def filter_by_type(collectors, collector_type):
+            return filter(lambda x: x['type'] == collector_type,
+                          collectors)
+        onion_addresses += filter_by_type(alternate_addresses, 'onion')
+        https_addresses += filter_by_type(alternate_addresses, 'https')
+        cloudfront_addresses += filter_by_type(alternate_addresses,
+                                                'cloudfront')
+
+        if config.advanced.insecure_collector is True:
+            plaintext_addresses += filter_by_type(alternate_addresses, 'http')
+
+        return ([priority_address] +
+                onion_addresses +
+                https_addresses +
+                cloudfront_addresses)
+
+    @defer.inlineCallbacks
+    def getReachableCollector(self, collector_address, collector_alternate):
+        # We prefer onion collector to https collector to cloudfront
+        # collectors to plaintext collectors
+        for collector_settings in self.sortAddressesByPriority(collector_address,
+                                                               collector_alternate):
+            try:
+                collector = self._CollectorClient(settings=collector_settings)
+                if not collector.isSupported():
+                    log.err("Unsupported %s collector %s" % (
+                                collector_settings['type'],
+                                collector_settings['address']))
+                    continue
+                reachable = yield collector.isReachable()
+                if not reachable:
+                    log.err("Unreachable %s collector %s" % (
+                                collector_settings['type'],
+                                collector_settings['address']))
+                    continue
+                defer.returnValue(collector)
+            except e.CollectorUnreachable:
+                log.msg("Could not reach %s collector %s" % (
+                            collector_settings['type'],
+                            collector_settings['address']))
+
+        raise e.NoReachableCollectors
+
+    @defer.inlineCallbacks
+    def getReachableTestHelper(self, test_helper_name, test_helper_address,
+                               test_helper_alternate):
+        # For the moment we look for alternate addresses only of
+        # web_connectivity test helpers.
+        if test_helper_name is 'web_connectivity':
+            for web_connectivity_settings in self.sortAddressesByPriority(
+                    test_helper_address, test_helper_alternate):
+                try:
+                    web_connectivity_test_helper = WebConnectivityClient(web_connectivity_settings)
+                    if not web_connectivity_test_helper.isSupported():
+                        log.err("Unsupported %s web_connectivity test_helper "
+                                "%s" % (
+                                web_connectivity_settings['type'],
+                                web_connectivity_settings['address']
+                        ))
+                        continue
+                    reachable = yield web_connectivity_test_helper.isReachable()
+                    if not reachable:
+                        log.err("Unreachable %s web_connectivity test helper %s" % (
+                            web_connectivity_settings['type'],
+                            web_connectivity_settings['address']
+                        ))
+                        continue
+                    defer.returnValue(web_connectivity_settings)
+                except e.TestHelperUnreachable:
+                    log.err("Unreachable %s web_connectivity test helper %s" % (
+                        web_connectivity_settings['type'],
+                        web_connectivity_settings['address']
+                    ))
+                    continue
+            raise e.NoReachableTestHelpers
+        else:
+            defer.returnValue(test_helper_address.encode('ascii'))
+
+    @defer.inlineCallbacks
+    def getReachableTestHelpersAndCollectors(self, net_tests):
+        for net_test in net_tests:
+            net_test['collector'] = yield self.getReachableCollector(
+                        net_test['collector'],
+                        net_test.get('collector-alternate', [])
+            )
+
+            for test_helper_name, test_helper_address in net_test['test-helpers'].items():
+                 test_helper_alternate = \
+                     net_test.get('test-helpers-alternate', {}).get(test_helper_name, [])
+                 net_test['test-helpers'][test_helper_name] = \
+                            yield self.getReachableTestHelper(
+                                test_helper_name,
+                                test_helper_address,
+                                test_helper_alternate)
+
+        defer.returnValue(net_tests)
+
     @defer.inlineCallbacks
     def lookupCollectorAndTestHelpers(self):
-        oonibclient = self._OONIBClient(self.bouncer)
+        oonibclient = self._BouncerClient(self.bouncer)
 
         required_nettests = []
 
@@ -201,7 +318,14 @@ class Deck(InputFile):
             defer.returnValue(None)
 
         response = yield oonibclient.lookupTestCollector(required_nettests)
-        provided_net_tests = response['net-tests']
+        try:
+            provided_net_tests = yield self.getReachableTestHelpersAndCollectors(response['net-tests'])
+        except e.NoReachableCollectors:
+            log.err("Could not find any reachable collector")
+            raise
+        except e.NoReachableTestHelpers:
+            log.err("Could not find any reachable test helpers")
+            raise
 
         def find_collector_and_test_helpers(test_name, test_version, input_files):
             input_files = [u""+x['hash'] for x in input_files]
@@ -224,12 +348,12 @@ class Deck(InputFile):
                                                 input_files=net_test_loader.inputFiles)
 
             for option, name in net_test_loader.missingTestHelpers:
-                test_helper_address = test_helpers[name].encode('utf-8')
-                net_test_loader.localOptions[option] = test_helper_address
-                net_test_loader.testHelpers[option] = test_helper_address
+                test_helper_address_or_settings = test_helpers[name]
+                net_test_loader.localOptions[option] = test_helper_address_or_settings
+                net_test_loader.testHelpers[option] = test_helper_address_or_settings
 
             if not net_test_loader.collector:
-                net_test_loader.collector = collector.encode('utf-8')
+                net_test_loader.collector = collector
 
     @defer.inlineCallbacks
     def fetchAndVerifyNetTestInput(self, net_test_loader):
@@ -238,7 +362,7 @@ class Deck(InputFile):
         for i in net_test_loader.inputFiles:
             if i['url']:
                 log.debug("Downloading %s" % i['url'])
-                oonibclient = self._OONIBClient(i['address'])
+                oonibclient = self._CollectorClient(i['address'])
 
                 try:
                     input_file = yield oonibclient.downloadInput(i['hash'])
diff --git a/ooni/director.py b/ooni/director.py
index 43ca36c..82d0e85 100644
--- a/ooni/director.py
+++ b/ooni/director.py
@@ -233,7 +233,7 @@ class Director(object):
 
     @defer.inlineCallbacks
     def startNetTest(self, net_test_loader, report_filename,
-                     collector_address=None, no_yamloo=False):
+                     collector_client=None, no_yamloo=False):
         """
         Create the Report for the NetTest and start the report NetTest.
 
@@ -250,7 +250,8 @@ class Director(object):
         if config.privacy.includepcap:
             self.startSniffing(test_details)
         report = Report(test_details, report_filename,
-                        self.reportEntryManager, collector_address,
+                        self.reportEntryManager,
+                        collector_client,
                         no_yamloo)
 
         yield report.open()
@@ -267,7 +268,7 @@ class Director(object):
         finally:
             self.netTestDone(net_test)
 
-    def startSniffing(self, testDetails):
+    def startSniffing(self, test_details):
         """ Start sniffing with Scapy. Exits if required privileges (root) are not
         available.
         """
@@ -276,12 +277,17 @@ class Director(object):
         if config.scapyFactory is None:
             config.scapyFactory = ScapyFactory(config.advanced.interface)
 
-        if not config.reports.pcap:
+        # XXX this is dumb option to have in the ooniprobe.conf. Drop it in
+        # the future.
+        prefix = config.reports.pcap
+        if prefix is None:
             prefix = 'report'
-        else:
-            prefix = config.reports.pcap
-        filename = config.global_options['reportfile'] if 'reportfile' in config.global_options.keys() else None
-        filename_pcap = generate_filename(testDetails, filename=filename, prefix=prefix, extension='pcap')
+
+        filename_pcap = config.global_options.get('pcapfile', None)
+        if filename_pcap is None:
+            filename_pcap = generate_filename(test_details,
+                                              prefix=prefix,
+                                              extension='pcap')
         if len(self.sniffers) > 0:
             pcap_filenames = set(sniffer.pcapwriter.filename for sniffer in self.sniffers.values())
             pcap_filenames.add(filename_pcap)
@@ -289,7 +295,7 @@ class Director(object):
                     ','.join(pcap_filenames))
 
         sniffer = ScapySniffer(filename_pcap)
-        self.sniffers[testDetails['test_name']] = sniffer
+        self.sniffers[test_details['test_name']] = sniffer
         config.scapyFactory.registerProtocol(sniffer)
         log.msg("Starting packet capture to: %s" % filename_pcap)
 
diff --git a/ooni/errors.py b/ooni/errors.py
index 0412b50..e5f26b2 100644
--- a/ooni/errors.py
+++ b/ooni/errors.py
@@ -90,11 +90,14 @@ class UnableToStartTor(DirectorException):
     pass
 
 
-class InvalidOONIBCollectorAddress(Exception):
+class InvalidAddress(Exception):
+    pass
+
+class InvalidOONIBCollectorAddress(InvalidAddress):
     pass
 
 
-class InvalidOONIBBouncerAddress(Exception):
+class InvalidOONIBBouncerAddress(InvalidAddress):
     pass
 
 
@@ -170,6 +173,13 @@ class OONIBInputDescriptorNotFound(OONIBInputError):
     pass
 
 
+class OONIBInvalidInputHash(OONIBError):
+    pass
+
+
+class OONIBInvalidNettestName(OONIBError):
+    pass
+
 class UnableToLoadDeckInput(Exception):
     pass
 
@@ -256,10 +266,14 @@ class ConfigFileIncoherent(Exception):
 def get_error(error_key):
     if error_key == 'test-helpers-key-missing':
         return CouldNotFindTestHelper
-    if error_key == 'input-descriptor-not-found':
+    elif error_key == 'input-descriptor-not-found':
         return OONIBInputDescriptorNotFound
-    if error_key == 'invalid-request':
+    elif error_key == 'invalid-request':
         return OONIBInvalidRequest
+    elif error_key == 'invalid-input-hash':
+        return OONIBInvalidInputHash
+    elif error_key == 'invalid-nettest-name':
+        return OONIBInvalidNettestName
     elif isinstance(error_key, int):
         return Error("%d" % error_key)
     else:
@@ -281,8 +295,33 @@ class ProtocolAlreadyRegistered(Exception):
 class LibraryNotInstalledError(Exception):
     pass
 
+
 class InsecureCollector(Exception):
     pass
 
-class HTTPSCollectorUnsupported(Exception):
+
+class CollectorUnsupported(Exception):
+    pass
+
+class HTTPSCollectorUnsupported(CollectorUnsupported):
+    pass
+
+
+class CollectorUnreachable(Exception):
+    pass
+
+
+class BackendNotSupported(Exception):
+    pass
+
+
+class NoReachableCollectors(Exception):
+    pass
+
+
+class TestHelperUnreachable(Exception):
+    pass
+
+
+class NoReachableTestHelpers(Exception):
     pass
diff --git a/ooni/nettests/blocking/web_connectivity.py b/ooni/nettests/blocking/web_connectivity.py
index eb05835..da25bdc 100644
--- a/ooni/nettests/blocking/web_connectivity.py
+++ b/ooni/nettests/blocking/web_connectivity.py
@@ -17,6 +17,8 @@ from twisted.python import usage
 from ooni import geoip
 from ooni.utils import log
 
+from ooni.backend_client import WebConnectivityClient
+
 from ooni.utils.net import StringProducer, BodyReceiver
 from ooni.templates import httpt, dnst
 from ooni.errors import failureToString
@@ -179,6 +181,14 @@ class WebConnectivityTest(httpt.HTTPTest, dnst.DNSTest):
                 'headers': {}
             }
         }
+        if isinstance(self.localOptions['backend'], dict):
+            self.web_connectivity_client = WebConnectivityClient(
+                settings=self.localOptions['backend']
+            )
+        else:
+            self.web_connectivity_client = WebConnectivityClient(
+                self.localOptions['backend']
+            )
 
     def experiment_dns_query(self):
         log.msg("* doing DNS query for {}".format(self.hostname))
@@ -214,28 +224,10 @@ class WebConnectivityTest(httpt.HTTPTest, dnst.DNSTest):
 
     @defer.inlineCallbacks
     def control_request(self, sockets):
-        bodyProducer = StringProducer(json.dumps({
-            'http_request': self.input,
-            'tcp_connect': sockets
-        }))
-        response = yield self.agent.request("POST",
-                                            str(self.localOptions['backend']),
-                                            bodyProducer=bodyProducer)
-        try:
-            content_length = int(response.headers.getRawHeaders('content-length')[0])
-        except Exception:
-            content_length = None
-
-        finished = defer.Deferred()
-        response.deliverBody(BodyReceiver(finished, content_length))
-        body = yield finished
-        try:
-            self.control = json.loads(body)
-            assert 'http_request' in self.control.keys()
-            assert 'tcp_connect' in self.control.keys()
-            assert 'dns' in self.control.keys()
-        except AssertionError, ValueError:
-            raise InvalidControlResponse(body)
+        self.control = yield self.web_connectivity_client.control(
+            http_request=self.input,
+            tcp_connect=sockets
+        )
         self.report['control'] = self.control
 
     def experiment_http_get_request(self):
diff --git a/ooni/oonibclient.py b/ooni/oonibclient.py
deleted file mode 100644
index 336ae4e..0000000
--- a/ooni/oonibclient.py
+++ /dev/null
@@ -1,232 +0,0 @@
-import json
-
-from urlparse import urljoin
-
-from twisted.web.client import Agent
-from twisted.internet import defer, reactor
-from twisted.internet.endpoints import TCP4ClientEndpoint
-
-from ooni import errors as e
-from ooni.settings import config
-from ooni.utils import log
-from ooni.utils.net import BodyReceiver, StringProducer, Downloader
-from ooni.utils.trueheaders import TrueHeadersSOCKS5Agent
-
-
-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):
-        address = self.address
-        if self.address.startswith('httpo://'):
-            address = self.address.replace('httpo://', 'http://')
-            agent = TrueHeadersSOCKS5Agent(reactor,
-                                           proxyEndpoint=TCP4ClientEndpoint(reactor, '127.0.0.1',
-                                                                            config.tor.socks_port))
-
-        elif self.address.startswith('https://'):
-            log.err("HTTPS based bouncers are currently not supported.")
-            raise e.InvalidOONIBBouncerAddress
-
-        elif self.address.startswith('http://'):
-            log.msg("Warning using unencrypted bouncer")
-            agent = Agent(reactor)
-
-        attempts = 0
-
-        finished = defer.Deferred()
-
-        def perform_request(attempts):
-            uri = urljoin(address, urn)
-            d = agent.request(method, uri, bodyProducer=bodyProducer)
-
-            @d.addCallback
-            def callback(response):
-                try:
-                    content_length = int(response.headers.getRawHeaders('content-length')[0])
-                except:
-                    content_length = None
-                response.deliverBody(genReceiver(finished, content_length))
-
-            def errback(err, attempts):
-                # We we will recursively keep trying to perform a request until
-                # we have reached the retry count.
-                if attempts < self.retries:
-                    log.err("Lookup failed. Retrying.")
-                    attempts += 1
-                    perform_request(attempts)
-                else:
-                    log.err("Failed. Giving up.")
-                    finished.errback(err)
-
-            d.addErrback(errback, attempts)
-
-        perform_request(attempts)
-
-        return finished
-
-    def queryBackend(self, method, urn, query=None):
-        bodyProducer = None
-        if query:
-            bodyProducer = StringProducer(json.dumps(query))
-
-        def genReceiver(finished, content_length):
-            def process_response(s):
-                # If empty string then don't parse it.
-                if not s:
-                    return
-                try:
-                    response = json.loads(s)
-                except ValueError:
-                    raise e.get_error(None)
-                if 'error' in response:
-                    log.err("Got this backend error message %s" % response)
-                    raise e.get_error(response['error'])
-                return response
-
-            return BodyReceiver(finished, content_length, process_response)
-
-        return self._request(method, urn, genReceiver, bodyProducer)
-
-    def download(self, urn, download_path):
-
-        def genReceiver(finished, content_length):
-            return Downloader(download_path, finished, content_length)
-
-        return self._request('GET', urn, genReceiver)
-
-    def getInput(self, input_hash):
-        from ooni.deck import InputFile
-
-        input_file = InputFile(input_hash)
-        if input_file.descriptorCached:
-            return defer.succeed(input_file)
-        else:
-            d = self.queryBackend('GET', '/input/' + input_hash)
-
-            @d.addCallback
-            def cb(descriptor):
-                input_file.load(descriptor)
-                input_file.save()
-                return input_file
-
-            @d.addErrback
-            def err(err):
-                log.err("Failed to get descriptor for input %s" % input_hash)
-                log.exception(err)
-
-            return d
-
-    def getInputList(self):
-        return self.queryBackend('GET', '/input')
-
-    def downloadInput(self, input_hash):
-        from ooni.deck import InputFile
-
-        input_file = InputFile(input_hash)
-
-        if input_file.fileCached:
-            return defer.succeed(input_file)
-        else:
-            d = self.download('/input/' + input_hash + '/file', input_file.cached_file)
-
-            @d.addCallback
-            def cb(res):
-                input_file.verify()
-                return input_file
-
-            @d.addErrback
-            def err(err):
-                log.err("Failed to download the input file %s" % input_hash)
-                log.exception(err)
-
-            return d
-
-    def getInputPolicy(self):
-        return self.queryBackend('GET', '/policy/input')
-
-    def getNettestPolicy(self):
-        return self.queryBackend('GET', '/policy/nettest')
-
-    def getDeckList(self):
-        return self.queryBackend('GET', '/deck')
-
-    def getDeck(self, deck_hash):
-        from ooni.deck import Deck
-
-        deck = Deck(deck_hash)
-        if deck.descriptorCached:
-            return defer.succeed(deck)
-        else:
-            d = self.queryBackend('GET', '/deck/' + deck_hash)
-
-            @d.addCallback
-            def cb(descriptor):
-                deck.load(descriptor)
-                deck.save()
-                return deck
-
-            @d.addErrback
-            def err(err):
-                log.err("Failed to get descriptor for deck %s" % deck_hash)
-                log.exception(err)
-
-            return d
-
-    def downloadDeck(self, deck_hash):
-        from ooni.deck import Deck
-
-        deck = Deck(deck_hash)
-        if deck.fileCached:
-            return defer.succeed(deck)
-        else:
-            d = self.download('/deck/' + deck_hash + '/file', deck.cached_file)
-
-            @d.addCallback
-            def cb(res):
-                deck.verify()
-                return deck
-
-            @d.addErrback
-            def err(err):
-                log.err("Failed to download the deck %s" % deck_hash)
-                log.exception(err)
-
-            return d
-
-    @defer.inlineCallbacks
-    def lookupTestCollector(self, net_tests):
-        try:
-            test_collector = yield self.queryBackend('POST', '/bouncer/net-tests',
-                                                     query={'net-tests': net_tests})
-        except Exception as exc:
-            log.exception(exc)
-            raise e.CouldNotFindTestCollector
-
-        defer.returnValue(test_collector)
-
-    @defer.inlineCallbacks
-    def lookupTestHelpers(self, test_helper_names):
-        try:
-            test_helper = yield self.queryBackend('POST', '/bouncer/test-helpers',
-                                                  query={'test-helpers': test_helper_names})
-        except Exception as exc:
-            log.exception(exc)
-            raise e.CouldNotFindTestHelper
-
-        if not test_helper:
-            raise e.CouldNotFindTestHelper
-
-        defer.returnValue(test_helper)
diff --git a/ooni/oonicli.py b/ooni/oonicli.py
index 281ff46..ca8774f 100644
--- a/ooni/oonicli.py
+++ b/ooni/oonicli.py
@@ -13,6 +13,7 @@ from twisted.internet import defer
 from ooni import errors, __version__
 from ooni.settings import config
 from ooni.utils import log
+from backend_client import CollectorClient
 
 class LifetimeExceeded(Exception): pass
 
@@ -222,18 +223,14 @@ def setupAnnotations(global_options):
     global_options["annotations"] = annotations
     return annotations
 
-def setupCollector(global_options, collector_address):
+def setupCollector(global_options, collector_client):
     if global_options['collector']:
-        collector_address = global_options['collector']
-    elif 'collector' in config.reports \
-            and config.reports['collector']:
-        collector_address = config.reports['collector']
-
-    if collector_address.startswith('httpo:') \
-            and (not (config.tor_state or config.tor.socks_port)):
-        raise errors.TorNotRunning
-    return collector_address
-
+        collector_client = CollectorClient(global_options['collector'])
+    elif config.reports.get('collector', None) is not None:
+        collector_client = CollectorClient(config.reports['collector'])
+    if not collector_client.isSupported():
+        raise errors.CollectorUnsupported
+    return collector_client
 
 def createDeck(global_options, url=None):
     from ooni.nettest import NetTestLoader
@@ -264,7 +261,8 @@ def createDeck(global_options, url=None):
                                             test_file=test_file,
                                             annotations=global_options['annotations'])
             if global_options['collector']:
-                net_test_loader.collector = global_options['collector']
+                net_test_loader.collector = \
+                    CollectorClient(global_options['collector'])
             deck.insert(net_test_loader)
     except errors.MissingRequiredOption as option_name:
         log.err('Missing required option: "%s"' % option_name)
@@ -309,7 +307,10 @@ def runTestWithDirector(director, global_options, url=None, start_tor=True):
             return deck.setup()
         except errors.UnableToLoadDeckInput as error:
             return defer.failure.Failure(error)
-
+        except errors.NoReachableTestHelpers as error:
+            return defer.failure.Failure(error)
+        except errors.NoReachableCollectors as error:
+            return defer.failure.Failure(error)
 
     # Wait until director has started up (including bootstrapping Tor)
     # before adding tests
@@ -324,14 +325,14 @@ def runTestWithDirector(director, global_options, url=None, start_tor=True):
             # If a collector is not specified in the deck, or the
             # deck is a singleton, the default collector set in
             # ooniprobe.conf will be used
-            collector_address = None
+            collector_client = None
             if not global_options['no-collector']:
-                collector_address = setupCollector(global_options,
-                                                   net_test_loader.collector)
+                collector_client = setupCollector(global_options,
+                                                  net_test_loader.collector)
 
             yield director.startNetTest(net_test_loader,
                                         global_options['reportfile'],
-                                        collector_address,
+                                        collector_client,
                                         global_options['no-yamloo'])
 
     d.addCallback(setup_nettest)
diff --git a/ooni/report/tool.py b/ooni/report/tool.py
index 5c7bcb2..fd504a6 100644
--- a/ooni/report/tool.py
+++ b/ooni/report/tool.py
@@ -9,7 +9,7 @@ from ooni.reporter import OONIBReporter, OONIBReportLog
 from ooni.utils import log
 from ooni.report import parser
 from ooni.settings import config
-from ooni.oonibclient import OONIBClient
+from ooni.backend_client import BouncerClient
 
 
 @defer.inlineCallbacks
@@ -23,7 +23,7 @@ def upload(report_file, collector=None, bouncer=None):
 
     report = parser.ReportLoader(report_file)
     if bouncer and not collector:
-        oonib_client = OONIBClient(bouncer)
+        oonib_client = BouncerClient(bouncer)
         net_tests = [{
             'test-helpers': [],
             'input-hashes': report.header['input_hashes'],
diff --git a/ooni/reporter.py b/ooni/reporter.py
index 6103c1e..70c2f56 100644
--- a/ooni/reporter.py
+++ b/ooni/reporter.py
@@ -1,8 +1,6 @@
 import uuid
 import yaml
-import json
 import os
-import re
 
 from copy import deepcopy
 
@@ -15,12 +13,8 @@ from yaml.serializer import Serializer
 from yaml.resolver import Resolver
 
 from twisted.python.util import untilConcludes
-from twisted.internet import defer, reactor
-from twisted.web.client import Agent
+from twisted.internet import defer
 from twisted.internet.error import ConnectionRefusedError
-from twisted.internet.endpoints import TCP4ClientEndpoint
-
-from txsocksx.http import SOCKS5Agent
 
 from ooni.utils import log
 from ooni.tasks import Measurement
@@ -35,8 +29,7 @@ except ImportError:
 from ooni import errors
 
 from ooni import otime
-from ooni.utils import pushFilenameStack, generate_filename
-from ooni.utils.net import BodyReceiver, StringProducer
+from ooni.utils import generate_filename
 
 from ooni.settings import config
 
@@ -147,6 +140,7 @@ class OReporter(object):
     def finish(self):
         pass
 
+
 class YAMLReporter(OReporter):
 
     """
@@ -157,24 +151,8 @@ class YAMLReporter(OReporter):
 
     """
 
-    def __init__(self, test_details, report_destination='.', report_filename=None):
-        self.reportDestination = report_destination
-
-        if not os.path.isdir(report_destination):
-            raise errors.InvalidDestination
-
-        report_filename = generate_filename(test_details,
-                                            filename=report_filename,
-                                            prefix='report',
-                                            extension='yamloo')
-
-        report_path = os.path.join(self.reportDestination, report_filename)
-
-        if os.path.exists(report_path):
-            log.msg("Report already exists with filename %s" % report_path)
-            pushFilenameStack(report_path)
-
-        self.report_path = os.path.abspath(report_path)
+    def __init__(self, test_details, report_filename):
+        self.report_path = report_filename
         OReporter.__init__(self, test_details)
 
     def _writeln(self, line):
@@ -229,42 +207,15 @@ class YAMLReporter(OReporter):
         self._stream.close()
 
 
-def collector_supported(collector_address):
-    if collector_address.startswith('httpo') \
-            and (not (config.tor_state or config.tor.socks_port)):
-        return False
-    return True
-
-
 class OONIBReporter(OReporter):
 
-    def __init__(self, test_details, collector_address):
-        self.collectorAddress = collector_address
-        self.validateCollectorAddress()
+    def __init__(self, test_details, collector_client):
+        self.collector_client = collector_client
 
         self.reportId = None
         self.supportedFormats = ["yaml"]
-
-        if self.collectorAddress.startswith('https://'):
-            # not sure if there's something else it needs.  Seems to work.
-            # Very difficult to get it to work with self-signed certs.
-            self.agent = Agent(reactor)
-
-        elif self.collectorAddress.startswith('http://'):
-            log.msg("Warning using unencrypted collector")
-            self.agent = Agent(reactor)
-
         OReporter.__init__(self, test_details)
 
-    def validateCollectorAddress(self):
-        """
-        Will raise :class:ooni.errors.InvalidOONIBCollectorAddress an exception
-        if the oonib reporter is not valid.
-        """
-        regexp = '^(http|https|httpo):\/\/[a-zA-Z0-9\-\.]+(:\d+)?$'
-        if not re.match(regexp, self.collectorAddress):
-            raise errors.InvalidOONIBCollectorAddress
-
     def serializeEntry(self, entry, serialisation_format="yaml"):
         if serialisation_format == "json":
             if isinstance(entry, Measurement):
@@ -303,29 +254,17 @@ class OONIBReporter(OReporter):
 
     @defer.inlineCallbacks
     def writeReportEntry(self, entry):
-        log.debug("Writing report with OONIB reporter")
-
-        url = self.collectorAddress + '/report/' + self.reportId
-
         if "json" in self.supportedFormats:
-            serialisation_format = 'json'
+            serialization_format = 'json'
         else:
-            serialisation_format = 'yaml'
-
-        request = {
-            'format': serialisation_format,
-            'content': self.serializeEntry(entry, serialisation_format)
-        }
-
-        log.debug("Updating report with id %s (%s)" % (self.reportId, url))
-        request_json = json.dumps(request)
-        log.debug("Sending %s" % request_json)
-
-        bodyProducer = StringProducer(request_json)
+            serialization_format = 'yaml'
 
+        log.debug("Updating report with id %s" % (self.reportId))
+        entry_content = self.serializeEntry(entry, serialization_format)
         try:
-            yield self.agent.request("POST", str(url),
-                                     bodyProducer=bodyProducer)
+            yield self.collector_client.updateReport(self.reportId,
+                                                     serialization_format,
+                                                     entry_content)
         except Exception as exc:
             log.err("Error in writing report entry")
             log.exception(exc)
@@ -336,100 +275,43 @@ class OONIBReporter(OReporter):
         """
         Creates a report on the oonib collector.
         """
-        # XXX we should probably be setting this inside of the constructor,
-        # however config.tor.socks_port is not set until Tor is started and the
-        # reporter is instantiated before Tor is started. We probably want to
-        # do this with some deferred kung foo or instantiate the reporter after
-        # tor is started.
-
-
-        if self.collectorAddress.startswith('httpo://'):
-            self.collectorAddress = \
-                self.collectorAddress.replace('httpo://', 'http://')
-            proxyEndpoint = TCP4ClientEndpoint(reactor, '127.0.0.1',
-                                               config.tor.socks_port)
-            self.agent = SOCKS5Agent(reactor, proxyEndpoint=proxyEndpoint)
-
-        url = self.collectorAddress + '/report'
-
-        request = {
-            'software_name': self.testDetails['software_name'],
-            'software_version': self.testDetails['software_version'],
-            'probe_asn': self.testDetails['probe_asn'],
-            'probe_cc': self.testDetails['probe_cc'],
-            'test_name': self.testDetails['test_name'],
-            'test_version': self.testDetails['test_version'],
-            'test_start_time': self.testDetails['test_start_time'],
-            'input_hashes': self.testDetails['input_hashes'],
-            'data_format_version': self.testDetails['data_format_version'],
-            'format': 'json'
-        }
-        # import values from the environment
-        request.update([(k.lower(),v) for (k,v) in os.environ.iteritems()
-                        if k.startswith('PROBE_')])
-
-        log.msg("Reporting %s" % url)
-        request_json = json.dumps(request)
-        log.debug("Sending %s" % request_json)
-
-        bodyProducer = StringProducer(request_json)
-
         log.msg("Creating report with OONIB Reporter. Please be patient.")
         log.msg("This may take up to 1-2 minutes...")
 
         try:
-            response = yield self.agent.request("POST", url,
-                                                bodyProducer=bodyProducer)
-
+            response = yield self.collector_client.createReport(
+                                            self.testDetails
+            )
         except ConnectionRefusedError:
             log.err("Connection to reporting backend failed "
                     "(ConnectionRefusedError)")
             raise errors.OONIBReportCreationError
-
         except errors.HostUnreachable:
             log.err("Host is not reachable (HostUnreachable error")
             raise errors.OONIBReportCreationError
-
-        except Exception, e:
-            log.err("Failed to connect to reporter backend")
-            log.exception(e)
-            raise errors.OONIBReportCreationError
-
-        # This is a little trix to allow us to unspool the response. We create
-        # a deferred and call yield on it.
-        response_body = defer.Deferred()
-        response.deliverBody(BodyReceiver(response_body))
-
-        backend_response = yield response_body
-
-        try:
-            parsed_response = json.loads(backend_response)
-        except Exception, e:
-            log.err("Failed to parse collector response %s" % backend_response)
-            log.exception(e)
-            raise errors.OONIBReportCreationError
-
-        if response.code == 406:
-            # XXX make this more strict
+        except (errors.OONIBInvalidInputHash,
+                errors.OONIBInvalidNettestName):
             log.err("The specified input or nettests cannot be submitted to "
                     "this collector.")
             log.msg("Try running a different test or try reporting to a "
                     "different collector.")
             raise errors.OONIBReportCreationError
+        except Exception, e:
+            log.err("Failed to connect to reporter backend")
+            log.exception(e)
+            raise errors.OONIBReportCreationError
 
-        self.reportId = parsed_response['report_id']
-        self.backendVersion = parsed_response['backend_version']
+        self.reportId = response['report_id'].encode('ascii')
+        self.backendVersion = response['backend_version']
 
-        self.supportedFormats = parsed_response.get('supported_formats', ["yaml"])
+        self.supportedFormats = response.get('supported_formats', ["yaml"])
 
-        log.debug("Created report with id %s" % parsed_response['report_id'])
-        defer.returnValue(parsed_response['report_id'])
+        log.debug("Created report with id %s" % response['report_id'])
+        defer.returnValue(response['report_id'])
 
     def finish(self):
-        url = self.collectorAddress + '/report/' + self.reportId + '/close'
-        log.debug("Closing the report %s" % url)
-        return self.agent.request("POST", str(url))
-
+        log.debug("Closing report with id %s" % self.reportId)
+        return self.collector_client.closeReport(self.reportId)
 
 class OONIBReportLog(object):
 
@@ -532,33 +414,34 @@ class OONIBReportLog(object):
     def not_created(self, report_file):
         return self.run(self._not_created, report_file)
 
-    def _created(self, report_file, collector_address, report_id):
+    def _created(self, report_file, collector_settings, report_id):
         with self.edit_log() as report:
+            assert report_file is not None
             report[report_file] = {
                 'pid': os.getpid(),
                 'created_at': datetime.now(),
                 'status': 'created',
-                'collector': collector_address,
+                'collector': collector_settings,
                 'report_id': report_id
             }
         return report_id
 
-    def created(self, report_file, collector_address, report_id):
+    def created(self, report_file, collector_settings, report_id):
         return self.run(self._created, report_file,
-                        collector_address, report_id)
+                        collector_settings, report_id)
 
-    def _creation_failed(self, report_file, collector_address):
+    def _creation_failed(self, report_file, collector_settings):
         with self.edit_log() as report:
             report[report_file] = {
                 'pid': os.getpid(),
                 'created_at': datetime.now(),
                 'status': 'creation-failed',
-                'collector': collector_address
+                'collector': collector_settings
             }
 
-    def creation_failed(self, report_file, collector_address):
+    def creation_failed(self, report_file, collector_settings):
         return self.run(self._creation_failed, report_file,
-                        collector_address)
+                        collector_settings)
 
     def _incomplete(self, report_file):
         with self.edit_log() as report:
@@ -583,7 +466,7 @@ class Report(object):
     reportId = None
 
     def __init__(self, test_details, report_filename,
-                 reportEntryManager, collector_address=None,
+                 reportEntryManager, collector_client=None,
                  no_yamloo=False):
         """
         This is an abstraction layer on top of all the configured reporters.
@@ -608,7 +491,9 @@ class Report(object):
                 If we should disable reporting to disk.
         """
         self.test_details = test_details
-        self.collector_address = collector_address
+        self.collector_client = collector_client
+        if report_filename is None:
+            report_filename = self.generateReportFilename()
         self.report_filename = report_filename
 
         self.report_log = OONIBReportLog()
@@ -620,18 +505,25 @@ class Report(object):
         self.done = defer.Deferred()
         self.reportEntryManager = reportEntryManager
 
+    def generateReportFilename(self):
+        report_filename = generate_filename(self.test_details,
+                                            prefix='report',
+                                            extension='yamloo')
+        report_path = os.path.join('.', report_filename)
+        return os.path.abspath(report_path)
+
     def open_oonib_reporter(self):
         def creation_failed(failure):
             self.oonib_reporter = None
             return self.report_log.creation_failed(self.report_filename,
-                                                   self.collector_address)
+                                                   self.collector_client.settings)
 
         def created(report_id):
             if not self.oonib_reporter:
                 return
             self.test_details['report_id'] = report_id
             return self.report_log.created(self.report_filename,
-                                           self.collector_address,
+                                           self.collector_client.settings,
                                            report_id)
 
         d = self.oonib_reporter.createReport()
@@ -645,15 +537,14 @@ class Report(object):
         This will create all the reports that need to be created and fires the
         created callback of the reporter whose report got created.
         """
-        if self.collector_address:
+        if self.collector_client:
             self.oonib_reporter = OONIBReporter(self.test_details,
-                                                self.collector_address)
+                                                self.collector_client)
             self.test_details['report_id'] = yield self.open_oonib_reporter()
 
         if not self.no_yamloo:
             self.yaml_reporter = YAMLReporter(self.test_details,
-                                              report_filename=self.report_filename)
-            self.report_filename = self.yaml_reporter.report_path
+                                              self.report_filename)
             if not self.oonib_reporter:
                 yield self.report_log.not_created(self.report_filename)
             yield defer.maybeDeferred(self.yaml_reporter.createReport)
@@ -724,6 +615,7 @@ class Report(object):
             return self.report_log.closed(self.report_filename)
 
         def oonib_report_failed(result):
+            log.exception(result)
             log.err("Failed to close oonib report.")
 
         def all_reports_closed(_):
diff --git a/ooni/tests/mocks.py b/ooni/tests/mocks.py
index 19b4692..f3f852f 100644
--- a/ooni/tests/mocks.py
+++ b/ooni/tests/mocks.py
@@ -3,7 +3,7 @@ from twisted.internet import defer
 
 from ooni.tasks import BaseTask, TaskWithTimeout
 from ooni.managers import TaskManager
-
+from ooni.backend_client import CollectorClient
 
 class MockMeasurementFailOnce(BaseTask):
     def run(self):
@@ -189,7 +189,7 @@ class MockTaskManager(TaskManager):
         self.successes.append((result, task))
 
 
-class MockOONIBClient(object):
+class MockBouncerClient(object):
     def __init__(self, *args, **kw):
         pass
 
@@ -225,3 +225,11 @@ class MockOONIBClient(object):
                 'test-helpers': test_helpers
             })
         return defer.succeed(ret)
+
+
+class MockCollectorClient(CollectorClient):
+    def isSupported(self):
+        return True
+
+    def isReachable(self):
+        return defer.succeed(True)
diff --git a/ooni/tests/test_deck.py b/ooni/tests/test_deck.py
index 3e2a322..8d415f0 100644
--- a/ooni/tests/test_deck.py
+++ b/ooni/tests/test_deck.py
@@ -5,7 +5,7 @@ from twisted.trial import unittest
 
 from hashlib import sha256
 from ooni.deck import InputFile, Deck
-from ooni.tests.mocks import MockOONIBClient
+from ooni.tests.mocks import MockBouncerClient, MockCollectorClient
 
 net_test_string = """
 from twisted.python import usage
@@ -151,15 +151,16 @@ class TestDeck(BaseTestCase):
     def test_lookup_test_helpers_and_collector(self):
         deck = Deck(bouncer="httpo://foo.onion",
                     decks_directory=".")
-        deck._OONIBClient = MockOONIBClient
+        deck._BouncerClient = MockBouncerClient
+        deck._CollectorClient = MockCollectorClient
         deck.loadDeck(self.deck_file)
 
         self.assertEqual(len(deck.netTestLoaders[0].missingTestHelpers), 1)
 
         yield deck.lookupCollectorAndTestHelpers()
 
-        self.assertEqual(deck.netTestLoaders[0].collector,
-                         'httpo://thirteenchars1234.onion')
+        self.assertEqual(deck.netTestLoaders[0].collector.settings['address'],
+                         'http://thirteenchars1234.onion')
 
         self.assertEqual(deck.netTestLoaders[0].localOptions['backend'],
                          '127.0.0.1')
diff --git a/ooni/tests/test_oonibclient.py b/ooni/tests/test_oonibclient.py
index d064d0a..a14a881 100644
--- a/ooni/tests/test_oonibclient.py
+++ b/ooni/tests/test_oonibclient.py
@@ -7,7 +7,7 @@ from twisted.web import error
 
 from ooni import errors as e
 from ooni.settings import config
-from ooni.oonibclient import OONIBClient
+from ooni.backend_client import CollectorClient, BouncerClient
 from ooni.tests.bases import ConfigTestCase
 
 input_id = '37e60e13536f6afe47a830bfb6b371b5cf65da66d7ad65137344679b24fdccd1'
@@ -34,79 +34,81 @@ class TestOONIBClient(ConfigTestCase):
             os.mkdir(os.path.join(data_dir, 'decks'))
         except Exception:
             self.skipTest("OONIB must be listening on port 8888 to run this test (tor_hidden_service: false)")
-        self.oonibclient = OONIBClient('http://' + host + ':' + str(port))
+        self.collector_client = CollectorClient('http://' + host + ':' + str(port))
 
     @defer.inlineCallbacks
     def test_query(self):
-        res = yield self.oonibclient.queryBackend('GET', '/policy/input')
+        res = yield self.collector_client.queryBackend('GET', '/policy/input')
         self.assertTrue(isinstance(res, list))
 
     @defer.inlineCallbacks
     def test_get_input_list(self):
-        input_list = yield self.oonibclient.getInputList()
+        input_list = yield self.collector_client.getInputList()
         self.assertTrue(isinstance(input_list, list))
 
     @defer.inlineCallbacks
     def test_get_input_descriptor(self):
-        input_descriptor = yield self.oonibclient.getInput(input_id)
+        input_descriptor = yield self.collector_client.getInput(input_id)
         for key in ['name', 'description',
                     'version', 'author', 'date', 'id']:
             self.assertTrue(hasattr(input_descriptor, key))
 
     @defer.inlineCallbacks
     def test_download_input(self):
-        yield self.oonibclient.downloadInput(input_id)
+        yield self.collector_client.downloadInput(input_id)
 
     @defer.inlineCallbacks
     def test_get_deck_list(self):
-        deck_list = yield self.oonibclient.getDeckList()
+        deck_list = yield self.collector_client.getDeckList()
         self.assertTrue(isinstance(deck_list, list))
 
     @defer.inlineCallbacks
     def test_get_deck_descriptor(self):
-        deck_descriptor = yield self.oonibclient.getDeck(deck_id)
+        deck_descriptor = yield self.collector_client.getDeck(deck_id)
         for key in ['name', 'description',
                     'version', 'author', 'date', 'id']:
             self.assertTrue(hasattr(deck_descriptor, key))
 
     @defer.inlineCallbacks
     def test_download_deck(self):
-        yield self.oonibclient.downloadDeck(deck_id)
+        yield self.collector_client.downloadDeck(deck_id)
 
     def test_lookup_invalid_helpers(self):
-        self.oonibclient.address = 'http://127.0.0.1:8888'
+        bouncer_client = BouncerClient('http://127.0.0.1:8888')
         return self.failUnlessFailure(
-            self.oonibclient.lookupTestHelpers([
+            bouncer_client.lookupTestHelpers([
                 'sdadsadsa', 'dns'
             ]), e.CouldNotFindTestHelper)
 
     @defer.inlineCallbacks
     def test_lookup_no_test_helpers(self):
-        self.oonibclient.address = 'http://127.0.0.1:8888'
+        bouncer_client = BouncerClient('http://127.0.0.1:8888')
         required_helpers = []
-        helpers = yield self.oonibclient.lookupTestHelpers(required_helpers)
+        helpers = yield bouncer_client.lookupTestHelpers(required_helpers)
         self.assertTrue('default' in helpers.keys())
 
     @defer.inlineCallbacks
     def test_lookup_test_helpers(self):
-        self.oonibclient.address = 'http://127.0.0.1:8888'
+        bouncer_client = BouncerClient('http://127.0.0.1:8888')
         required_helpers = [u'http-return-json-headers', u'dns']
-        helpers = yield self.oonibclient.lookupTestHelpers(required_helpers)
+        helpers = yield bouncer_client.lookupTestHelpers(required_helpers)
         self.assertEqual(set(helpers.keys()), set(required_helpers + [u'default']))
         self.assertTrue(helpers['http-return-json-headers']['address'].startswith('http'))
         self.assertTrue(int(helpers['dns']['address'].split('.')[0]))
 
     @defer.inlineCallbacks
     def test_input_descriptor_not_found(self):
-        yield self.assertFailure(self.oonibclient.queryBackend('GET', '/input/' + 'a'*64), e.OONIBInputDescriptorNotFound)
+        yield self.assertFailure(self.collector_client.queryBackend('GET',
+                                                             '/input/' + 'a'*64), e.OONIBInputDescriptorNotFound)
 
     @defer.inlineCallbacks
     def test_http_errors(self):
-        yield self.assertFailure(self.oonibclient.queryBackend('PUT', '/policy/input'), error.Error)
+        yield self.assertFailure(self.collector_client.queryBackend('PUT',
+                                                     '/policy/input'), error.Error)
 
     @defer.inlineCallbacks
     def test_create_report(self):
-        res = yield self.oonibclient.queryBackend('POST', '/report', {
+        res = yield self.collector_client.queryBackend('POST', '/report', {
             'software_name': 'spam',
             'software_version': '2.0',
             'probe_asn': 'AS0',
@@ -119,7 +121,7 @@ class TestOONIBClient(ConfigTestCase):
 
     @defer.inlineCallbacks
     def test_report_lifecycle(self):
-        res = yield self.oonibclient.queryBackend('POST', '/report', {
+        res = yield self.collector_client.queryBackend('POST', '/report', {
             'software_name': 'spam',
             'software_version': '2.0',
             'probe_asn': 'AS0',
@@ -130,12 +132,13 @@ class TestOONIBClient(ConfigTestCase):
         })
         report_id = str(res['report_id'])
 
-        res = yield self.oonibclient.queryBackend('POST', '/report/' + report_id, {
+        res = yield self.collector_client.queryBackend('POST', '/report/' + report_id, {
             'content': '---\nspam: ham\n...\n'
         })
 
-        res = yield self.oonibclient.queryBackend('POST', '/report/' + report_id, {
+        res = yield self.collector_client.queryBackend('POST', '/report/' + report_id, {
             'content': '---\nspam: ham\n...\n'
         })
 
-        res = yield self.oonibclient.queryBackend('POST', '/report/' + report_id + '/close')
+        res = yield self.collector_client.queryBackend('POST', '/report/' + report_id +
+                                        '/close')
diff --git a/ooni/tests/test_reporter.py b/ooni/tests/test_reporter.py
index b4e7592..8f32733 100644
--- a/ooni/tests/test_reporter.py
+++ b/ooni/tests/test_reporter.py
@@ -8,6 +8,7 @@ from twisted.internet import defer
 from twisted.trial import unittest
 
 from ooni import errors as e
+from ooni.tests.mocks import MockCollectorClient
 from ooni.reporter import YAMLReporter, OONIBReporter, OONIBReportLog
 
 
@@ -58,7 +59,7 @@ class TestYAMLReporter(unittest.TestCase):
             os.remove(self.filename)
 
     def test_write_report(self):
-        y_reporter = YAMLReporter(test_details)
+        y_reporter = YAMLReporter(test_details, 'dummy-report.yaml')
         y_reporter.createReport()
         with open(y_reporter.report_path) as f:
             self.filename = y_reporter.report_path
@@ -72,33 +73,30 @@ class TestOONIBReporter(unittest.TestCase):
 
     def setUp(self):
         self.mock_response = {}
-        self.collector_address = 'http://example.com'
 
-        self.oonib_reporter = OONIBReporter(
-            test_details,
-            self.collector_address)
-        self.oonib_reporter.agent = MagicMock()
-        self.mock_agent_response = MagicMock()
+        def mockRequest(method, urn, genReceiver, *args, **kw):
+            receiver = genReceiver(None, None)
+            return defer.maybeDeferred(receiver.body_processor,
+                                       json.dumps(self.mock_response))
 
-        def deliverBody(body_receiver):
-            body_receiver.dataReceived(json.dumps(self.mock_response))
-            body_receiver.connectionLost(None)
+        mock_collector_client = MockCollectorClient('http://example.com')
+        mock_collector_client._request = mockRequest
 
-        self.mock_agent_response.deliverBody = deliverBody
-        self.oonib_reporter.agent.request.return_value = defer.succeed(
-            self.mock_agent_response)
+        self.oonib_reporter = OONIBReporter(
+            test_details,
+            mock_collector_client
+        )
 
     @defer.inlineCallbacks
     def test_create_report(self):
         self.mock_response = oonib_new_report_message
         yield self.oonib_reporter.createReport()
-        assert self.oonib_reporter.reportId == oonib_new_report_message[
-            'report_id']
+        self.assertEqual(self.oonib_reporter.reportId,
+                         oonib_new_report_message['report_id'])
 
     @defer.inlineCallbacks
     def test_create_report_failure(self):
         self.mock_response = oonib_generic_error_message
-        self.mock_agent_response.code = 406
         yield self.assertFailure(self.oonib_reporter.createReport(),
                                  e.OONIBReportCreationError)
 
@@ -108,7 +106,6 @@ class TestOONIBReporter(unittest.TestCase):
         yield self.oonib_reporter.createReport()
         req = {'content': 'something'}
         yield self.oonib_reporter.writeReportEntry(req)
-        assert self.oonib_reporter.agent.request.called
 
     @defer.inlineCallbacks
     def test_write_report_entry_in_yaml(self):
@@ -116,7 +113,6 @@ class TestOONIBReporter(unittest.TestCase):
         yield self.oonib_reporter.createReport()
         req = {'content': 'something'}
         yield self.oonib_reporter.writeReportEntry(req)
-        assert self.oonib_reporter.agent.request.called
 
 class TestOONIBReportLog(unittest.TestCase):
 
diff --git a/ooni/tests/test_utils.py b/ooni/tests/test_utils.py
index 855eb19..bbaa26b 100644
--- a/ooni/tests/test_utils.py
+++ b/ooni/tests/test_utils.py
@@ -1,7 +1,7 @@
 import os
 from twisted.trial import unittest
 
-from ooni.utils import pushFilenameStack, log, generate_filename, net
+from ooni.utils import log, generate_filename, net
 
 
 class TestUtils(unittest.TestCase):
@@ -15,26 +15,6 @@ class TestUtils(unittest.TestCase):
         self.basename = 'filename'
         self.filename = 'filename.txe'
 
-    def test_pushFilenameStack(self):
-        basefilename = os.path.join(os.getcwd(), 'dummyfile')
-        f = open(basefilename, "w+")
-        f.write("0\n")
-        f.close()
-        for i in xrange(1, 20):
-            f = open("%s.%d" % (basefilename, i), "w+")
-            f.write("%s\n" % i)
-            f.close()
-
-        pushFilenameStack(basefilename)
-        for i in xrange(1, 20):
-            f = open("%s.%d" % (basefilename, i))
-            c = f.readlines()[0].strip()
-            self.assertEqual(str(i-1), str(c))
-            f.close()
-
-        for i in xrange(1, 21):
-            os.remove("%s.%d" % (basefilename, i))
-
     def test_log_encode(self):
         logmsgs = (
             (r"spam\x07\x08", "spam\a\b"),
@@ -60,18 +40,6 @@ class TestUtils(unittest.TestCase):
         filename = generate_filename(self.test_details, prefix=self.prefix, extension=self.extension)
         self.assertEqual(filename, 'prefix-foo-2016-01-01T012222Z.ext')
 
-    def test_generate_filename_with_filename(self):
-        filename = generate_filename(self.test_details, filename=self.filename)
-        self.assertEqual(filename, 'filename.txe')
-
-    def test_generate_filename_with_extension_and_filename(self):
-        filename = generate_filename(self.test_details, extension=self.extension, filename=self.filename)
-        self.assertEqual(filename, 'filename.ext')
-
-    def test_generate_filename_with_extension_and_basename(self):
-        filename = generate_filename(self.test_details, extension=self.extension, filename=self.basename)
-        self.assertEqual(filename, 'filename.ext')
-
     def test_get_addresses(self):
         addresses = net.getAddresses()
         assert isinstance(addresses, list)
diff --git a/ooni/utils/__init__.py b/ooni/utils/__init__.py
index 78a1c7b..22af062 100644
--- a/ooni/utils/__init__.py
+++ b/ooni/utils/__init__.py
@@ -91,51 +91,29 @@ def randomStr(length, num=True):
         chars += string.digits
     return ''.join(random.choice(chars) for x in range(length))
 
-
-def pushFilenameStack(filename):
-    """
-    Takes as input a target filename and checks to see if a file by such name
-    already exists. If it does exist then it will attempt to rename it to .1,
-    if .1 exists it will rename .1 to .2 if .2 exists then it will rename it to
-    .3, etc.
-    This is similar to pushing into a LIFO stack.
-
-    Args:
-        filename (str): the path to filename that you wish to create.
-    """
-    stack = glob.glob(filename + ".*")
-    stack.sort(key=lambda x: int(x.split('.')[-1]))
-    for f in reversed(stack):
-        c_idx = f.split(".")[-1]
-        c_filename = '.'.join(f.split(".")[:-1])
-        new_idx = int(c_idx) + 1
-        new_filename = "%s.%s" % (c_filename, new_idx)
-        os.rename(f, new_filename)
-    os.rename(filename, filename + ".1")
-
-
-def generate_filename(testDetails, prefix=None, extension=None, filename=None):
+def generate_filename(test_details, prefix=None, extension=None):
     """
     Returns a filename for every test execution.
 
     It's used to assure that all files of a certain test have a common basename but different
     extension.
     """
-    if filename is None:
-        test_name, start_time = testDetails['test_name'], testDetails['test_start_time']
-        start_time = datetime.strptime(start_time, "%Y-%m-%d %H:%M:%S").strftime("%Y-%m-%dT%H%M%SZ")
-        suffix = "%s-%s" % (test_name, start_time)
-        basename = '%s-%s' % (prefix, suffix) if prefix is not None else suffix
-        final_filename = '%s.%s' % (basename, extension) if extension is not None else basename
-    else:
-        if extension is not None:
-            basename = filename.split('.')[0] if '.' in filename else filename
-            final_filename = '%s.%s' % (basename, extension)
-        else:
-            final_filename = filename
-
-    return final_filename
-
+    LONG_DATE = "%Y-%m-%d %H:%M:%S"
+    SHORT_DATE = "%Y-%m-%dT%H%M%SZ"
+
+    kwargs = {}
+    filename_format = ""
+    if prefix is not None:
+        kwargs["prefix"] = prefix
+        filename_format += "{prefix}-"
+    filename_format += "{test_name}-{timestamp}"
+    if extension is not None:
+        kwargs["extension"] = extension
+        filename_format += ".{extension}"
+    kwargs['test_name']  = test_details['test_name']
+    kwargs['timestamp'] = datetime.strptime(test_details['test_start_time'],
+                                            LONG_DATE).strftime(SHORT_DATE)
+    return filename_format.format(**kwargs)
 
 def sanitize_options(options):
     """





More information about the tor-commits mailing list