Merge branch 'master' into 3552.test_system-python-3

nix/tahoe-lafs.nix

@@ -29,10 +29,6 @@ python.pkgs.buildPythonPackage rec {
     rm src/allmydata/test/test_connections.py
     rm src/allmydata/test/cli/test_create.py
     rm src/allmydata/test/test_client.py
-
-    # Some eliot code changes behavior based on whether stdout is a tty or not
-    # and fails when it is not.
-    rm src/allmydata/test/test_eliotutil.py
   '';
 
 

src/allmydata/client.py

@@ -270,7 +270,7 @@ def create_client_from_config(config, _client_factory=None, _introducer_factory=
 
     i2p_provider = create_i2p_provider(reactor, config)
     tor_provider = create_tor_provider(reactor, config)
-    handlers = node.create_connection_handlers(reactor, config, i2p_provider, tor_provider)
+    handlers = node.create_connection_handlers(config, i2p_provider, tor_provider)
     default_connection_handlers, foolscap_connection_handlers = handlers
     tub_options = node.create_tub_options(config)
 

src/allmydata/interfaces.py

@@ -3141,3 +3141,24 @@ class IAnnounceableStorageServer(Interface):
         :type: ``IReferenceable`` provider
         """
     )
+
+
+class IAddressFamily(Interface):
+    """
+    Support for one specific address family.
+
+    This stretches the definition of address family to include things like Tor
+    and I2P.
+    """
+    def get_listener():
+        """
+        Return a string endpoint description or an ``IStreamServerEndpoint``.
+
+        This would be named ``get_server_endpoint`` if not for historical
+        reasons.
+        """
+
+    def get_client_endpoint():
+        """
+        Return an ``IStreamClientEndpoint``.
+        """

src/allmydata/introducer/server.py

@@ -70,7 +70,7 @@ def create_introducer(basedir=u"."):
         i2p_provider = create_i2p_provider(reactor, config)
         tor_provider = create_tor_provider(reactor, config)
 
-        default_connection_handlers, foolscap_connection_handlers = create_connection_handlers(reactor, config, i2p_provider, tor_provider)
+        default_connection_handlers, foolscap_connection_handlers = create_connection_handlers(config, i2p_provider, tor_provider)
         tub_options = create_tub_options(config)
 
         # we don't remember these because the Introducer doesn't make

src/allmydata/node.py

@@ -616,28 +616,20 @@ def _make_tcp_handler():
     return default()
 
 
-def create_connection_handlers(reactor, config, i2p_provider, tor_provider):
+def create_default_connection_handlers(config, handlers):
     """
-    :returns: 2-tuple of default_connection_handlers, foolscap_connection_handlers
+    :return: A dictionary giving the default connection handlers.  The keys
+        are strings like "tcp" and the values are strings like "tor" or
+        ``None``.
     """
     reveal_ip = config.get_config("node", "reveal-IP-address", True, boolean=True)
 
-    # We store handlers for everything. None means we were unable to
+    # Remember the default mappings from tahoe.cfg
-    # create that handler, so hints which want it will be ignored.
+    default_connection_handlers = {
-    handlers = foolscap_connection_handlers = {
+        name: name
-        "tcp": _make_tcp_handler(),
+        for name
-        "tor": tor_provider.get_tor_handler(),
+        in handlers
-        "i2p": i2p_provider.get_i2p_handler(),
+    }
-        }
-    log.msg(
-        format="built Foolscap connection handlers for: %(known_handlers)s",
-        known_handlers=sorted([k for k,v in handlers.items() if v]),
-        facility="tahoe.node",
-        umid="PuLh8g",
-    )
-
-    # then we remember the default mappings from tahoe.cfg
-    default_connection_handlers = {"tor": "tor", "i2p": "i2p"}
     tcp_handler_name = config.get_config("connections", "tcp", "tcp").lower()
     if tcp_handler_name == "disabled":
         default_connection_handlers["tcp"] = None
@@ -662,10 +654,35 @@ def create_connection_handlers(reactor, config, i2p_provider, tor_provider):
 
     if not reveal_ip:
         if default_connection_handlers.get("tcp") == "tcp":
-            raise PrivacyError("tcp = tcp, must be set to 'tor' or 'disabled'")
+            raise PrivacyError(
-    return default_connection_handlers, foolscap_connection_handlers
+                "Privacy requested with `reveal-IP-address = false` "
+                "but `tcp = tcp` conflicts with this.",
+            )
+    return default_connection_handlers
 
 
+def create_connection_handlers(config, i2p_provider, tor_provider):
+    """
+    :returns: 2-tuple of default_connection_handlers, foolscap_connection_handlers
+    """
+    # We store handlers for everything. None means we were unable to
+    # create that handler, so hints which want it will be ignored.
+    handlers = {
+        "tcp": _make_tcp_handler(),
+        "tor": tor_provider.get_tor_handler(),
+        "i2p": i2p_provider.get_i2p_handler(),
+    }
+    log.msg(
+        format="built Foolscap connection handlers for: %(known_handlers)s",
+        known_handlers=sorted([k for k,v in handlers.items() if v]),
+        facility="tahoe.node",
+        umid="PuLh8g",
+    )
+    return create_default_connection_handlers(
+        config,
+        handlers,
+    ), handlers
+
 
 def create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,
                handler_overrides={}, **kwargs):
@@ -705,8 +722,21 @@ def _convert_tub_port(s):
     return us
 
 
-def _tub_portlocation(config):
+class PortAssignmentRequired(Exception):
     """
+    A Tub port number was configured to be 0 where this is not allowed.
+    """
+
+
+def _tub_portlocation(config, get_local_addresses_sync, allocate_tcp_port):
+    """
+    Figure out the network location of the main tub for some configuration.
+
+    :param get_local_addresses_sync: A function like
+        ``iputil.get_local_addresses_sync``.
+
+    :param allocate_tcp_port: A function like ``iputil.allocate_tcp_port``.
+
     :returns: None or tuple of (port, location) for the main tub based
         on the given configuration. May raise ValueError or PrivacyError
         if there are problems with the config
@@ -746,7 +776,7 @@ def _tub_portlocation(config):
             file_tubport = fileutil.read(config.portnum_fname).strip()
             tubport = _convert_tub_port(file_tubport)
         else:
-            tubport = "tcp:%d" % iputil.allocate_tcp_port()
+            tubport = "tcp:%d" % (allocate_tcp_port(),)
             fileutil.write_atomically(config.portnum_fname, tubport + "\n",
                                       mode="")
     else:
@@ -754,7 +784,7 @@ def _tub_portlocation(config):
 
     for port in tubport.split(","):
         if port in ("0", "tcp:0"):
-            raise ValueError("tub.port cannot be 0: you must choose")
+            raise PortAssignmentRequired()
 
     if cfg_location is None:
         cfg_location = "AUTO"
@@ -766,7 +796,7 @@ def _tub_portlocation(config):
     if "AUTO" in split_location:
         if not reveal_ip:
             raise PrivacyError("tub.location uses AUTO")
-        local_addresses = iputil.get_local_addresses_sync()
+        local_addresses = get_local_addresses_sync()
         # tubport must be like "tcp:12345" or "tcp:12345:morestuff"
         local_portnum = int(tubport.split(":")[1])
     new_locations = []
@@ -797,6 +827,33 @@ def _tub_portlocation(config):
     return tubport, location
 
 
+def tub_listen_on(i2p_provider, tor_provider, tub, tubport, location):
+    """
+    Assign a Tub its listener locations.
+
+    :param i2p_provider: See ``allmydata.util.i2p_provider.create``.
+    :param tor_provider: See ``allmydata.util.tor_provider.create``.
+    """
+    for port in tubport.split(","):
+        if port == "listen:i2p":
+            # the I2P provider will read its section of tahoe.cfg and
+            # return either a fully-formed Endpoint, or a descriptor
+            # that will create one, so we don't have to stuff all the
+            # options into the tub.port string (which would need a lot
+            # of escaping)
+            port_or_endpoint = i2p_provider.get_listener()
+        elif port == "listen:tor":
+            port_or_endpoint = tor_provider.get_listener()
+        else:
+            port_or_endpoint = port
+        # Foolscap requires native strings:
+        if isinstance(port_or_endpoint, (bytes, str)):
+            port_or_endpoint = ensure_str(port_or_endpoint)
+        tub.listenOn(port_or_endpoint)
+    # This last step makes the Tub is ready for tub.registerReference()
+    tub.setLocation(location)
+
+
 def create_main_tub(config, tub_options,
                     default_connection_handlers, foolscap_connection_handlers,
                     i2p_provider, tor_provider,
@@ -821,36 +878,34 @@ def create_main_tub(config, tub_options,
     :param tor_provider: None, or a _Provider instance if txtorcon +
         Tor are installed.
     """
-    portlocation = _tub_portlocation(config)
+    portlocation = _tub_portlocation(
+        config,
+        iputil.get_local_addresses_sync,
+        iputil.allocate_tcp_port,
+    )
 
-    certfile = config.get_private_path("node.pem")  # FIXME? "node.pem" was the CERTFILE option/thing
+    # FIXME? "node.pem" was the CERTFILE option/thing
-    tub = create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,
+    certfile = config.get_private_path("node.pem")
-                     handler_overrides=handler_overrides, certFile=certfile)
 
-    if portlocation:
+    tub = create_tub(
-        tubport, location = portlocation
+        tub_options,
-        for port in tubport.split(","):
+        default_connection_handlers,
-            if port == "listen:i2p":
+        foolscap_connection_handlers,
-                # the I2P provider will read its section of tahoe.cfg and
+        handler_overrides=handler_overrides,
-                # return either a fully-formed Endpoint, or a descriptor
+        certFile=certfile,
-                # that will create one, so we don't have to stuff all the
+    )
-                # options into the tub.port string (which would need a lot
+    if portlocation is None:
-                # of escaping)
-                port_or_endpoint = i2p_provider.get_listener()
-            elif port == "listen:tor":
-                port_or_endpoint = tor_provider.get_listener()
-            else:
-                port_or_endpoint = port
-            # Foolscap requires native strings:
-            if isinstance(port_or_endpoint, (bytes, str)):
-                port_or_endpoint = ensure_str(port_or_endpoint)
-            tub.listenOn(port_or_endpoint)
-        tub.setLocation(location)
-        log.msg("Tub location set to %s" % (location,))
-        # the Tub is now ready for tub.registerReference()
-    else:
         log.msg("Tub is not listening")
-
+    else:
+        tubport, location = portlocation
+        tub_listen_on(
+            i2p_provider,
+            tor_provider,
+            tub,
+            tubport,
+            location,
+        )
+        log.msg("Tub location set to %s" % (location,))
     return tub
 
 

src/allmydata/scripts/tahoe_run.py

@@ -18,6 +18,10 @@ from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_pat
 from allmydata.util.configutil import UnknownConfigError
 from allmydata.util.deferredutil import HookMixin
 
+from allmydata.node import (
+    PortAssignmentRequired,
+    PrivacyError,
+)
 
 def get_pidfile(basedir):
     """
@@ -146,6 +150,10 @@ class DaemonizeTheRealService(Service, HookMixin):
             def handle_config_error(reason):
                 if reason.check(UnknownConfigError):
                     self.stderr.write("\nConfiguration error:\n{}\n\n".format(reason.value))
+                elif reason.check(PortAssignmentRequired):
+                    self.stderr.write("\ntub.port cannot be 0: you must choose.\n\n")
+                elif reason.check(PrivacyError):
+                    self.stderr.write("\n{}\n\n".format(reason.value))
                 else:
                     self.stderr.write("\nUnknown error\n")
                     reason.printTraceback(self.stderr)

src/allmydata/test/cli/test_run.py

@@ -0,0 +1,127 @@
+"""
+Tests for ``allmydata.scripts.tahoe_run``.
+"""
+
+from six.moves import (
+    StringIO,
+)
+
+from testtools.matchers import (
+    Contains,
+    Equals,
+)
+
+from twisted.python.filepath import (
+    FilePath,
+)
+from twisted.internet.testing import (
+    MemoryReactor,
+)
+from twisted.internet.test.modulehelpers import (
+    AlternateReactor,
+)
+
+from ...scripts.tahoe_run import (
+    DaemonizeTheRealService,
+)
+
+from ...scripts.runner import (
+    parse_options
+)
+from ..common import (
+    SyncTestCase,
+)
+
+class DaemonizeTheRealServiceTests(SyncTestCase):
+    """
+    Tests for ``DaemonizeTheRealService``.
+    """
+    def _verify_error(self, config, expected):
+        """
+        Assert that when ``DaemonizeTheRealService`` is started using the given
+        configuration it writes the given message to stderr and stops the
+        reactor.
+
+        :param bytes config: The contents of a ``tahoe.cfg`` file to give to
+            the service.
+
+        :param bytes expected: A string to assert appears in stderr after the
+            service starts.
+        """
+        nodedir = FilePath(self.mktemp())
+        nodedir.makedirs()
+        nodedir.child("tahoe.cfg").setContent(config)
+        nodedir.child("tahoe-client.tac").touch()
+
+        options = parse_options(["run", nodedir.path])
+        stdout = options.stdout = StringIO()
+        stderr = options.stderr = StringIO()
+        run_options = options.subOptions
+
+        reactor = MemoryReactor()
+        with AlternateReactor(reactor):
+            service = DaemonizeTheRealService(
+                "client",
+                nodedir.path,
+                run_options,
+            )
+            service.startService()
+
+            # We happen to know that the service uses reactor.callWhenRunning
+            # to schedule all its work (though I couldn't tell you *why*).
+            # Make sure those scheduled calls happen.
+            waiting = reactor.whenRunningHooks[:]
+            del reactor.whenRunningHooks[:]
+            for f, a, k in waiting:
+                f(*a, **k)
+
+        self.assertThat(
+            reactor.hasStopped,
+            Equals(True),
+        )
+
+        self.assertThat(
+            stdout.getvalue(),
+            Equals(""),
+        )
+
+        self.assertThat(
+            stderr.getvalue(),
+            Contains(expected),
+        )
+
+    def test_unknown_config(self):
+        """
+        If there are unknown items in the node configuration file then a short
+        message introduced with ``"Configuration error:"`` is written to
+        stderr.
+        """
+        self._verify_error("[invalid-section]\n", "Configuration error:")
+
+    def test_port_assignment_required(self):
+        """
+        If ``tub.port`` is configured to use port 0 then a short message rejecting
+        this configuration is written to stderr.
+        """
+        self._verify_error(
+            """
+            [node]
+            tub.port = 0
+            """,
+            "tub.port cannot be 0",
+        )
+
+    def test_privacy_error(self):
+        """
+        If ``reveal-IP-address`` is set to false and the tub is not configured in
+        a way that avoids revealing the node's IP address, a short message
+        about privacy is written to stderr.
+        """
+        self._verify_error(
+            """
+            [node]
+            tub.port = AUTO
+            reveal-IP-address = false
+            """,
+            "Privacy requested",
+        )

src/allmydata/test/common.py

@@ -64,10 +64,16 @@ from twisted.internet.endpoints import AdoptedStreamServerEndpoint
 from twisted.trial.unittest import TestCase as _TrialTestCase
 
 from allmydata import uri
-from allmydata.interfaces import IMutableFileNode, IImmutableFileNode,\
+from allmydata.interfaces import (
-                                 NotEnoughSharesError, ICheckable, \
+    IMutableFileNode,
-                                 IMutableUploadable, SDMF_VERSION, \
+    IImmutableFileNode,
-                                 MDMF_VERSION
+    NotEnoughSharesError,
+    ICheckable,
+    IMutableUploadable,
+    SDMF_VERSION,
+    MDMF_VERSION,
+    IAddressFamily,
+)
 from allmydata.check_results import CheckResults, CheckAndRepairResults, \
      DeepCheckResults, DeepCheckAndRepairResults
 from allmydata.storage_client import StubServer
@@ -1147,6 +1153,28 @@ def _corrupt_uri_extension(data, debug=False):
     return corrupt_field(data, 0x0c+uriextoffset, uriextlen)
 
 
+
+@attr.s
+@implementer(IAddressFamily)
+class ConstantAddresses(object):
+    """
+    Pretend to provide support for some address family but just hand out
+    canned responses.
+    """
+    _listener = attr.ib(default=None)
+    _handler = attr.ib(default=None)
+
+    def get_listener(self):
+        if self._listener is None:
+            raise Exception("{!r} has no listener.")
+        return self._listener
+
+    def get_client_endpoint(self):
+        if self._handler is None:
+            raise Exception("{!r} has no client endpoint.")
+        return self._handler
+
+
 class _TestCaseMixin(object):
     """
     A mixin for ``TestCase`` which collects helpful behaviors for subclasses.

src/allmydata/test/test_connections.py

@@ -6,7 +6,7 @@ from twisted.internet.interfaces import IStreamClientEndpoint
 from foolscap.connections import tcp
 from ..node import PrivacyError, config_from_string
 from ..node import create_connection_handlers
-from ..node import create_main_tub, _tub_portlocation
+from ..node import create_main_tub
 from ..util.i2p_provider import create as create_i2p_provider
 from ..util.tor_provider import create as create_tor_provider
 
@@ -22,7 +22,7 @@ class TCP(unittest.TestCase):
             "no-basedir",
             BASECONFIG,
         )
-        _, foolscap_handlers = create_connection_handlers(None, config, mock.Mock(), mock.Mock())
+        _, foolscap_handlers = create_connection_handlers(config, mock.Mock(), mock.Mock())
         self.assertIsInstance(
             foolscap_handlers['tcp'],
             tcp.DefaultTCP,
@@ -341,7 +341,7 @@ class Connections(unittest.TestCase):
         self.config = config_from_string("fake.port", self.basedir, BASECONFIG)
 
     def test_default(self):
-        default_connection_handlers, _ = create_connection_handlers(None, self.config, mock.Mock(), mock.Mock())
+        default_connection_handlers, _ = create_connection_handlers(self.config, mock.Mock(), mock.Mock())
         self.assertEqual(default_connection_handlers["tcp"], "tcp")
         self.assertEqual(default_connection_handlers["tor"], "tor")
         self.assertEqual(default_connection_handlers["i2p"], "i2p")
@@ -352,7 +352,7 @@ class Connections(unittest.TestCase):
             "no-basedir",
             BASECONFIG + "[connections]\ntcp = tor\n",
         )
-        default_connection_handlers, _ = create_connection_handlers(None, config, mock.Mock(), mock.Mock())
+        default_connection_handlers, _ = create_connection_handlers(config, mock.Mock(), mock.Mock())
 
         self.assertEqual(default_connection_handlers["tcp"], "tor")
         self.assertEqual(default_connection_handlers["tor"], "tor")
@@ -368,7 +368,7 @@ class Connections(unittest.TestCase):
             )
             with self.assertRaises(ValueError) as ctx:
                 tor_provider = create_tor_provider(reactor, self.config)
-                default_connection_handlers, _ = create_connection_handlers(None, self.config, mock.Mock(), tor_provider)
+                default_connection_handlers, _ = create_connection_handlers(self.config, mock.Mock(), tor_provider)
         self.assertEqual(
             str(ctx.exception),
             "'tahoe.cfg [connections] tcp='"
@@ -383,7 +383,7 @@ class Connections(unittest.TestCase):
             BASECONFIG + "[connections]\ntcp = unknown\n",
         )
         with self.assertRaises(ValueError) as ctx:
-            create_connection_handlers(None, config, mock.Mock(), mock.Mock())
+            create_connection_handlers(config, mock.Mock(), mock.Mock())
         self.assertIn("'tahoe.cfg [connections] tcp='", str(ctx.exception))
         self.assertIn("uses unknown handler type 'unknown'", str(ctx.exception))
 
@@ -393,7 +393,7 @@ class Connections(unittest.TestCase):
             "no-basedir",
             BASECONFIG + "[connections]\ntcp = disabled\n",
         )
-        default_connection_handlers, _ = create_connection_handlers(None, config, mock.Mock(), mock.Mock())
+        default_connection_handlers, _ = create_connection_handlers(config, mock.Mock(), mock.Mock())
         self.assertEqual(default_connection_handlers["tcp"], None)
         self.assertEqual(default_connection_handlers["tor"], "tor")
         self.assertEqual(default_connection_handlers["i2p"], "i2p")
@@ -408,11 +408,12 @@ class Privacy(unittest.TestCase):
         )
 
         with self.assertRaises(PrivacyError) as ctx:
-            create_connection_handlers(None, config, mock.Mock(), mock.Mock())
+            create_connection_handlers(config, mock.Mock(), mock.Mock())
 
         self.assertEqual(
             str(ctx.exception),
-            "tcp = tcp, must be set to 'tor' or 'disabled'",
+            "Privacy requested with `reveal-IP-address = false` "
+            "but `tcp = tcp` conflicts with this.",
         )
 
     def test_connections_tcp_disabled(self):
@@ -422,7 +423,7 @@ class Privacy(unittest.TestCase):
             BASECONFIG + "[connections]\ntcp = disabled\n" +
             "[node]\nreveal-IP-address = false\n",
         )
-        default_connection_handlers, _ = create_connection_handlers(None, config, mock.Mock(), mock.Mock())
+        default_connection_handlers, _ = create_connection_handlers(config, mock.Mock(), mock.Mock())
         self.assertEqual(default_connection_handlers["tcp"], None)
 
     def test_tub_location_auto(self):
@@ -438,31 +439,3 @@ class Privacy(unittest.TestCase):
             str(ctx.exception),
             "tub.location uses AUTO",
         )
-
-    def test_tub_location_tcp(self):
-        config = config_from_string(
-            "fake.port",
-            "no-basedir",
-            BASECONFIG + "[node]\nreveal-IP-address = false\ntub.location=tcp:hostname:1234\n",
-        )
-        with self.assertRaises(PrivacyError) as ctx:
-            _tub_portlocation(config)
-        self.assertEqual(
-            str(ctx.exception),
-            "tub.location includes tcp: hint",
-        )
-
-    def test_tub_location_legacy_tcp(self):
-        config = config_from_string(
-            "fake.port",
-            "no-basedir",
-            BASECONFIG + "[node]\nreveal-IP-address = false\ntub.location=hostname:1234\n",
-        )
-
-        with self.assertRaises(PrivacyError) as ctx:
-            _tub_portlocation(config)
-
-        self.assertEqual(
-            str(ctx.exception),
-            "tub.location includes tcp: hint",
-        )

src/allmydata/test/test_eliotutil.py

@@ -1,5 +1,7 @@
 """
-Tests for ``allmydata.test.eliotutil``.
+Tests for ``allmydata.util.eliotutil``.
+
+Ported to Python 3.
 """
 
 from __future__ import (
@@ -9,6 +11,10 @@ from __future__ import (
     division,
 )
 
+from future.utils import PY2
+if PY2:
+    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
+
 from sys import stdout
 import logging
 

src/allmydata/test/test_node.py

@@ -6,7 +6,7 @@ from __future__ import division
 from __future__ import print_function
 from __future__ import unicode_literals
 
-from future.utils import PY2
+from future.utils import PY2, native_str
 if PY2:
     from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
 
@@ -15,7 +15,6 @@ import os
 import stat
 import sys
 import time
-import mock
 from textwrap import dedent
 import configparser
 
@@ -39,9 +38,13 @@ import foolscap.logging.log
 
 from twisted.application import service
 from allmydata.node import (
+    PortAssignmentRequired,
+    PrivacyError,
+    tub_listen_on,
     create_tub_options,
     create_main_tub,
     create_node_dir,
+    create_default_connection_handlers,
     create_connection_handlers,
     config_from_string,
     read_config,
@@ -64,6 +67,9 @@ from allmydata.util.i2p_provider import create as create_i2p_provider
 from allmydata.util.tor_provider import create as create_tor_provider
 import allmydata.test.common_util as testutil
 
+from .common import (
+    ConstantAddresses,
+)
 
 def port_numbers():
     return integers(min_value=1, max_value=2 ** 16 - 1)
@@ -85,7 +91,7 @@ def testing_tub(config_data=''):
 
     i2p_provider = create_i2p_provider(reactor, config)
     tor_provider = create_tor_provider(reactor, config)
-    handlers = create_connection_handlers(reactor, config, i2p_provider, tor_provider)
+    handlers = create_connection_handlers(config, i2p_provider, tor_provider)
     default_connection_handlers, foolscap_connection_handlers = handlers
     tub_options = create_tub_options(config)
 
@@ -511,27 +517,63 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             new_config.get_config("foo", "bar")
 
 
+def _stub_get_local_addresses_sync():
+    """
+    A function like ``allmydata.util.iputil.get_local_addresses_sync``.
+    """
+    return ["LOCAL"]
+
+
+def _stub_allocate_tcp_port():
+    """
+    A function like ``allmydata.util.iputil.allocate_tcp_port``.
+    """
+    return 999
+
+
 class TestMissingPorts(unittest.TestCase):
     """
-    Test certain error-cases for ports setup
+    Test certain ``_tub_portlocation`` error cases for ports setup.
     """
-
     def setUp(self):
         self.basedir = self.mktemp()
         create_node_dir(self.basedir, "testing")
 
+    def test_listen_on_zero(self):
+        """
+        ``_tub_portlocation`` raises ``PortAssignmentRequired`` called with a
+        listen address including port 0 and no interface.
+        """
+        config_data = (
+            "[node]\n"
+            "tub.port = tcp:0\n"
+        )
+        config = config_from_string(self.basedir, "portnum", config_data)
+        with self.assertRaises(PortAssignmentRequired):
+            _tub_portlocation(config, None, None)
+
+    def test_listen_on_zero_with_host(self):
+        """
+        ``_tub_portlocation`` raises ``PortAssignmentRequired`` called with a
+        listen address including port 0 and an interface.
+        """
+        config_data = (
+            "[node]\n"
+            "tub.port = tcp:0:interface=127.0.0.1\n"
+        )
+        config = config_from_string(self.basedir, "portnum", config_data)
+        with self.assertRaises(PortAssignmentRequired):
+            _tub_portlocation(config, None, None)
+    test_listen_on_zero_with_host.todo = native_str(
+        "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3563"
+    )
+
     def test_parsing_tcp(self):
         """
-        parse explicit tub.port with explicitly-default tub.location
+        When ``tub.port`` is given and ``tub.location`` is **AUTO** the port
+        number from ``tub.port`` is used as the port number for the value
+        constructed for ``tub.location``.
         """
-        get_addr = mock.patch(
-            "allmydata.util.iputil.get_local_addresses_sync",
-            return_value=["LOCAL"],
-        )
-        alloc_port = mock.patch(
-            "allmydata.util.iputil.allocate_tcp_port",
-            return_value=999,
-        )
         config_data = (
             "[node]\n"
             "tub.port = tcp:777\n"
@@ -539,8 +581,11 @@ class TestMissingPorts(unittest.TestCase):
         )
         config = config_from_string(self.basedir, "portnum", config_data)
 
-        with get_addr, alloc_port:
+        tubport, tublocation = _tub_portlocation(
-            tubport, tublocation = _tub_portlocation(config)
+            config,
+            _stub_get_local_addresses_sync,
+            _stub_allocate_tcp_port,
+        )
         self.assertEqual(tubport, "tcp:777")
         self.assertEqual(tublocation, b"tcp:LOCAL:777")
 
@@ -548,21 +593,16 @@ class TestMissingPorts(unittest.TestCase):
         """
         parse empty config, check defaults
         """
-        get_addr = mock.patch(
-            "allmydata.util.iputil.get_local_addresses_sync",
-            return_value=["LOCAL"],
-        )
-        alloc_port = mock.patch(
-            "allmydata.util.iputil.allocate_tcp_port",
-            return_value=999,
-        )
         config_data = (
             "[node]\n"
         )
         config = config_from_string(self.basedir, "portnum", config_data)
 
-        with get_addr, alloc_port:
+        tubport, tublocation = _tub_portlocation(
-            tubport, tublocation = _tub_portlocation(config)
+            config,
+            _stub_get_local_addresses_sync,
+            _stub_allocate_tcp_port,
+        )
         self.assertEqual(tubport, "tcp:999")
         self.assertEqual(tublocation, b"tcp:LOCAL:999")
 
@@ -570,22 +610,17 @@ class TestMissingPorts(unittest.TestCase):
         """
         location with two options (including defaults)
         """
-        get_addr = mock.patch(
-            "allmydata.util.iputil.get_local_addresses_sync",
-            return_value=["LOCAL"],
-        )
-        alloc_port = mock.patch(
-            "allmydata.util.iputil.allocate_tcp_port",
-            return_value=999,
-        )
         config_data = (
             "[node]\n"
             "tub.location = tcp:HOST:888,AUTO\n"
         )
         config = config_from_string(self.basedir, "portnum", config_data)
 
-        with get_addr, alloc_port:
+        tubport, tublocation = _tub_portlocation(
-            tubport, tublocation = _tub_portlocation(config)
+            config,
+            _stub_get_local_addresses_sync,
+            _stub_allocate_tcp_port,
+        )
         self.assertEqual(tubport, "tcp:999")
         self.assertEqual(tublocation, b"tcp:HOST:888,tcp:LOCAL:999")
 
@@ -593,14 +628,6 @@ class TestMissingPorts(unittest.TestCase):
         """
         parse config with both port + location disabled
         """
-        get_addr = mock.patch(
-            "allmydata.util.iputil.get_local_addresses_sync",
-            return_value=["LOCAL"],
-        )
-        alloc_port = mock.patch(
-            "allmydata.util.iputil.allocate_tcp_port",
-            return_value=999,
-        )
         config_data = (
             "[node]\n"
             "tub.port = disabled\n"
@@ -608,8 +635,11 @@ class TestMissingPorts(unittest.TestCase):
         )
         config = config_from_string(self.basedir, "portnum", config_data)
 
-        with get_addr, alloc_port:
+        res = _tub_portlocation(
-            res = _tub_portlocation(config)
+            config,
+            _stub_get_local_addresses_sync,
+            _stub_allocate_tcp_port,
+        )
         self.assertTrue(res is None)
 
     def test_empty_tub_port(self):
@@ -623,7 +653,11 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config)
+            _tub_portlocation(
+                config,
+                _stub_get_local_addresses_sync,
+                _stub_allocate_tcp_port,
+            )
         self.assertIn(
             "tub.port must not be empty",
             str(ctx.exception)
@@ -640,7 +674,11 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config)
+            _tub_portlocation(
+                config,
+                _stub_get_local_addresses_sync,
+                _stub_allocate_tcp_port,
+            )
         self.assertIn(
             "tub.location must not be empty",
             str(ctx.exception)
@@ -658,7 +696,11 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config)
+            _tub_portlocation(
+                config,
+                _stub_get_local_addresses_sync,
+                _stub_allocate_tcp_port,
+            )
         self.assertIn(
             "tub.port is disabled, but not tub.location",
             str(ctx.exception)
@@ -676,12 +718,62 @@ class TestMissingPorts(unittest.TestCase):
         config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
-            _tub_portlocation(config)
+            _tub_portlocation(
+                config,
+                _stub_get_local_addresses_sync,
+                _stub_allocate_tcp_port,
+            )
         self.assertIn(
             "tub.location is disabled, but not tub.port",
             str(ctx.exception)
         )
 
+    def test_tub_location_tcp(self):
+        """
+        If ``reveal-IP-address`` is set to false and ``tub.location`` includes a
+        **tcp** hint then ``_tub_portlocation`` raises `PrivacyError`` because
+        TCP leaks IP addresses.
+        """
+        config = config_from_string(
+            "fake.port",
+            "no-basedir",
+            "[node]\nreveal-IP-address = false\ntub.location=tcp:hostname:1234\n",
+        )
+        with self.assertRaises(PrivacyError) as ctx:
+            _tub_portlocation(
+                config,
+                _stub_get_local_addresses_sync,
+                _stub_allocate_tcp_port,
+            )
+        self.assertEqual(
+            str(ctx.exception),
+            "tub.location includes tcp: hint",
+        )
+
+    def test_tub_location_legacy_tcp(self):
+        """
+        If ``reveal-IP-address`` is set to false and ``tub.location`` includes a
+        "legacy" hint with no explicit type (which means it is a **tcp** hint)
+        then the behavior is the same as for an explicit **tcp** hint.
+        """
+        config = config_from_string(
+            "fake.port",
+            "no-basedir",
+            "[node]\nreveal-IP-address = false\ntub.location=hostname:1234\n",
+        )
+
+        with self.assertRaises(PrivacyError) as ctx:
+            _tub_portlocation(
+                config,
+                _stub_get_local_addresses_sync,
+                _stub_allocate_tcp_port,
+            )
+
+        self.assertEqual(
+            str(ctx.exception),
+            "tub.location includes tcp: hint",
+        )
+
 
 BASE_CONFIG = """
 [tor]
@@ -725,33 +817,6 @@ class FakeTub(object):
 
 class Listeners(unittest.TestCase):
 
-    def test_listen_on_zero(self):
-        """
-        Trying to listen on port 0 should be an error
-        """
-        basedir = self.mktemp()
-        create_node_dir(basedir, "testing")
-        with open(os.path.join(basedir, "tahoe.cfg"), "w") as f:
-            f.write(BASE_CONFIG)
-            f.write("[node]\n")
-            f.write("tub.port = tcp:0\n")
-            f.write("tub.location = AUTO\n")
-
-        config = client.read_config(basedir, "client.port")
-        i2p_provider = mock.Mock()
-        tor_provider = mock.Mock()
-        dfh, fch = create_connection_handlers(None, config, i2p_provider, tor_provider)
-        tub_options = create_tub_options(config)
-        t = FakeTub()
-
-        with mock.patch("allmydata.node.Tub", return_value=t):
-            with self.assertRaises(ValueError) as ctx:
-                create_main_tub(config, tub_options, dfh, fch, i2p_provider, tor_provider)
-        self.assertIn(
-            "you must choose",
-            str(ctx.exception),
-        )
-
     # Randomly allocate a couple distinct port numbers to try out.  The test
     # never actually binds these port numbers so we don't care if they're "in
     # use" on the system or not.  We just want a couple distinct values we can
@@ -763,62 +828,39 @@ class Listeners(unittest.TestCase):
         ``tub.location`` configuration, the node's *main* port listens on all
         of them.
         """
-        basedir = self.mktemp()
-        config_fname = os.path.join(basedir, "tahoe.cfg")
-        os.mkdir(basedir)
-        os.mkdir(os.path.join(basedir, "private"))
         port1, port2 = iter(ports)
         port = ("tcp:%d:interface=127.0.0.1,tcp:%d:interface=127.0.0.1" %
                 (port1, port2))
         location = "tcp:localhost:%d,tcp:localhost:%d" % (port1, port2)
-        with open(config_fname, "w") as f:
-            f.write(BASE_CONFIG)
-            f.write("[node]\n")
-            f.write("tub.port = %s\n" % port)
-            f.write("tub.location = %s\n" % location)
-
-        config = client.read_config(basedir, "client.port")
-        i2p_provider = mock.Mock()
-        tor_provider = mock.Mock()
-        dfh, fch = create_connection_handlers(None, config, i2p_provider, tor_provider)
-        tub_options = create_tub_options(config)
         t = FakeTub()
-
+        tub_listen_on(None, None, t, port, location)
-        with mock.patch("allmydata.node.Tub", return_value=t):
-            create_main_tub(config, tub_options, dfh, fch, i2p_provider, tor_provider)
         self.assertEqual(t.listening_ports,
                          ["tcp:%d:interface=127.0.0.1" % port1,
                           "tcp:%d:interface=127.0.0.1" % port2])
 
     def test_tor_i2p_listeners(self):
-        basedir = self.mktemp()
+        """
-        config_fname = os.path.join(basedir, "tahoe.cfg")
+        When configured to listen on an "i2p" or "tor" address, ``tub_listen_on``
-        os.mkdir(basedir)
+        tells the Tub to listen on endpoints supplied by the given Tor and I2P
-        os.mkdir(os.path.join(basedir, "private"))
+        providers.
-        with open(config_fname, "w") as f:
+        """
-            f.write(BASE_CONFIG)
-            f.write("[node]\n")
-            f.write("tub.port = listen:i2p,listen:tor\n")
-            f.write("tub.location = tcp:example.org:1234\n")
-        config = client.read_config(basedir, "client.port")
-        tub_options = create_tub_options(config)
         t = FakeTub()
 
-        i2p_provider = mock.Mock()
+        i2p_listener = object()
-        tor_provider = mock.Mock()
+        i2p_provider = ConstantAddresses(i2p_listener)
-        dfh, fch = create_connection_handlers(None, config, i2p_provider, tor_provider)
+        tor_listener = object()
+        tor_provider = ConstantAddresses(tor_listener)
 
-        with mock.patch("allmydata.node.Tub", return_value=t):
+        tub_listen_on(
-            create_main_tub(config, tub_options, dfh, fch, i2p_provider, tor_provider)
+            i2p_provider,
-
+            tor_provider,
-        self.assertEqual(i2p_provider.get_listener.mock_calls, [mock.call()])
+            t,
-        self.assertEqual(tor_provider.get_listener.mock_calls, [mock.call()])
+            "listen:i2p,listen:tor",
+            "tcp:example.org:1234",
+        )
         self.assertEqual(
             t.listening_ports,
-            [
+            [i2p_listener, tor_listener],
-                i2p_provider.get_listener(),
-                tor_provider.get_listener(),
-            ]
         )
 
 
@@ -926,19 +968,9 @@ class Configuration(unittest.TestCase):
 
 
 
-class FakeProvider(object):
+class CreateDefaultConnectionHandlersTests(unittest.TestCase):
-    """Emulate Tor and I2P providers."""
-
-    def get_tor_handler(self):
-        return "TORHANDLER!"
-
-    def get_i2p_handler(self):
-        return "I2PHANDLER!"
-
-
-class CreateConnectionHandlers(unittest.TestCase):
     """
-    Tests for create_connection_handlers().
+    Tests for create_default_connection_handlers().
     """
 
     def test_tcp_disabled(self):
@@ -949,9 +981,8 @@ class CreateConnectionHandlers(unittest.TestCase):
         [connections]
         tcp = disabled
         """))
-        reactor = object()  # it's not actually used?!
+        default_handlers = create_default_connection_handlers(
-        provider = FakeProvider()
+            config,
-        default_handlers, _ = create_connection_handlers(
+            {},
-            reactor, config, provider, provider
         )
         self.assertIs(default_handlers["tcp"], None)

src/allmydata/test/web/common.py

@@ -25,7 +25,8 @@ def assert_soup_has_tag_with_attributes(testcase, soup, tag_name, attrs):
     tags = soup.find_all(tag_name)
     for tag in tags:
         if all(v in tag.attrs.get(k, []) for k, v in attrs.items()):
-            return  # we found every attr in this tag; done
+            # we found every attr in this tag; done
+            return tag
     testcase.fail(
         u"No <{}> tags contain attributes: {}".format(tag_name, attrs)
     )

src/allmydata/test/web/test_root.py

@@ -1,7 +1,13 @@
-from mock import Mock
-
 import time
 
+from urllib import (
+    quote,
+)
+
+from bs4 import (
+    BeautifulSoup,
+)
+
 from twisted.trial import unittest
 from twisted.web.template import Tag
 from twisted.web.test.requesthelper import DummyRequest
@@ -16,6 +22,9 @@ from ...util.connection_status import ConnectionStatus
 from allmydata.web.root import URIHandler
 from allmydata.client import _Client
 
+from .common import (
+    assert_soup_has_tag_with_attributes,
+)
 from ..common_web import (
     render,
 )
@@ -30,28 +39,37 @@ class RenderSlashUri(unittest.TestCase):
     """
 
     def setUp(self):
-        self.client = Mock()
+        self.client = object()
         self.res = URIHandler(self.client)
 
-    def test_valid(self):
+    def test_valid_query_redirect(self):
         """
-        A valid capbility does not result in error
+        A syntactically valid capability given in the ``uri`` query argument
+        results in a redirect.
         """
-        query_args = {b"uri": [
+        cap = (
             b"URI:CHK:nt2xxmrccp7sursd6yh2thhcky:"
             b"mukesarwdjxiyqsjinbfiiro6q7kgmmekocxfjcngh23oxwyxtzq:2:5:5874882"
-        ]}
+        )
+        query_args = {b"uri": [cap]}
         response_body = self.successResultOf(
             render(self.res, query_args),
         )
-        self.assertNotEqual(
+        soup = BeautifulSoup(response_body, 'html5lib')
-            response_body,
+        tag = assert_soup_has_tag_with_attributes(
-            "Invalid capability",
+            self,
+            soup,
+            u"meta",
+            {u"http-equiv": "refresh"},
+        )
+        self.assertIn(
+            quote(cap, safe=""),
+            tag.attrs.get(u"content"),
         )
 
     def test_invalid(self):
         """
-        A (trivially) invalid capbility is an error
+        A syntactically invalid capbility results in an error.
         """
         query_args = {b"uri": [b"not a capability"]}
         response_body = self.successResultOf(

src/allmydata/util/_python3.py

@@ -90,6 +90,7 @@ PORTED_MODULES = [
     "allmydata.util.connection_status",
     "allmydata.util.deferredutil",
     "allmydata.util.dictutil",
+    "allmydata.util.eliotutil",
     "allmydata.util.encodingutil",
     "allmydata.util.fileutil",
     "allmydata.util.gcutil",
@@ -141,6 +142,7 @@ PORTED_TEST_MODULES = [
     "allmydata.test.test_dictutil",
     "allmydata.test.test_dirnode",
     "allmydata.test.test_download",
+    "allmydata.test.test_eliotutil",
     "allmydata.test.test_encode",
     "allmydata.test.test_encodingutil",
     "allmydata.test.test_filenode",

src/allmydata/util/eliotutil.py

@@ -1,6 +1,12 @@
 """
 Tools aimed at the interaction between Tahoe-LAFS implementation and Eliot.
+
+Ported to Python 3.
 """
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
 
 from __future__ import (
     unicode_literals,
@@ -18,6 +24,11 @@ __all__ = [
     "validateSetMembership",
 ]
 
+from future.utils import PY2
+if PY2:
+    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
+from six import ensure_text
+
 from sys import (
     stdout,
 )
@@ -228,7 +239,7 @@ def _stdlib_logging_to_eliot_configuration(stdlib_logger, eliot_logger=None):
 
 class _DestinationParser(object):
     def parse(self, description):
-        description = description.decode(u"ascii")
+        description = ensure_text(description)
 
         try:
             kind, args = description.split(u":", 1)

src/allmydata/util/i2p_provider.py

@@ -2,11 +2,18 @@
 from __future__ import absolute_import, print_function, with_statement
 import os
 
+from zope.interface import (
+    implementer,
+)
+
 from twisted.internet.defer import inlineCallbacks, returnValue
 from twisted.internet.endpoints import clientFromString
 from twisted.internet.error import ConnectionRefusedError, ConnectError
 from twisted.application import service
 
+from ..interfaces import (
+    IAddressFamily,
+)
 
 def create(reactor, config):
     """
@@ -135,6 +142,7 @@ def create_config(reactor, cli_config):
     returnValue((tahoe_config_i2p, i2p_port, i2p_location))
 
 
+@implementer(IAddressFamily)
 class _Provider(service.MultiService):
     def __init__(self, config, reactor):
         service.MultiService.__init__(self)
@@ -160,7 +168,14 @@ class _Provider(service.MultiService):
                    (privkeyfile, external_port, escaped_sam_port)
         return i2p_port
 
-    def get_i2p_handler(self):
+    def get_client_endpoint(self):
+        """
+        Get an ``IStreamClientEndpoint`` which will set up a connection to an I2P
+        address.
+
+        If I2P is not enabled or the dependencies are not available, return
+        ``None`` instead.
+        """
         enabled = self._get_i2p_config("enabled", True, boolean=True)
         if not enabled:
             return None
@@ -188,6 +203,9 @@ class _Provider(service.MultiService):
 
         return self._i2p.default(self._reactor, keyfile=keyfile)
 
+    # Backwards compatibility alias
+    get_i2p_handler = get_client_endpoint
+
     def check_dest_config(self):
         if self._get_i2p_config("dest", False, boolean=True):
             if not self._txi2p:

src/allmydata/util/tor_provider.py

@@ -2,6 +2,10 @@
 from __future__ import absolute_import, print_function, with_statement
 import os
 
+from zope.interface import (
+    implementer,
+)
+
 from twisted.internet.defer import inlineCallbacks, returnValue
 from twisted.internet.endpoints import clientFromString, TCP4ServerEndpoint
 from twisted.internet.error import ConnectionRefusedError, ConnectError
@@ -9,7 +13,9 @@ from twisted.application import service
 
 from .observer import OneShotObserverList
 from .iputil import allocate_tcp_port
-
+from ..interfaces import (
+    IAddressFamily,
+)
 
 def create(reactor, config):
     """
@@ -209,6 +215,7 @@ def create_config(reactor, cli_config):
     returnValue((tahoe_config_tor, tor_port, tor_location))
 
 
+@implementer(IAddressFamily)
 class _Provider(service.MultiService):
     def __init__(self, config, reactor):
         service.MultiService.__init__(self)
@@ -228,7 +235,13 @@ class _Provider(service.MultiService):
         ep = TCP4ServerEndpoint(self._reactor, local_port, interface="127.0.0.1")
         return ep
 
-    def get_tor_handler(self):
+    def get_client_endpoint(self):
+        """
+        Get an ``IStreamClientEndpoint`` which will set up a connection using Tor.
+
+        If Tor is not enabled or the dependencies are not available, return
+        ``None`` instead.
+        """
         enabled = self._get_tor_config("enabled", True, boolean=True)
         if not enabled:
             return None
@@ -253,6 +266,9 @@ class _Provider(service.MultiService):
 
         return self._tor.default_socks()
 
+    # Backwards compatibility alias
+    get_tor_handler = get_client_endpoint
+
     @inlineCallbacks
     def _make_control_endpoint(self, reactor, update_status):
         # this will only be called when tahoe.cfg has "[tor] launch = true"