[tor-commits] [stem/master] Support mixed MAPADDRESS response
atagar at torproject.org
atagar at torproject.org
Sun Aug 16 00:17:57 UTC 2020
commit 2e66a4846e980ce70bca40bea968b89909130029
Author: Damian Johnson <atagar at torproject.org>
Date: Sat Aug 15 16:56:18 2020 -0700
Support mixed MAPADDRESS response
Recently tor documented that MAPADDRESS can contain a mixture of successful and
failed responses...
https://gitweb.torproject.org/torspec.git/commit/?id=1417af05de247d9f858863849d16a7185577d369
Such a response didn't break us, our map_address() method simply ignored the
failures and returned a dictionary with whatever mappings succeeded.
However, we should provide our caller with failures as well for completeness.
This breaks backward compatibility in a couple ways...
1. Our map_address() method now returns a MapAddressResponse rather
than a dictionary.
2. Renamed MapAddressResponse's "entries" attribute to "mappings".
Personally I'd prefer if MAPADDRESS was all-or-nothing (that is to say, if any
entry is malformed then reject the request). That would produce a simpler API
for our callers. But I don't control that so this is the method response we'll
need to supply.
---
stem/control.py | 7 ++++---
stem/response/mapaddress.py | 27 ++++++++++++++++++---------
test/integ/control/controller.py | 24 ++++++++++++++++++++----
test/unit/response/mapaddress.py | 6 +++---
4 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/stem/control.py b/stem/control.py
index 678afa8f..e1c68ce9 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -3870,7 +3870,7 @@ class Controller(BaseController):
return value
- async def map_address(self, mapping: Mapping[str, str]) -> Dict[str, str]:
+ async def map_address(self, mapping: Mapping[str, str]) -> 'stem.response.mapaddress.MapAddressResponse':
"""
Replace Tor connections for the given addresses with alternate
destionations. If the destination is **None** then its mapping will be
@@ -3878,7 +3878,8 @@ class Controller(BaseController):
:param mapping: mapping of original addresses to replacement addresses
- :returns: **dict** with 'original -> replacement' address mappings
+ :returns: :class:`~stem.response.mapaddress.MapAddressResponse` with the
+ address mappings and failrues
:raises:
* :class:`stem.InvalidRequest` if the addresses are malformed
@@ -3887,7 +3888,7 @@ class Controller(BaseController):
mapaddress_arg = ' '.join(['%s=%s' % (k, v if v else k) for (k, v) in list(mapping.items())])
response = await self.msg('MAPADDRESS %s' % mapaddress_arg)
- return stem.response._convert_to_mapaddress(response).entries
+ return stem.response._convert_to_mapaddress(response)
async def drop_guards(self) -> None:
"""
diff --git a/stem/response/mapaddress.py b/stem/response/mapaddress.py
index 92ce16d2..69877109 100644
--- a/stem/response/mapaddress.py
+++ b/stem/response/mapaddress.py
@@ -7,10 +7,20 @@ import stem.socket
class MapAddressResponse(stem.response.ControlMessage):
"""
- Reply for a MAPADDRESS query.
- Doesn't raise an exception unless no addresses were mapped successfully.
+ MAPADDRESS reply. Responses can contain a mixture of successes and failures,
+ such as...
- :var dict entries: mapping between the original and replacement addresses
+ ::
+
+ 512-syntax error: invalid address '@@@'
+ 250 1.2.3.4=tor.freehaven.net
+
+ This only raises an exception if **every** mapping fails. Otherwise
+ **mapped** enumerates our successes and **failures** lists any
+ failure messages.
+
+ :var dict mapped: mapping between the original and replacement addresses
+ :var list failures: failure listed within this reply
:raises:
* :class:`stem.OperationFailed` if Tor was unable to satisfy the request
@@ -18,10 +28,6 @@ class MapAddressResponse(stem.response.ControlMessage):
"""
def _parse_message(self) -> None:
- # Example:
- # 250-127.192.10.10=torproject.org
- # 250 1.2.3.4=tor.freehaven.net
-
if not self.is_ok():
for code, _, message in self.content():
if code == '512':
@@ -31,12 +37,15 @@ class MapAddressResponse(stem.response.ControlMessage):
else:
raise stem.ProtocolError('MAPADDRESS returned unexpected response code: %s', code)
- self.entries = {}
+ self.mapped = {}
+ self.failures = []
for code, _, message in self.content():
if code == '250':
try:
key, value = message.split('=', 1)
- self.entries[key] = value
+ self.mapped[key] = value
except ValueError:
raise stem.ProtocolError(None, "MAPADDRESS returned '%s', which isn't a mapping" % message)
+ else:
+ self.failures.append(message)
diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py
index 8fc0e099..9c077966 100644
--- a/test/integ/control/controller.py
+++ b/test/integ/control/controller.py
@@ -1210,14 +1210,14 @@ class TestController(unittest.TestCase):
# try mapping one element, ensuring results are as expected
map1 = {'1.2.1.2': 'ifconfig.me'}
- self.assertEqual(map1, await controller.map_address(map1))
+ self.assertEqual(map1, (await controller.map_address(map1)).mapped)
# try mapping two elements, ensuring results are as expected
map2 = {'1.2.3.4': 'foobar.example.com',
'1.2.3.5': 'barfuzz.example.com'}
- self.assertEqual(map2, await controller.map_address(map2))
+ self.assertEqual(map2, (await controller.map_address(map2)).mapped)
# try mapping zero elements
@@ -1229,7 +1229,7 @@ class TestController(unittest.TestCase):
map3 = {'0.0.0.0': 'quux'}
response = await controller.map_address(map3)
self.assertEquals(len(response), 1)
- addr1, target = list(response.items())[0]
+ addr1, target = list(response.mapped.items())[0]
self.assertTrue('%s did not start with 127.' % addr1, addr1.startswith('127.'))
self.assertEquals('quux', target)
@@ -1239,7 +1239,7 @@ class TestController(unittest.TestCase):
map4 = {'::': 'quibble'}
response = await controller.map_address(map4)
self.assertEquals(1, len(response))
- addr2, target = list(response.items())[0]
+ addr2, target = list(response.mapped.items())[0]
self.assertTrue(addr2.startswith('[fe'), '%s did not start with [fe.' % addr2)
self.assertEquals('quibble', target)
@@ -1285,6 +1285,22 @@ class TestController(unittest.TestCase):
await controller.map_address(dict([(addr, None) for addr in mapped_addresses]))
self.assertEquals({}, await address_mappings('control'))
+ @test.require.controller
+ @async_test
+ async def test_mapaddress_mixed_response(self):
+ runner = test.runner.get_runner()
+
+ async with await runner.get_tor_controller() as controller:
+ # mix a valid and invalid mapping
+
+ response = await controller.map_address({
+ '1.2.1.2': 'ifconfig.me',
+ 'foo': '@@@',
+ })
+
+ self.assertEqual({'1.2.1.2': 'ifconfig.me'}, response.mapped)
+ self.assertEqual(["syntax error: invalid address '@@@'"], response.failures)
+
@test.require.controller
@test.require.online
@async_test
diff --git a/test/unit/response/mapaddress.py b/test/unit/response/mapaddress.py
index 7482507d..177ce95c 100644
--- a/test/unit/response/mapaddress.py
+++ b/test/unit/response/mapaddress.py
@@ -35,7 +35,7 @@ class TestMapAddressResponse(unittest.TestCase):
"""
control_message = ControlMessage.from_str('250 foo=bar\r\n', 'MAPADDRESS')
- self.assertEqual({'foo': 'bar'}, control_message.entries)
+ self.assertEqual({'foo': 'bar'}, control_message.mapped)
def test_batch_response(self):
"""
@@ -50,7 +50,7 @@ class TestMapAddressResponse(unittest.TestCase):
}
control_message = ControlMessage.from_str(BATCH_RESPONSE, 'MAPADDRESS', normalize = True)
- self.assertEqual(expected, control_message.entries)
+ self.assertEqual(expected, control_message.mapped)
def test_invalid_requests(self):
"""
@@ -61,7 +61,7 @@ class TestMapAddressResponse(unittest.TestCase):
self.assertRaises(stem.InvalidRequest, stem.response.convert, 'MAPADDRESS', control_message)
control_message = ControlMessage.from_str(PARTIAL_FAILURE_RESPONSE, 'MAPADDRESS', normalize = True)
- self.assertEqual({'23': '324'}, control_message.entries)
+ self.assertEqual({'23': '324'}, control_message.mapped)
def test_invalid_response(self):
"""
More information about the tor-commits
mailing list