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

[tor-commits] [stem/master] Changing HiddenServicePort to be a three value tuple



commit 590d548cad596df44674d68099ba69a7b230a5e7
Author: Damian Johnson <atagar@xxxxxxxxxxxxxx>
Date:   Sat Dec 13 14:32:07 2014 -0800

    Changing HiddenServicePort to be a three value tuple
    
    Our Controller hidden service method's HiddenServicePort was a string. This is
    clunky because it left parsing that string to our caller. The string might be
    three different formats, anything from a simple port to two ports and an
    address.
    
    Our get_hidden_service_conf() now provides a list of three value tuples...
    
      "HiddenServicePort": [
        (port, target_address, target_port),
      ]
    
    This both normalizes and means no parsing necessary.
---
 stem/control.py                  |  110 ++++++++++++++++++++++++++------------
 test/integ/control/controller.py |   16 +++---
 2 files changed, 83 insertions(+), 43 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index cace3ad..a90dfcb 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -2106,8 +2106,8 @@ class Controller(BaseController):
         "/var/lib/tor/hidden_service_with_two_ports/": {
           "HiddenServiceAuthorizeClient": "stealth a, b",
           "HiddenServicePort": [
-            "8020 127.0.0.1:8020",  # the ports order is kept
-            "8021 127.0.0.1:8021"
+            (8020, "127.0.0.1", 8020),  # the ports order is kept
+            (8021, "127.0.0.1", 8021)
           ],
           "HiddenServiceVersion": "2"
         },
@@ -2150,7 +2150,25 @@ class Controller(BaseController):
         directory = v
         service_dir_map[directory] = {'HiddenServicePort': []}
       elif k == 'HiddenServicePort':
-        service_dir_map[directory]['HiddenServicePort'].append(v)
+        port = target_port = v
+        target_address = '127.0.0.1'
+
+        if not v.isdigit():
+          port, target = v.split()
+
+          if target.isdigit():
+            target_port = target
+          else:
+            target_address, target_port = target.split(':')
+
+        if not stem.util.connection.is_valid_port(port):
+          raise stem.ProtocolError('GETCONF provided an invalid HiddenServicePort port (%s): %s' % (port, content))
+        elif not stem.util.connection.is_valid_ipv4_address(target_address):
+          raise stem.ProtocolError('GETCONF provided an invalid HiddenServicePort target address (%s): %s' % (target_address, content))
+        elif not stem.util.connection.is_valid_port(target_port):
+          raise stem.ProtocolError('GETCONF provided an invalid HiddenServicePort target port (%s): %s' % (target_port, content))
+
+        service_dir_map[directory]['HiddenServicePort'].append((int(port), target_address, int(target_port)))
       else:
         service_dir_map[directory][k] = v
 
@@ -2162,6 +2180,22 @@ class Controller(BaseController):
     the same format as
     :func:`~stem.control.Controller.get_hidden_service_conf`.
 
+    For convenience the HiddenServicePort entries can be an integer, string, or
+    tuple. If an **int** then we treat it as just a port. If a **str** we pass
+    that directly as the HiddenServicePort. And finally, if a **tuple** then
+    it's expected to be the **(port, target_address, target_port)** as provided
+    by :func:`~stem.control.Controller.get_hidden_service_conf`.
+
+    This is to say the following three are equivalent...
+
+    ::
+
+      "HiddenServicePort": [
+        80,
+        '80 127.0.0.1:80',
+        (80, '127.0.0.1', 80),
+      ]
+
     .. versionadded:: 1.3.0
 
     :param dict conf: configuration dictionary
@@ -2183,14 +2217,22 @@ class Controller(BaseController):
 
       for k, v in conf[directory].items():
         if k == 'HiddenServicePort':
-          for port in v:
-            hidden_service_options.append(('HiddenServicePort', port))
+          for entry in v:
+            if isinstance(entry, int):
+              entry = '%s 127.0.0.1:%s' % (entry, entry)
+            elif isinstance(entry, str):
+              pass  # just pass along what the user gave us
+            elif isinstance(entry, tuple):
+              port, target_address, target_port = entry
+              entry = '%s %s:%s' % (port, target_address, target_port)
+
+            hidden_service_options.append(('HiddenServicePort', entry))
         else:
           hidden_service_options.append((k, str(v)))
 
     self.set_options(hidden_service_options)
 
-  def create_hidden_service(self, path, port, target = None):
+  def create_hidden_service(self, path, port, target_address = None, target_port = None):
     """
     Create a new hidden service. If the directory is already present, a
     new port is added.
@@ -2199,7 +2241,9 @@ class Controller(BaseController):
 
     :param str path: path for the hidden service's data directory
     :param int port: hidden service port
-    :param str target: optional ipaddr:port target e.g. '127.0.0.1:8080'
+    :param str target_address: address of the service, by default 127.0.0.1
+    :param int target_port: port of the service, by default this is the same as
+      **port**
 
     :returns: **True** if the hidden service is created, **False** if the
       hidden service port is already in use
@@ -2209,26 +2253,31 @@ class Controller(BaseController):
 
     if not stem.util.connection.is_valid_port(port):
       raise ValueError("%s isn't a valid port number" % port)
+    elif target_address and not stem.util.connection.is_valid_ipv4_address(target_address):
+      raise ValueError("%s isn't a valid IPv4 address" % target_address)
+    elif target_port is not None and not stem.util.connection.is_valid_port(target_port):
+      raise ValueError("%s isn't a valid port number" % target_port)
+
+    port = int(port)
+    target_address = target_address if target_address else '127.0.0.1'
+    target_port = port if target_port is None else int(target_port)
 
-    port, conf = str(port), self.get_hidden_service_conf()
+    conf = self.get_hidden_service_conf()
 
     if path in conf:
       ports = conf[path]['HiddenServicePort']
 
-      if target is None:
-        if port in ports or '%s 127.0.0.1:%s' % (port, port) in ports:
-          return False
-      elif '%s %s' % (port, target) in ports:
+      if (port, target_address, target_port) in ports:
         return False
     else:
       conf[path] = {'HiddenServicePort': []}
 
-    conf[path]['HiddenServicePort'].append('%s %s' % (port, target) if target else port)
+    conf[path]['HiddenServicePort'].append((port, target_address, target_port))
     self.set_hidden_service_conf(conf)
 
     return True
 
-  def remove_hidden_service(self, path, port = None, target = None):
+  def remove_hidden_service(self, path, port = None):
     """
     Discontinues a given hidden service.
 
@@ -2236,7 +2285,6 @@ class Controller(BaseController):
 
     :param str path: path for the hidden service's data directory
     :param int port: hidden service port
-    :param str target: optional ipaddr:port target e.g. '127.0.0.1:8080'
 
     :returns: **True** if the hidden service is discontinued, **False** if it
       wasn't running in the first place
@@ -2244,36 +2292,28 @@ class Controller(BaseController):
     :raises: :class:`stem.ControllerError` if the call fails
     """
 
-    if not stem.util.connection.is_valid_port(port):
+    if port and not stem.util.connection.is_valid_port(port):
       raise ValueError("%s isn't a valid port number" % port)
 
-    port, conf = str(port), self.get_hidden_service_conf()
+    port = int(port) if port else None
+    conf = self.get_hidden_service_conf()
 
     if path not in conf:
       return False
 
-    if port:
-      if not target:
-        longport = '%s 127.0.0.1:%s' % (port, port)
+    if not port:
+      del conf[path]
+    else:
+      to_remove = [entry for entry in conf[path]['HiddenServicePort'] if entry[0] == port]
 
-        if port in conf[path]['HiddenServicePort']:
-          conf[path]['HiddenServicePort'].remove(port)
-        elif longport in conf[path]['HiddenServicePort']:
-          conf[path]['HiddenServicePort'].remove(longport)
-        else:
-          return False  # wasn't configured to be a hidden service
-      else:
-        longport = '%s %s' % (port, target)
+      if not to_remove:
+        return False
 
-        if longport in conf[path]['HiddenServicePort']:
-          conf[path]['HiddenServicePort'].remove(longport)
-        else:
-          return False  # wasn't configured to be a hidden service
+      for entry in to_remove:
+        conf[path]['HiddenServicePort'].remove(entry)
 
       if not conf[path]['HiddenServicePort']:
-        del(conf[path])  # no ports left, drop it entirely
-    else:
-      del(conf[path])
+        del conf[path]  # no ports left
 
     self.set_hidden_service_conf(conf)
     return True
diff --git a/test/integ/control/controller.py b/test/integ/control/controller.py
index adaaf8d..386eb8e 100644
--- a/test/integ/control/controller.py
+++ b/test/integ/control/controller.py
@@ -482,17 +482,17 @@ class TestController(unittest.TestCase):
         initialconf = {
           'test_hidden_service1/': {
             'HiddenServicePort': [
-              '8020 127.0.0.1:8020',
-              '8021 127.0.0.1:8021',
+              (8020, '127.0.0.1', 8020),
+              (8021, '127.0.0.1', 8021),
             ],
             'HiddenServiceVersion': '2',
           },
           'test_hidden_service2/': {
             'HiddenServiceAuthorizeClient': 'stealth a, b',
             'HiddenServicePort': [
-              '8030 127.0.0.1:8030',
-              '8031 127.0.0.1:8031',
-              '8032 127.0.0.1:8032',
+              (8030, '127.0.0.1', 8030),
+              (8031, '127.0.0.1', 8031),
+              (8032, '127.0.0.1', 8032),
             ]
           },
           'test_hidden_service_empty/': {
@@ -506,13 +506,13 @@ class TestController(unittest.TestCase):
         # add already existing services, with/without explicit target
 
         self.assertFalse(controller.create_hidden_service('test_hidden_service1/', 8020))
-        self.assertFalse(controller.create_hidden_service('test_hidden_service1/', 8021, target = '127.0.0.1:8021'))
+        self.assertFalse(controller.create_hidden_service('test_hidden_service1/', 8021, target_port = 8021))
         self.assertDictEqual(initialconf, controller.get_hidden_service_conf())
 
         # add a new service, with/without explicit target
 
         self.assertTrue(controller.create_hidden_service('test_hidden_serviceX/', 8888))
-        self.assertTrue(controller.create_hidden_service('test_hidden_serviceX/', 8989, target = '127.0.0.1:8021'))
+        self.assertTrue(controller.create_hidden_service('test_hidden_serviceX/', 8989, target_port = 8021))
 
         conf = controller.get_hidden_service_conf()
         self.assertEqual(4, len(conf))
@@ -525,7 +525,7 @@ class TestController(unittest.TestCase):
 
         # remove a service completely, it should now be gone
 
-        controller.remove_hidden_service('test_hidden_serviceX/', 8989, target = '127.0.0.1:8021')
+        controller.remove_hidden_service('test_hidden_serviceX/', 8989)
         self.assertEqual(3, len(controller.get_hidden_service_conf()))
       finally:
         controller.set_hidden_service_conf({})  # drop hidden services created during the test



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