Repository: meejah/txtorcon
Issue: De-duplicate hidden-services by HiddenServiceDir

It turns out that Tor currently only "warns" if you have two hidden services that have the very same HiddenServiceDir.

So TorConfig should verify that we don't have two HiddenService instances pointing at the same directory, or tor will accept it with a warn, but one of them won't work at all.


Changed files:
txtorcon/torconfig.py, test/test_torconfig.py

diff --git a/test/test_torconfig.py b/test/test_torconfig.py
index a387d3e..4f676f2 100644
--- a/test/test_torconfig.py
+++ b/test/test_torconfig.py
@@ -775,6 +775,27 @@ HiddenServicePort=90 127.0.0.1:2345''')
         conf._setup_hidden_services('HiddenServiceDir=/fake/path/../path')
         self.assertEqual(len(self.flushWarnings()), 1)
 
+    def test_hidden_service_same_directory(self):
+        conf = TorConfig(FakeControlProtocol(['config/names=']))
+        servicelines = '''HiddenServiceDir=/fake/path
+HiddenServiceDir=/fake/path'''
+        self.assertRaises(RuntimeError, conf._setup_hidden_services, servicelines)
+
+        conf = TorConfig()
+        conf.HiddenServices = [HiddenService(conf, '/fake/path', ['80 127.0.0.1:1234'])]
+        conf.HiddenServices.append(HiddenService(conf, '/fake/path', ['80 127.0.0.1:1234']))
+        self.assertTrue(conf.needs_save())
+        self.assertRaises(RuntimeError, conf.save)
+
+        conf = TorConfig()
+        conf.HiddenServices = [HiddenService(conf, '/fake/path', ['80 127.0.0.1:1234'])]
+        conf.HiddenServices.append(HiddenService(conf, '/fake/path/two', ['80 127.0.0.1:1234']))
+        self.assertTrue(conf.needs_save())
+        conf.save()
+        conf.hiddenservices[1].dir = '/fake/path'
+        self.assertTrue(conf.needs_save())
+        self.assertRaises(RuntimeError, conf.save)
+
     def test_multiple_modify_hidden_service(self):
         self.protocol.answers.append('HiddenServiceDir=/fake/path\\nHiddenServicePort=80 127.0.0.1:1234\\n')
 
diff --git a/txtorcon/torconfig.py b/txtorcon/torconfig.py
index 7e9e5aa..519c0c8 100644
--- a/txtorcon/torconfig.py
+++ b/txtorcon/torconfig.py
@@ -1129,13 +1129,22 @@ class TorConfig(object):
             return defer.succeed(self)
 
         args = []
+        directories = []
         for (key, value) in self.unsaved.items():
             if key == 'HiddenServices':
                 self.config['HiddenServices'] = value
                 for hs in value:
                     for (k, v) in hs.config_attributes():
-                        args.append(k)
-                        args.append(v)
+                        if k == 'HiddenServiceDir':
+                            if v not in directories:
+                                directories.append(v)
+                                args.append(k)
+                                args.append(v)
+                            else:
+                                raise RuntimeError("Trying to add hidden service with same HiddenServiceDir: %s" % v)
+                        else:
+                            args.append(k)
+                            args.append(v)
                 continue
 
             if isinstance(value, types.ListType):
@@ -1225,10 +1234,24 @@ class TorConfig(object):
         defer.returnValue(self)
 
     def _setup_hidden_services(self, servicelines):
+        def maybe_add_hidden_service():
+            if directory is not None:
+                if directory not in directories:
+                    directories.append(directory)
+                    hs.append(
+                        HiddenService(
+                            self, directory, ports, auth, ver, group_read
+                        )
+                    )
+                else:
+                    raise RuntimeError("Trying to add hidden service with same HiddenServiceDir: %s" % directory)
+
         hs = []
         directory = None
+        directories = []
         ports = []
         ver = None
+        group_read = None
         auth = None
         for line in servicelines.split('\\n'):
             if not len(line.strip()):
@@ -1238,12 +1261,7 @@ class TorConfig(object):
                 continue
             k, v = line.split('=')
             if k == 'HiddenServiceDir':
-                if directory is not None:
-                    hs.append(
-                        HiddenService(
-                            self, directory, ports, auth, ver, group_read
-                        )
-                    )
+                maybe_add_hidden_service()
                 directory = v
                 _directory = directory
                 directory = os.path.abspath(directory)
@@ -1272,12 +1290,7 @@ class TorConfig(object):
             else:
                 raise RuntimeError("Can't parse HiddenServiceOptions: " + k)
 
-        if directory is not None:
-            hs.append(
-                HiddenService(
-                    self, directory, ports, auth, ver, group_read
-                )
-            )
+        maybe_add_hidden_service()
 
         name = 'HiddenServices'
         self.config[name] = _ListWrapper(