Dependency Injection for _tub_portlocation

This commit is contained in:
Jean-Paul Calderone 2020-12-09 15:48:40 -05:00
parent 46f18fbbc3
commit e2963856d3
3 changed files with 103 additions and 80 deletions

View File

@ -705,8 +705,15 @@ def _convert_tub_port(s):
return us
def _tub_portlocation(config):
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 +753,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:
@ -766,7 +773,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 = []
@ -821,7 +828,11 @@ 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
tub = create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,

View File

@ -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
@ -438,31 +438,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",
)

View File

@ -39,6 +39,7 @@ import foolscap.logging.log
from twisted.application import service
from allmydata.node import (
PrivacyError,
create_tub_options,
create_main_tub,
create_node_dir,
@ -513,25 +514,24 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
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 _get_addr(self):
return ["LOCAL"]
def _alloc_port(self):
return 999
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 +539,11 @@ class TestMissingPorts(unittest.TestCase):
)
config = config_from_string(self.basedir, "portnum", config_data)
with get_addr, alloc_port:
tubport, tublocation = _tub_portlocation(config)
tubport, tublocation = _tub_portlocation(
config,
self._get_addr,
self._alloc_port,
)
self.assertEqual(tubport, "tcp:777")
self.assertEqual(tublocation, b"tcp:LOCAL:777")
@ -548,21 +551,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(config)
tubport, tublocation = _tub_portlocation(
config,
self._get_addr,
self._alloc_port,
)
self.assertEqual(tubport, "tcp:999")
self.assertEqual(tublocation, b"tcp:LOCAL:999")
@ -570,22 +568,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(config)
tubport, tublocation = _tub_portlocation(
config,
self._get_addr,
self._alloc_port,
)
self.assertEqual(tubport, "tcp:999")
self.assertEqual(tublocation, b"tcp:HOST:888,tcp:LOCAL:999")
@ -593,14 +586,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 +593,11 @@ class TestMissingPorts(unittest.TestCase):
)
config = config_from_string(self.basedir, "portnum", config_data)
with get_addr, alloc_port:
res = _tub_portlocation(config)
res = _tub_portlocation(
config,
self._get_addr,
self._alloc_port,
)
self.assertTrue(res is None)
def test_empty_tub_port(self):
@ -623,7 +611,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,
self._get_addr,
self._alloc_port,
)
self.assertIn(
"tub.port must not be empty",
str(ctx.exception)
@ -640,7 +632,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,
self._get_addr,
self._alloc_port,
)
self.assertIn(
"tub.location must not be empty",
str(ctx.exception)
@ -658,7 +654,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,
self._get_addr,
self._alloc_port,
)
self.assertIn(
"tub.port is disabled, but not tub.location",
str(ctx.exception)
@ -676,12 +676,52 @@ 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,
self._get_addr,
self._alloc_port,
)
self.assertIn(
"tub.location is disabled, but not tub.port",
str(ctx.exception)
)
def test_tub_location_tcp(self):
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,
self._get_addr,
self._alloc_port,
)
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",
"[node]\nreveal-IP-address = false\ntub.location=hostname:1234\n",
)
with self.assertRaises(PrivacyError) as ctx:
_tub_portlocation(
config,
self._get_addr,
self._alloc_port,
)
self.assertEqual(
str(ctx.exception),
"tub.location includes tcp: hint",
)
BASE_CONFIG = """
[tor]