commit 9c34acd384a57f23f451142978b0fdbc4694dc72 Author: Isis Lovecruft <isis@torproject.org> Date: Mon May 11 01:04:42 2015 +0000 Change BridgeRequestBase.addressClass → BridgeRequestBase.ipVersion. Passing an integer around both requires less memory, and makes the IP version filters in bridgedb.filters run faster (since they no longer need to use isinstance()). * CHANGE bridgedb.bridges.Bridge to use ipVersion when working with BridgeRequestBase. --- lib/bridgedb/bridgerequest.py | 57 ++++++++++++++++++------------- lib/bridgedb/bridges.py | 12 +++---- lib/bridgedb/filters.py | 4 +-- lib/bridgedb/test/test_email_request.py | 16 ++++----- lib/bridgedb/test/util.py | 2 +- 5 files changed, 51 insertions(+), 40 deletions(-) diff --git a/lib/bridgedb/bridgerequest.py b/lib/bridgedb/bridgerequest.py index 2ded7b3..1266145 100644 --- a/lib/bridgedb/bridgerequest.py +++ b/lib/bridgedb/bridgerequest.py @@ -27,10 +27,10 @@ from bridgedb.filters import byTransport class IRequestBridges(Interface): """Interface specification of client options for requested bridges.""" - addressClass = Attribute( - "The IP version of bridge addresses to distribute to the client.") filters = Attribute( "A list of callables used to filter bridges from a hashring.") + ipVersion = Attribute( + "The IP version of bridge addresses to distribute to the client.") transports = Attribute( "A list of strings of Pluggable Transport types requested.") notBlockedIn = Attribute( @@ -54,7 +54,7 @@ class IRequestBridges(Interface): def generateFilters(): """Build the list of callables, ``filters``, according to the current contents of the lists of ``transports``, ``notBlockedIn``, and the - ``addressClass``. + ``ipVersion``. """ def getHashringPlacement(): @@ -68,10 +68,10 @@ class IRequestBridges(Interface): """Determine if the request is ``valid`` according to some parameters.""" def withIPv4(): - """Set the ``addressClass`` to IPv4.""" + """Set the ``ipVersion`` to IPv4.""" def withIPv6(): - """Set the ``addressClass`` to IPv6.""" + """Set the ``ipVersion`` to IPv6.""" def withPluggableTransportType(typeOfPT): """Add this **typeOfPT** to the list of requested ``transports``.""" @@ -87,13 +87,8 @@ class BridgeRequestBase(object): implements(IRequestBridges) - def __init__(self, addressClass=None): - #: (:class:`ipaddr.IPv4Address` or :class:`ipaddr.IPv6Address`) The IP - #: version of bridge addresses to distribute to the client. - self.addressClass = addressClass - if not ((self.addressClass is ipaddr.IPv4Address) or - (self.addressClass is ipaddr.IPv6Address)): - self.addressClass = ipaddr.IPv4Address + def __init__(self, ipVersion=None): + self.ipVersion = ipVersion #: (list) A list of callables used to filter bridges from a hashring. self.filters = list() #: (list) A list of strings of Pluggable Transport types requested. @@ -110,6 +105,26 @@ class BridgeRequestBase(object): #: (bool) Should be ``True`` if the client's request was valid. self.valid = False + @property + def ipVersion(self): + """The IP version of bridge addresses to distribute to the client. + + :rtype: int + :returns: Either ``4`` or ``6``. + """ + return self._ipVersion + + @ipVersion.setter + def ipVersion(self, ipVersion): + """The IP version of bridge addresses to distribute to the client. + + :param int ipVersion: The IP address version for the bridge lines we + should distribute in response to this client request. + """ + if not ipVersion in (4, 6): + ipVersion = 4 + self._ipVersion = ipVersion + def getHashringPlacement(self, key, client=None): """Create an HMAC of some **client** info using a **key**. @@ -145,10 +160,10 @@ class BridgeRequestBase(object): return self.valid def withIPv4(self): - self.addressClass = ipaddr.IPv4Address + self.ipVersion = 4 def withIPv6(self): - self.addressClass = ipaddr.IPv6Address + self.ipVersion = 6 def withoutBlockInCountry(self, country): self.notBlockedIn.append(country.lower()) @@ -175,21 +190,17 @@ class BridgeRequestBase(object): self.clearFilters() pt = self.justOnePTType() - msg = ("Adding a filter to %s for %s for IPv%s" - % (self.__class__.__name__, self.client, self.addressClass)) - - self.ipVersion = 4 - if self.addressClass is ipaddr.IPv6Address: - self.ipVersion = 6 + msg = ("Adding a filter to %s for %s for IPv%d" + % (self.__class__.__name__, self.client, self.ipVersion)) if self.notBlockedIn: for country in self.notBlockedIn: logging.info("%s %s bridges not blocked in %s..." % (msg, pt or "vanilla", country)) - self.addFilter(byNotBlockedIn(country, pt, self.addressClass)) + self.addFilter(byNotBlockedIn(country, pt, self.ipVersion)) elif pt: logging.info("%s %s bridges..." % (msg, pt)) - self.addFilter(byTransport(pt, self.addressClass)) + self.addFilter(byTransport(pt, self.ipVersion)) else: logging.info("%s bridges..." % msg) - self.addFilter(byIPv(self.addressClass)) + self.addFilter(byIPv(self.ipVersion)) diff --git a/lib/bridgedb/bridges.py b/lib/bridgedb/bridges.py index f6e1d6b..f56b0e7 100644 --- a/lib/bridgedb/bridges.py +++ b/lib/bridgedb/bridges.py @@ -785,7 +785,8 @@ class BridgeBackwardsCompatibility(BridgeBase): :data:`bridgerequest.BridgeRequestBase.client`. :param str transport: A pluggable transport method name. """ - bridgeRequest = bridgerequest.BridgeRequestBase(addressClass) + ipVersion = 6 if addressClass is ipaddr.IPv6Address else 4 + bridgeRequest = bridgerequest.BridgeRequestBase(ipVersion) bridgeRequest.client = request if request else bridgeRequest.client bridgeRequest.isValid(True) @@ -1087,7 +1088,7 @@ class Bridge(BridgeBackwardsCompatibility): type. """ desired = bridgeRequest.justOnePTType() - addressClass = bridgeRequest.addressClass + ipVersion = bridgeRequest.ipVersion logging.info("Bridge %s answering request for %s transport..." % (self, desired)) @@ -1096,8 +1097,7 @@ class Bridge(BridgeBackwardsCompatibility): # their ``methodname`` matches the requested transport: transports = filter(lambda pt: pt.methodname == desired, self.transports) # Filter again for whichever of IPv4 or IPv6 was requested: - transports = filter(lambda pt: isinstance(pt.address, addressClass), - transports) + transports = filter(lambda pt: pt.address.version == ipVersion, transports) if not transports: raise PluggableTransportUnavailable( @@ -1136,13 +1136,13 @@ class Bridge(BridgeBackwardsCompatibility): """ logging.info( "Bridge %s answering request for IPv%s vanilla address..." % - (self, "6" if bridgeRequest.addressClass is ipaddr.IPv6Address else "4")) + (self, bridgeRequest.ipVersion)) addresses = [] for address, port, version in self.allVanillaAddresses: # Filter ``allVanillaAddresses`` by whether IPv4 or IPv6 was requested: - if isinstance(address, bridgeRequest.addressClass): + if version == bridgeRequest.ipVersion: # Determine if the address is blocked in any of the country # codes. Because :meth:`addressIsBlockedIn` returns a bool, # we get a list like: ``[True, False, False, True]``, and diff --git a/lib/bridgedb/filters.py b/lib/bridgedb/filters.py index 8843937..ca9b673 100644 --- a/lib/bridgedb/filters.py +++ b/lib/bridgedb/filters.py @@ -126,8 +126,8 @@ def byTransport(methodname=None, ipVersion=None): 1. The :data:`~bridge.bridges.PluggableTransport.methodname` matches **methodname**, and - 2. The :data:`~bridgedb.bridges.PluggableTransport.address`` is an - instance of **addressClass**. + 2. The :data:`~bridgedb.bridges.PluggableTransport.address`` version + matches the **ipVersion**. :param str methodname: A Pluggable Transport :data:`~bridge.bridges.PluggableTransport.methodname`. diff --git a/lib/bridgedb/test/test_email_request.py b/lib/bridgedb/test/test_email_request.py index 7e7f6ea..745ea71 100644 --- a/lib/bridgedb/test/test_email_request.py +++ b/lib/bridgedb/test/test_email_request.py @@ -55,7 +55,7 @@ class DetermineBridgeRequestOptionsTests(unittest.TestCase): self.assertEqual(reqvest.isValid(), False) self.assertFalse(reqvest.wantsKey()) # Though they did request IPv6, technically. - self.assertIs(reqvest.addressClass, ipaddr.IPv6Address) + self.assertIs(reqvest.ipVersion, 6) # And they did request a transport, technically. self.assertEqual(len(reqvest.transports), 1) self.assertEqual(reqvest.transports[0], 'obfs3') @@ -71,7 +71,7 @@ class DetermineBridgeRequestOptionsTests(unittest.TestCase): self.assertEqual(reqvest.isValid(), True) self.assertFalse(reqvest.wantsKey()) # Though they didn't request IPv6, so it should default to IPv4. - self.assertIs(reqvest.addressClass, ipaddr.IPv4Address) + self.assertIs(reqvest.ipVersion, 4) # And they requested two transports. self.assertEqual(len(reqvest.transports), 2) self.assertEqual(reqvest.transports[0], 'obfs3') @@ -93,7 +93,7 @@ class DetermineBridgeRequestOptionsTests(unittest.TestCase): self.assertEqual(reqvest.isValid(), True) self.assertFalse(reqvest.wantsKey()) # Though they didn't request IPv6, so it should default to IPv4. - self.assertIs(reqvest.addressClass, ipaddr.IPv4Address) + self.assertIs(reqvest.ipVersion, 4) # And they requested two transports. self.assertEqual(len(reqvest.transports), 2) self.assertEqual(reqvest.transports[0], 'obfs3') @@ -116,7 +116,7 @@ class DetermineBridgeRequestOptionsTests(unittest.TestCase): lines = ['', 'get ipv6'] reqvest = request.determineBridgeRequestOptions(lines) - self.assertIs(reqvest.addressClass, ipaddr.IPv6Address) + self.assertIs(reqvest.ipVersion, 6) self.assertEqual(reqvest.isValid(), True) @@ -128,7 +128,7 @@ class EmailBridgeRequestTests(unittest.TestCase): self.request = request.EmailBridgeRequest() def tearDown(self): - """Reset cached 'unblocked'/'transport' lists and addressClass between + """Reset cached 'unblocked'/'transport' lists and ipVersion between tests. """ self.request.withIPv4() @@ -174,10 +174,10 @@ class EmailBridgeRequestTests(unittest.TestCase): self.assertEqual(self.request.wantsKey(), False) def test_EmailBridgeRequest_withIPv6(self): - """IPv6 requests should have ``addressClass = ipaddr.IPv6Address``.""" - self.assertEqual(self.request.addressClass, ipaddr.IPv4Address) + """IPv6 requests should have ``ipVersion == 6``.""" + self.assertEqual(self.request.ipVersion, 4) self.request.withIPv6() - self.assertEqual(self.request.addressClass, ipaddr.IPv6Address) + self.assertEqual(self.request.ipVersion, 6) def test_EmailBridgeRequest_withoutBlockInCountry_CN(self): """Country codes that aren't lowercase should be ignored.""" diff --git a/lib/bridgedb/test/util.py b/lib/bridgedb/test/util.py index 2cddc11..500bbb8 100644 --- a/lib/bridgedb/test/util.py +++ b/lib/bridgedb/test/util.py @@ -273,7 +273,7 @@ class DummyBridge(object): line = [] if bridgeRequest.transports: line.append(bridgeRequest.transports[-1]) # Just the last PT - if bridgeRequest.addressClass is ipaddr.IPv6Address: + if bridgeRequest.ipVersion is 6: line.append("[%s]:%s" % self.orAddresses[0][:2]) else: line.append("%s:%s" % (self.address, self.orPort))