BridgeDB fails with a traceback when turning off a distributor

BridgeDB breaks with a traceback when one first runs it with a distributor turned on, then reconfigures it to turn that distributor off, and then runs it again. It doesn't know where to put the bridges from the database.

added component::circumvention/bridgedb owner::nickm priority::medium resolution::fixed status::closed type::defect labels

Proposed fix in c518422f708e5d059f3d77e882e1da04b38d30c1 in git://git.torproject.org/kaner/bridgedb.git Disabled distributors are re-assigned to "unallocated" now. Tested on my local system. Nick, please review and merge if it is ok.

Trac:
Status: new to needs_review

Sure, let's give it a shot. Merging.

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Patch looks good. Two minor comments:

• I'm not 100% certain about the new semantics of insertBridgeAndGetRing(). What's a "validated ring"? And should the comment mention that we're returning defaultPool if the new ring (or the ring in the database?) is not contained in validRings?

• I wonder why the changes in Tests.py work at all. If there are only ring1, ring2, and ring3 and we insert a bridge into ring10, how does it end up in ring1 and not in unallocated?

Trac:
Resolution: fixed to N/A
Status: closed to reopened

Thanks for reviewing. Please see commit 48527eaf4686a2c4f436b3e0e17d6c7fb7c79ba5 for a minor fix. (Merge, close if ok)

karsten, if you like the revision, feel free to merge it.

Replying to kaner:

Thanks for reviewing. Please see commit 48527eaf4686a2c4f436b3e0e17d6c7fb7c79ba5 for a minor fix. (Merge, close if ok)

The tests now fail for me:

FAIL: testBridgeStorage (bridgedb.Tests.SQLStorageTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "build/lib/bridgedb/Tests.py", line 164, in testBridgeStorage
    self.assertEquals(r, "unallocated")
AssertionError: u'ring1' != 'unallocated'

And that's what I don't understand. Your change to the tests in 48527ea looks correct to me. So, maybe there's something wrong in the implementation? Also note that there's another test right after adding a bridge to ring99 that needs to be changed.

Replying to karsten:

And that's what I don't understand. Your change to the tests in 48527ea looks correct to me. So, maybe there's something wrong in the implementation? Also note that there's another test right after adding a bridge to ring99 that needs to be changed.

Ok. Sorry. I should have checked your initial comment about "how does it end up in ring1 and not in unallocated" better. Of course the bridge ends up in ring1, because it initially gets assigned to ring1 by the first call to insertBridgeAndGetRing. Its the same bridge that goes in. Bridges aren't moved between rings. Only the time stamp gets updated the second time it goes in. So it was correct from the beginning on. Same goes for the bridge that wants to go into ring99 btw. Same key, same ring. All ring1.

Feel free to revert part of the last commit or simply use 971f8aee61a03eefde45ffd319eff646e6e8252e.

Okay, sorry for the confusion. I squashed your two commits and pushed them to master. Closing. Thanks!

Trac:
Status: reopened to closed
Resolution: N/A to fixed

closed

mentioned in issue #2558 (moved)

moved to tpo/anti-censorship/bridgedb#2557 (closed)

mentioned in issue tpo/anti-censorship/bridgedb#2558 (closed)