commit 2e66a4846e980ce70bca40bea968b89909130029 Author: Damian Johnson atagar@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=1417af05de247d9f8588638...
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): """