unit-test and fix for deleted routers

test/test_torcontrolprotocol.py

@@ -828,7 +828,7 @@ def test_addevent(self):
                 'SOMETHING_INVALID', lambda _: None
             )
             self.assertTrue(False)
-        except:
+        except Exception:
             pass
 
     def test_eventlistener(self):

test/test_torstate.py

@@ -1063,6 +1063,79 @@ def test_double_newconsensus(self):
         self.assertTrue('Unnamed' in self.state.routers)
         self.assertTrue('$00126582E505CF596F412D23ABC9E14DD4625C49' in self.state.routers)
 
+    def test_newconsensus_remove_routers(self):
+        """
+        router removed from consensus is removed
+        """
+
+        # bootstrap the TorState so we can send it a "real" 650
+        # update
+
+        self.protocol._set_valid_events(' '.join(self.state.event_map.keys()))
+        self.state._bootstrap()
+
+        self.send(b"250+ns/all=")
+        self.send(b".")
+        self.send(b"250 OK")
+
+        self.send(b"250+circuit-status=")
+        self.send(b".")
+        self.send(b"250 OK")
+
+        self.send(b"250-stream-status=")
+        self.send(b"250 OK")
+
+        self.send(b"250-address-mappings/all=")
+        self.send(b'250 OK')
+
+        for ignored in self.state.event_map.items():
+            self.send(b"250 OK")
+
+        self.send(b"250-entry-guards=")
+        self.send(b"250 OK")
+
+        self.send(b"250 OK")
+
+        # state is now bootstrapped, we can send our NEWCONSENSUS update
+
+        self.protocol.dataReceived(b'\r\n'.join(b'''650+NEWCONSENSUS
+r Unnamed ABJlguUFz1lvQS0jq8nhTdRiXEk /zIVUg1tKMUeyUBoyimzorbQN9E 2012-05-23 01:10:22 219.94.255.254 9001 0
+s Fast Guard Running Stable Valid
+w Bandwidth=166
+p reject 1-65535
+r Foo ABJJJJUFz1lvQS0jq8nhTdRiXEk /zzzUg1tKMUeyUBoyimzorbQN9E 2012-05-23 01:10:22 219.94.255.254 9001 0
+s Fast Guard Running Stable Valid
+w Bandwidth=166
+p reject 1-65535
+.
+650 OK
+'''.split(b'\n')))
+
+        self.assertEqual(2, len(self.state.all_routers))
+        self.assertTrue('Unnamed' in self.state.routers)
+        self.assertTrue('Foo' in self.state.routers_by_name)
+        self.assertTrue('$00126582E505CF596F412D23ABC9E14DD4625C49' in self.state.routers)
+        self.assertTrue('$001249249505CF596F412D23ABC9E14DD4625C49' in self.state.routers)
+
+        # this is a different fingerprint, but same name
+        self.protocol.dataReceived(b'\r\n'.join(b'''650+NEWCONSENSUS
+r Unnamed ABBBguUFz1lvQS0jq8nhTdRiXEk /zIVUg1tKMUeyUBoyimzorbQN9E 2012-05-23 01:10:22 219.94.255.254 9001 0
+s Fast Guard Running Stable Valid
+w Bandwidth=166
+p reject 1-65535
+.
+650 OK
+'''.split(b'\n')))
+
+        self.assertEqual(1, len(self.state.all_routers))
+        self.assertIn('Unnamed', self.state.routers)
+        self.assertIn('$00104182E505CF596F412D23ABC9E14DD4625C49', self.state.routers)
+        self.assertNotIn('$00126582E505CF596F412D23ABC9E14DD4625C49', self.state.routers)
+        self.assertNotIn('$00126582E505CF596F412D23ABC9E14DD4625C49', self.state.routers_by_hash)
+        self.assertNotIn('$001249249505CF596F412D23ABC9E14DD4625C49', self.state.routers)
+        self.assertNotIn('$001249249505CF596F412D23ABC9E14DD4625C49', self.state.routers_by_hash)
+        self.assertNotIn('Foo', self.state.routers_by_name)
+
     def test_NEWCONSENSUS_ends_with_OK_on_w(self):
         """
         The arrival of a second NEWCONSENSUS event causes parsing

txtorcon/torcontrolprotocol.py

@@ -493,7 +493,7 @@ def add_event_listener(self, evt, callback):
         if evt not in self.valid_events.values():
             try:
                 evt = self.valid_events[evt]
-            except:
+            except KeyError:
                 raise RuntimeError("Unknown event type: " + evt)
 
         if evt.name not in self.events:
@@ -514,7 +514,7 @@ def remove_event_listener(self, evt, cb):
             # this lets us pass a string or a real event-object
             try:
                 evt = self.valid_events[evt]
-            except:
+            except KeyError:
                 raise RuntimeError("Unknown event type: " + evt)
 
         evt.unlisten(cb)
@@ -832,7 +832,7 @@ def _is_single_line_response(self, line):
         "for FSM"
         try:
             code = int(line[:3])
-        except:
+        except Exception:
             return False
 
         sl = len(line) > 3 and line[3] == ' '

txtorcon/torstate.py

@@ -144,7 +144,7 @@ def build_local_tor_connection(reactor, host='127.0.0.1', port=9051,
 
     try:
         return build_tor_connection((reactor, socket), *args, **kwargs)
-    except:
+    except Exception:
         return build_tor_connection((reactor, host, port), *args, **kwargs)
 
 
@@ -256,6 +256,7 @@ def __init__(self, protocol, bootstrap=True):
 
         #: keys by hexid (string) and by unique names
         self.routers = {}
+        self._old_routers = {}
 
         #: keys on name, value always list (many duplicate "Unnamed"
         #: routers, for example)
@@ -290,10 +291,11 @@ def __init__(self, protocol, bootstrap=True):
     def _create_router(self, **kw):
         id_hex = hexIdFromHash(kw['idhash'])
         try:
-            router = self.routers[id_hex]
+            router = self._old_routers[id_hex]
         except KeyError:
             router = Router(self.protocol)
-            self.routers[router.id_hex] = router
+
+        self.routers[id_hex] = router
         router.from_consensus = True
         router.update(
             kw['nickname'],
@@ -315,15 +317,11 @@ def _create_router(self, **kw):
         if 'authority' in router.flags:
             self.authorities[router.name] = router
 
-        if router.id_hex in self.routers:
-            # FIXME should I do an update() on this one??
-            router = self.routers[router.id_hex]
-        else:
-            if router.name in self.routers:
-                self.routers[router.name] = None
+        if router.name in self.routers:
+            self.routers[router.name] = None
 
-            else:
-                self.routers[router.name] = router
+        else:
+            self.routers[router.name] = router
 
         if router.name in self.routers_by_name:
             self.routers_by_name[router.name].append(router)
@@ -405,7 +403,7 @@ def _bootstrap(self, arg=None):
             try:
                 pid = parse_keywords(pid)['process/pid']
                 self.tor_pid = int(pid)
-            except:
+            except Exception:  # fixme: ValueError and KeyError ..?
                 self.tor_pid = 0
         if not self.tor_pid and self.protocol.is_owned:
             self.tor_pid = self.protocol.is_owned
@@ -718,9 +716,13 @@ def _update_network_status(self, data):
         from NEWCONSENSUS events.
         """
 
-        # XXX why are we getting this with 0 data?
+        # XXX why are we ever getting this with 0 data?
         if len(data):
+            self._old_routers = self.routers
+            self.routers = dict()
             self.all_routers = set()
+            self.routers_by_hash = dict()
+            self.routers_by_name = dict()
             for line in data.split('\n'):
                 self._network_status_parser.feed_line(line)
             self._network_status_parser.done()
@@ -732,7 +734,6 @@ def _update_network_status(self, data):
             if v is None:
                 txtorlog.msg(len(self.routers_by_name[k]), "dups:", k)
                 remove_keys.add(k)
-
         for k in remove_keys:
             del self.routers[k]
 

txtorcon/util.py

@@ -265,7 +265,7 @@ def __init__(self, ipaddr):
         if city:
             try:
                 r = city.record_by_addr(self.ip)
-            except:
+            except Exception:
                 r = None
             if r is not None:
                 self.countrycode = r['country_code']
@@ -284,7 +284,7 @@ def __init__(self, ipaddr):
         if asn:
             try:
                 self.asn = asn.org_by_addr(self.ip)
-            except:
+            except Exception:
                 self.asn = None