#33631: BridgeDB doesn't allow bridges to change their distribution mechanism ------------------------------------+------------------------------- Reporter: phw | Owner: phw Type: defect | Status: needs_review Priority: Medium | Milestone: Component: Circumvention/BridgeDB | Version: Severity: Normal | Resolution: Keywords: s30-o23a1 | Actual Points: Parent ID: #31280 | Points: 0.5 Reviewer: cohosh | Sponsor: Sponsor30-can ------------------------------------+------------------------------- Changes (by phw):
* status: needs_revision => needs_review Comment: Replying to [comment:4 cohosh]: > - Since we're ignoring the returned distributor from the SQL query [https://github.com/NullHypothesis/bridgedb/compare/develop...defect/33631?expand=1 #diff-f5e72cabd3c2e29288c80ecdbfc51cabL185 here], it makes more sense to modify the query to return only what we need: > > {{{cur.execute("SELECT id FROM Bridges WHERE hex_key = ?", (h,))}}} [[br]] Good catch! I fixed it [https://github.com/NullHypothesis/bridgedb/commit/164552a070c59c6a00066fe618aee12f3bf3df59 here]. [[br]] > - The logic for removing bridges by finding their index [https://github.com/NullHypothesis/bridgedb/compare/develop...defect/33631?expand=1 #diff-db025301d124ca0c9fe5cfeea36077c8R626 here] seems a bit unwieldy to me. I noticed another, similar function of the code [https://github.com/NullHypothesis/bridgedb/blob/21cf51c227d8e53ca70ff7e4831e8b770d6eeb9a/bridgedb/Bridges.py#L658 here] that does the following: > {{{ > > index = 0 > logging.debug("Inserting %s into hashring..." % bridge) > for old_bridge in self.bridges[:]: > if bridge.fingerprint == old_bridge.fingerprint: > self.bridges[index] = bridge > break > index += 1 > }}} > > Is there a better way to handle this? Or a way to be more consistent? Perhaps we could refactor this logic into its own function that takes a bridge and returns its index in `self.bridges`. Would making `self.bridges` a map help? [[br]] That's a good observation, also [https://github.com/NullHypothesis/bridgedb/commit/94b3e63188c9b2b5136700347c0db20ee48c1586 fixed]. [[br]] > - Do the tests also check that bridges were removed from all subrings? Can we do that easily? [[br]] Actually, no. Fixed [https://github.com/NullHypothesis/bridgedb/commit/ef71bf152cb7d1b45e1048f37af6b0d2ce1e6eca here]. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/33631#comment:5> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs