[tor-commits] [bridgedb/master] Fix #2557: Don't crash if a distributor gets disabled.

nickm at torproject.org nickm at torproject.org
Mon Mar 7 22:06:09 UTC 2011


commit c518422f708e5d059f3d77e882e1da04b38d30c1
Author: Christian Fromme <kaner at strace.org>
Date:   Sun Mar 6 14:19:27 2011 +0100

    Fix #2557: Don't crash if a distributor gets disabled.
---
 lib/bridgedb/Bridges.py |   16 +++++++++++++---
 lib/bridgedb/Bucket.py  |   12 ------------
 lib/bridgedb/Main.py    |    4 ++++
 lib/bridgedb/Storage.py |   17 +++++++++++------
 lib/bridgedb/Tests.py   |   11 ++++++-----
 5 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/lib/bridgedb/Bridges.py b/lib/bridgedb/Bridges.py
index a06cf26..67db9af 100644
--- a/lib/bridgedb/Bridges.py
+++ b/lib/bridgedb/Bridges.py
@@ -401,6 +401,7 @@ class BridgeSplitter(BridgeHolder):
         self.totalP = 0
         self.pValues = []
         self.rings = []
+        self.pseudoRings = []
         self.statsHolders = []
 
     def __len__(self):
@@ -423,6 +424,11 @@ class BridgeSplitter(BridgeHolder):
         self.rings.append(ringname)
         self.totalP += p
 
+    def addPseudoRing(self, ringname):
+        """Add a pseudo ring to the list of pseudo rings.
+        """
+        self.pseudoRings.append(bridgedb.Bucket.PSEUDO_DISTRI_PREFIX + ringname)
+
     def addTracker(self, t):
         """Adds a statistics tracker that gets told about every bridge we see.
         """
@@ -451,11 +457,15 @@ class BridgeSplitter(BridgeHolder):
         assert 0 <= pos < len(self.rings)
         ringname = self.rings[pos]
 
-        ringname = db.insertBridgeAndGetRing(bridge, ringname, time.time())
+        validRings = self.rings + self.pseudoRings
+
+        ringname = db.insertBridgeAndGetRing(bridge, ringname, time.time(), 
+                                             validRings)
         db.commit()
 
-        # Resolve pseudo distributor ring names
-        ringname = bridgedb.Bucket.getRealDistributorName(ringname)
+        # Pseudo distributors are always held in the "unallocated" ring
+        if ringname in self.pseudoRings:
+            ringname = "unallocated"
 
         ring = self.ringsByName.get(ringname)
         ring.insert(bridge)
diff --git a/lib/bridgedb/Bucket.py b/lib/bridgedb/Bucket.py
index 39edaea..1d5d753 100644
--- a/lib/bridgedb/Bucket.py
+++ b/lib/bridgedb/Bucket.py
@@ -28,18 +28,6 @@ import bridgedb.Storage
 # distinguish them from real distributors?
 PSEUDO_DISTRI_PREFIX = "pseudo_"
 
-def getRealDistributorName(distributor):
-    """Return the *real* ring name for a given one. This is needed because
-       with pseudo distributors, we've got strings in the database that aren't
-       real distributors. 
-    """
-
-    # If it starts with "pseudo_", its really "unallocated"
-    if distributor.startswith(PSEUDO_DISTRI_PREFIX):
-        distributor = "unallocated"
-
-    return distributor
-
 class BucketData:
     """A file bucket value class.
        name      - Name of the bucket (From config)
diff --git a/lib/bridgedb/Main.py b/lib/bridgedb/Main.py
index 5959a41..82181b8 100644
--- a/lib/bridgedb/Main.py
+++ b/lib/bridgedb/Main.py
@@ -276,6 +276,10 @@ def startup(cfg):
                          "unallocated",
                          cfg.RESERVED_SHARE)
 
+    # Add pseudo distributors to splitter
+    for p in cfg.FILE_BUCKETS.keys():
+        splitter.addPseudoRing(p)
+
     # Make the parse-bridges function get re-called on SIGHUP.
     def reload():
         logging.info("Caught SIGHUP")
diff --git a/lib/bridgedb/Storage.py b/lib/bridgedb/Storage.py
index 2c64e83..ae48dde 100644
--- a/lib/bridgedb/Storage.py
+++ b/lib/bridgedb/Storage.py
@@ -176,9 +176,10 @@ class Database:
         self._cur.close()
         self._conn.close()
 
-    def insertBridgeAndGetRing(self, bridge, setRing, seenAt):
+    def insertBridgeAndGetRing(self, bridge, setRing, seenAt, validRings,
+                               defaultPool="unallocated"):
         '''updates info about bridge, setting ring to setRing if none was set.
-           Returns the bridge's ring.
+           Returns the bridge's validated ring.
         '''
         cur = self._cur
 
@@ -190,11 +191,15 @@ class Database:
                     "FROM Bridges WHERE hex_key = ?", (h,))
         v = cur.fetchone()
         if v is not None:
-            idx, ring = v
-            # Update last_seen and address.
+            i, ring = v
+            # Check if this is currently a valid ring name. If not, move back
+            # into default pool.
+            if ring not in validRings:
+                ring = defaultPool
+            # Update last_seen, address, port and (possibly) distributor.
             cur.execute("UPDATE Bridges SET address = ?, or_port = ?, "
-                        "last_seen = ? WHERE id = ?",
-                        (bridge.ip, bridge.orport, timeToStr(seenAt), idx))
+                        "distributor = ?, last_seen = ? WHERE id = ?",
+                        (bridge.ip, bridge.orport, ring, timeToStr(seenAt), i))
             return ring
         else:
             # Insert it.
diff --git a/lib/bridgedb/Tests.py b/lib/bridgedb/Tests.py
index f5198c4..1353658 100644
--- a/lib/bridgedb/Tests.py
+++ b/lib/bridgedb/Tests.py
@@ -156,10 +156,11 @@ class SQLStorageTests(unittest.TestCase):
         b1_v2 = B("serv1", "1.2.3.5", 9099, fingerprint=k1)
         b2 = B("serv2", "2.3.4.5", 9990, fingerprint=k2)
         b3 = B("serv3", "2.3.4.6", 9008, fingerprint=k3)
+        validRings = ["ring1", "ring2", "ring3"]
 
-        r = db.insertBridgeAndGetRing(b1, "ring1", t)
+        r = db.insertBridgeAndGetRing(b1, "ring1", t, validRings)
         self.assertEquals(r, "ring1")
-        r = db.insertBridgeAndGetRing(b1, "ring10", t+500)
+        r = db.insertBridgeAndGetRing(b1, "ring10", t+500, validRings)
         self.assertEquals(r, "ring1")
 
         cur.execute("SELECT distributor, address, or_port, first_seen, "
@@ -170,7 +171,7 @@ class SQLStorageTests(unittest.TestCase):
                            bridgedb.Storage.timeToStr(t),
                            bridgedb.Storage.timeToStr(t+500)))
 
-        r = db.insertBridgeAndGetRing(b1_v2, "ring99", t+800)
+        r = db.insertBridgeAndGetRing(b1_v2, "ring99", t+800, validRings)
         self.assertEquals(r, "ring1")
         cur.execute("SELECT distributor, address, or_port, first_seen, "
                     "last_seen FROM Bridges WHERE hex_key = ?", (k1,))
@@ -180,8 +181,8 @@ class SQLStorageTests(unittest.TestCase):
                            bridgedb.Storage.timeToStr(t),
                            bridgedb.Storage.timeToStr(t+800)))
 
-        db.insertBridgeAndGetRing(b2, "ring2", t)
-        db.insertBridgeAndGetRing(b3, "ring3", t)
+        db.insertBridgeAndGetRing(b2, "ring2", t, validRings)
+        db.insertBridgeAndGetRing(b3, "ring3", t, validRings)
 
         cur.execute("SELECT COUNT(distributor) FROM Bridges")
         v = cur.fetchone()



More information about the tor-commits mailing list