[tor-commits] [stem/master] Splitting up ExitPolicyRule parsing

atagar at torproject.org atagar at torproject.org
Thu Jul 19 16:01:03 UTC 2012


commit 004650ae2b09ec295f6e7ce4d70993c1764761d9
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Jul 15 14:53:53 2012 -0700

    Splitting up ExitPolicyRule parsing
    
    The ExitPolicyRule constructor was unpleasantly huge (and by extension, hard to
    read). Moving most of it to helper funcitons for parsing the addrspec and
    portspec.
---
 stem/exit_policy.py |  226 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 124 insertions(+), 102 deletions(-)

diff --git a/stem/exit_policy.py b/stem/exit_policy.py
index 312ba65..9f1cd94 100644
--- a/stem/exit_policy.py
+++ b/stem/exit_policy.py
@@ -55,7 +55,26 @@ AddressType = stem.util.enum.Enum(("WILDCARD", "Wildcard"), ("IPv4", "IPv4"), ("
 PRIVATE_IP_RANGES = ("0.0.0.0/8", "169.254.0.0/16", "127.0.0.0/8",
                      "192.168.0.0/16", "10.0.0.0/8", "172.16.0.0/12")
 
-class ExitPolicyRule:
+# TODO: The ExitPolicyRule's exitpatterns are used everywhere except the torrc.
+# This is fine for now, but we should add a subclass to handle those slight
+# differences later if we want to provide the ability to parse torrcs.
+
+# TODO: The ExitPolicyRule could easily be a mutable class if we did the
+# following...
+#
+# * Provided setter methods that acquired an RLock which also wrapped all of
+#   our current methods to provide thread safety.
+#
+# * Reset our derived attributes (self._addr_bin, self._mask_bin, and
+#   self._str_representation) when we changed something that it was based on.
+#
+# That said, I'm not sure if this is entirely desirable since for most use
+# cases we *want* the caller to have an immutable ExitPolicy (since it
+# reflects something they... well, can't modify). However, I can think of
+# some use cases where we might want to construct custom policies. Mabye make
+# it a CustomExitPolicyRule subclass?
+
+class ExitPolicyRule(object):
   """
   Single rule from the user's exit policy. These rules are chained together to
   form complete policies that describe where a relay will and will not allow
@@ -67,7 +86,9 @@ class ExitPolicyRule:
   scricter in what it'll accept. For instance, ports are not optional and it
   does not contain the 'private' alias.
   
-  :var str rule: rule that we were created from
+  This should be treated as an immutable object.
+  
+  :var str rule: rule that we were originally created from
   :var bool is_accept: indicates if exiting is allowed or disallowed
   
   :var AddressType address_type: type of address that we have
@@ -83,10 +104,6 @@ class ExitPolicyRule:
   :raises: ValueError if input isn't a valid tor exit policy rule
   """
   
-  # TODO: Exitpatterns are used everywhere except the torrc. This is fine for
-  # now, but we should add a subclass to handle those slight differences later
-  # if we want to provide the ability to parse torrcs.
- 
   def __init__(self, rule):
     self.rule = rule
     
@@ -110,96 +127,14 @@ class ExitPolicyRule:
     if not ":" in exitpattern:
       raise ValueError("An exitpattern must be of the form 'addrspec:portspec': %s" % rule)
     
-    addrspec, portspec = exitpattern.rsplit(":", 1)
-    self._addr_bin = self._mask_bin = None
-    
-    # Parses the addrspec...
-    # addrspec ::= "*" | ip4spec | ip6spec
-    
-    if "/" in addrspec:
-      self.address, addr_extra = addrspec.split("/", 1)
-    else:
-      self.address, addr_extra = addrspec, None
+    self.address = None
+    self.address_type = None
+    self.mask = self.masked_bits = None
+    self.min_port = self.max_port = None
     
-    if addrspec == "*":
-      self.address_type = AddressType.WILDCARD
-      self.address = self.mask = self.masked_bits = None
-    elif stem.util.connection.is_valid_ip_address(self.address):
-      # ipv4spec ::= ip4 | ip4 "/" num_ip4_bits | ip4 "/" ip4mask
-      # ip4 ::= an IPv4 address in dotted-quad format
-      # ip4mask ::= an IPv4 mask in dotted-quad format
-      # num_ip4_bits ::= an integer between 0 and 32
-      
-      self.address_type = AddressType.IPv4
-      
-      if addr_extra is None:
-        self.mask = stem.util.connection.FULL_IPv4_MASK
-        self.masked_bits = 32
-      elif stem.util.connection.is_valid_ip_address(addr_extra):
-        # provided with an ip4mask
-        self.mask = addr_extra
-        
-        try:
-          self.masked_bits = stem.util.connection.get_masked_bits(addr_extra)
-        except ValueError:
-          # mask can't be represented as a number of bits (ex. "255.255.0.255")
-          self.masked_bits = None
-      elif addr_extra.isdigit():
-        # provided with a num_ip4_bits
-        self.mask = stem.util.connection.get_mask(int(addr_extra))
-        self.masked_bits = int(addr_extra)
-      else:
-        raise ValueError("The '%s' isn't a mask nor number of bits: %s" % (addr_extra, rule))
-    elif self.address.startswith("[") and self.address.endswith("]") and \
-      stem.util.connection.is_valid_ipv6_address(self.address[1:-1]):
-      # ip6spec ::= ip6 | ip6 "/" num_ip6_bits
-      # ip6 ::= an IPv6 address, surrounded by square brackets.
-      # num_ip6_bits ::= an integer between 0 and 128
-      
-      self.address = stem.util.connection.expand_ipv6_address(self.address[1:-1].upper())
-      self.address_type = AddressType.IPv6
-      
-      if addr_extra is None:
-        self.mask = stem.util.connection.FULL_IPv6_MASK
-        self.masked_bits = 128
-      elif addr_extra.isdigit():
-        # provided with a num_ip6_bits
-        self.mask = stem.util.connection.get_mask_ipv6(int(addr_extra))
-        self.masked_bits = int(addr_extra)
-      else:
-        raise ValueError("The '%s' isn't a number of bits: %s" % (addr_extra, rule))
-    else:
-      raise ValueError("Address isn't a wildcard, IPv4, or IPv6 address: %s" % rule)
-    
-    # Parses the portspec...
-    # portspec ::= "*" | port | port "-" port
-    # port ::= an integer between 1 and 65535, inclusive.
-    #
-    # Due to a tor bug the spec says that we should accept port of zero, but
-    # connections to port zero are never permitted.
-    
-    if portspec == "*":
-      self.min_port, self.max_port = 1, 65535
-    elif portspec.isdigit():
-      # provided with a single port
-      if stem.util.connection.is_valid_port(portspec, allow_zero = True):
-        self.min_port = self.max_port = int(portspec)
-      else:
-        raise ValueError("'%s' isn't within a valid port range: %s" % (portspec, rule))
-    elif "-" in portspec:
-      # provided with a port range
-      port_comp = portspec.split("-", 1)
-      
-      if stem.util.connection.is_valid_port(port_comp, allow_zero = True):
-        self.min_port = int(port_comp[0])
-        self.max_port = int(port_comp[1])
-        
-        if self.min_port > self.max_port:
-          raise ValueError("Port range has a lower bound that's greater than its upper bound: %s" % rule)
-      else:
-        raise ValueError("Malformed port range: %s" % rule)
-    else:
-      raise ValueError("Port value isn't a wildcard, integer, or range: %s" % rule)
+    addrspec, portspec = exitpattern.rsplit(":", 1)
+    self._apply_addrspec(addrspec)
+    self._apply_portspec(portspec)
     
     # Pre-calculating the integer representation of our mask and masked
     # address. These are used by our is_match() method to compare ourselves to
@@ -327,17 +262,104 @@ class ExitPolicyRule:
       self._str_representation = label
     
     return self._str_representation
+  
+  def _apply_addrspec(self, addrspec):
+    # Parses the addrspec...
+    # addrspec ::= "*" | ip4spec | ip6spec
+    
+    if "/" in addrspec:
+      self.address, addr_extra = addrspec.split("/", 1)
+    else:
+      self.address, addr_extra = addrspec, None
+    
+    if addrspec == "*":
+      self.address_type = AddressType.WILDCARD
+      self.address = self.mask = self.masked_bits = None
+    elif stem.util.connection.is_valid_ip_address(self.address):
+      # ipv4spec ::= ip4 | ip4 "/" num_ip4_bits | ip4 "/" ip4mask
+      # ip4 ::= an IPv4 address in dotted-quad format
+      # ip4mask ::= an IPv4 mask in dotted-quad format
+      # num_ip4_bits ::= an integer between 0 and 32
+      
+      self.address_type = AddressType.IPv4
+      
+      if addr_extra is None:
+        self.mask = stem.util.connection.FULL_IPv4_MASK
+        self.masked_bits = 32
+      elif stem.util.connection.is_valid_ip_address(addr_extra):
+        # provided with an ip4mask
+        self.mask = addr_extra
+        
+        try:
+          self.masked_bits = stem.util.connection.get_masked_bits(addr_extra)
+        except ValueError:
+          # mask can't be represented as a number of bits (ex. "255.255.0.255")
+          self.masked_bits = None
+      elif addr_extra.isdigit():
+        # provided with a num_ip4_bits
+        self.mask = stem.util.connection.get_mask(int(addr_extra))
+        self.masked_bits = int(addr_extra)
+      else:
+        raise ValueError("The '%s' isn't a mask nor number of bits: %s" % (addr_extra, self.rule))
+    elif self.address.startswith("[") and self.address.endswith("]") and \
+      stem.util.connection.is_valid_ipv6_address(self.address[1:-1]):
+      # ip6spec ::= ip6 | ip6 "/" num_ip6_bits
+      # ip6 ::= an IPv6 address, surrounded by square brackets.
+      # num_ip6_bits ::= an integer between 0 and 128
+      
+      self.address = stem.util.connection.expand_ipv6_address(self.address[1:-1].upper())
+      self.address_type = AddressType.IPv6
+      
+      if addr_extra is None:
+        self.mask = stem.util.connection.FULL_IPv6_MASK
+        self.masked_bits = 128
+      elif addr_extra.isdigit():
+        # provided with a num_ip6_bits
+        self.mask = stem.util.connection.get_mask_ipv6(int(addr_extra))
+        self.masked_bits = int(addr_extra)
+      else:
+        raise ValueError("The '%s' isn't a number of bits: %s" % (addr_extra, self.rule))
+    else:
+      raise ValueError("Address isn't a wildcard, IPv4, or IPv6 address: %s" % self.rule)
+  
+  def _apply_portspec(self, portspec):
+    # Parses the portspec...
+    # portspec ::= "*" | port | port "-" port
+    # port ::= an integer between 1 and 65535, inclusive.
+    #
+    # Due to a tor bug the spec says that we should accept port of zero, but
+    # connections to port zero are never permitted.
+    
+    if portspec == "*":
+      self.min_port, self.max_port = 1, 65535
+    elif portspec.isdigit():
+      # provided with a single port
+      if stem.util.connection.is_valid_port(portspec, allow_zero = True):
+        self.min_port = self.max_port = int(portspec)
+      else:
+        raise ValueError("'%s' isn't within a valid port range: %s" % (portspec, self.rule))
+    elif "-" in portspec:
+      # provided with a port range
+      port_comp = portspec.split("-", 1)
+      
+      if stem.util.connection.is_valid_port(port_comp, allow_zero = True):
+        self.min_port = int(port_comp[0])
+        self.max_port = int(port_comp[1])
+        
+        if self.min_port > self.max_port:
+          raise ValueError("Port range has a lower bound that's greater than its upper bound: %s" % self.rule)
+      else:
+        raise ValueError("Malformed port range: %s" % self.rule)
+    else:
+      raise ValueError("Port value isn't a wildcard, integer, or range: %s" % self.rule)
 
-class ExitPolicy:
+class ExitPolicy(object):
   """
-  Provides a wrapper to ExitPolicyRule. This is iterable and can be stringified for
-  individual Exit Policy lines.
+  Policy for the destinations that a relay allows or denies exiting to. This
+  is, in effect, simply a list of ExitPolicyRule entries.
   """
-    
+  
   def __init__(self):
-    """
-    ExitPolicy constructor
-    """
     self._policies = []
     self.summary = ""
 





More information about the tor-commits mailing list