[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [stem/master] Support mixed MAPADDRESS response



commit 2e66a4846e980ce70bca40bea968b89909130029
Author: Damian Johnson <atagar@xxxxxxxxxxxxxx>
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):
     """

_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits