[tor-commits] [stem/master] Parsing the directory-footer attribute

[atagar at torproject.org]

commit 5c4a3ec4cb22fe0fff6c44f3eecf2a2639788ed6
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Sep 16 17:07:03 2012 -0700

    Parsing the directory-footer attribute
    
    For being an empty attribute this sure is a strangely big change. Checking the
    following...
    
    - that footers don't appear prior to consensus-method 9
    - that the directory-footer lacks any content
    - that prior to consensus-method 9 we're happy to not have the line (bug I
      introduced because the footer has mandatory fields)
---
 stem/descriptor/networkstatus.py               |   35 +++++++++++++-----
 test/unit/descriptor/networkstatus/document.py |   46 +++++++++++++++++++----
 2 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py
index 2402460..e262264 100644
--- a/stem/descriptor/networkstatus.py
+++ b/stem/descriptor/networkstatus.py
@@ -268,6 +268,19 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
   def _get_router_type(self):
     return RouterStatusEntry
   
+  def meets_consensus_method(self, method):
+    """
+    Checks if we meet the given consensus-method. This works for both votes and
+    consensuses, checking our 'consensus-method' and 'consensus-methods'
+    entries.
+    
+    :param int method: consensus-method to check for
+    
+    :returns: True if we meet the given consensus-method, and False otherwise
+    """
+    
+    return bool(self.consensus_method >= method or filter(lambda x: x >= method, self.consensus_methods))
+  
   def get_unrecognized_lines(self):
     """
     Returns any unrecognized trailing lines.
@@ -398,7 +411,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
         # Parameters ::= Parameter | Parameters SP Parameter
         
         # should only appear in consensus-method 7 or later
-        if validate and not (self.consensus_method >= 7 or filter(lambda x: x >= 7, self.consensus_methods)):
+        if validate and not self.meets_consensus_method(7):
           raise ValueError("A network status document's 'params' line should only appear in consensus-method 7 or later")
         
         # skip if this is a blank line
@@ -435,6 +448,11 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
         
         if validate:
           self._check_params_constraints()
+      elif keyword == "directory-footer":
+        # nothing to parse, simply checking that we don't have a value
+        
+        if validate and value:
+          raise ValueError("A network status document's 'directory-footer' line shouldn't have any content, got '%s'" % line)
     
     # doing this validation afterward so we know our 'is_consensus' and
     # 'is_vote' attributes
@@ -471,12 +489,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
       dirauth_data = "".join(dirauth_data).rstrip()
       self.directory_authorities.append(DirectoryAuthority(dirauth_data, vote, validate))
     
-    # footer section
-    if self.consensus_method >= 9 or (vote and filter(lambda x: x >= 9, self.consensus_methods)):
-      if _peek_keyword(content) == "directory-footer":
-        content.readline()
-      elif validate:
-        raise ValueError("Network status document missing directory-footer")
+    _read_keyword_line("directory-footer", content, False, True)
     
     if not vote:
       read_keyword_line("bandwidth-weights", True)
@@ -511,7 +524,11 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
     for entries, fields in ((header_entries, HEADER_STATUS_DOCUMENT_FIELDS),\
                             (footer_entries, FOOTER_STATUS_DOCUMENT_FIELDS)):
       for field, in_votes, in_consensus, mandatory in fields:
-        if mandatory and ((self.is_consensus and in_consensus) or (self.is_vote and in_votes)):
+        if field in ('directory-footer', 'directory-signature') and not self.meets_consensus_method(9):
+          # footers only appear in consensus-method 9 or later
+          if field in entries.keys():
+            disallowed_fields.append(field)
+        elif mandatory and ((self.is_consensus and in_consensus) or (self.is_vote and in_votes)):
           # mandatory field, check that we have it
           if not field in entries.keys():
             missing_fields.append(field)
@@ -524,7 +541,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
       raise ValueError("Network status document is missing mandatory field: %s" % ', '.join(missing_fields))
     
     if disallowed_fields:
-      raise ValueError("Network status document has fields that shouldn't appear in this document type: %s" % ', '.join(disallowed_fields))
+      raise ValueError("Network status document has fields that shouldn't appear in this document type or version: %s" % ', '.join(disallowed_fields))
   
   def _check_for_misordered_fields(self, header_entries, footer_entries):
     """
diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py
index 1fa932a..6d93624 100644
--- a/test/unit/descriptor/networkstatus/document.py
+++ b/test/unit/descriptor/networkstatus/document.py
@@ -30,6 +30,8 @@ NETWORK_STATUS_DOCUMENT_ATTR = {
     "-----END SIGNATURE-----")),
 }
 
+SIG = DirectorySignature("directory-signature " + NETWORK_STATUS_DOCUMENT_ATTR["directory-signature"])
+
 def get_network_status_document(attr = None, exclude = None, routers = None):
   """
   Constructs a minimal network status document with the given attributes. This
@@ -99,8 +101,6 @@ class TestNetworkStatusDocument(unittest.TestCase):
       Flag.FAST, Flag.GUARD, Flag.HSDIR, Flag.NAMED, Flag.RUNNING,
       Flag.STABLE, Flag.UNNAMED, Flag.V2DIR, Flag.VALID]
     
-    sig = DirectorySignature("directory-signature " + NETWORK_STATUS_DOCUMENT_ATTR["directory-signature"])
-    
     self.assertEqual((), document.routers)
     self.assertEqual("3", document.version)
     self.assertEqual(True, document.is_consensus)
@@ -119,7 +119,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
     self.assertEqual(DEFAULT_PARAMS, document.params)
     self.assertEqual([], document.directory_authorities)
     self.assertEqual(None, document.bandwidth_weights)
-    self.assertEqual([sig], document.directory_signatures)
+    self.assertEqual([SIG], document.directory_signatures)
     self.assertEqual([], document.get_unrecognized_lines())
   
   def test_minimal_vote(self):
@@ -133,8 +133,6 @@ class TestNetworkStatusDocument(unittest.TestCase):
       Flag.FAST, Flag.GUARD, Flag.HSDIR, Flag.NAMED, Flag.RUNNING,
       Flag.STABLE, Flag.UNNAMED, Flag.V2DIR, Flag.VALID]
     
-    sig = DirectorySignature("directory-signature " + NETWORK_STATUS_DOCUMENT_ATTR["directory-signature"])
-    
     self.assertEqual((), document.routers)
     self.assertEqual("3", document.version)
     self.assertEqual(False, document.is_consensus)
@@ -153,7 +151,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
     self.assertEqual(DEFAULT_PARAMS, document.params)
     self.assertEqual([], document.directory_authorities)
     self.assertEqual({}, document.bandwidth_weights)
-    self.assertEqual([sig], document.directory_signatures)
+    self.assertEqual([SIG], document.directory_signatures)
     self.assertEqual([], document.get_unrecognized_lines())
   
   def test_missing_fields(self):
@@ -274,7 +272,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
     
     # check that we default to including consensus-method 1
     content = get_network_status_document({"vote-status": "vote"}, ("consensus-methods",))
-    document = NetworkStatusDocument(content)
+    document = NetworkStatusDocument(content, False)
     self.assertEquals([1], document.consensus_methods)
     self.assertEquals(None, document.consensus_method)
     
@@ -304,7 +302,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
     
     # check that we default to being consensus-method 1
     content = get_network_status_document(exclude = ("consensus-method",))
-    document = NetworkStatusDocument(content)
+    document = NetworkStatusDocument(content, False)
     self.assertEquals(1, document.consensus_method)
     self.assertEquals([], document.consensus_methods)
     
@@ -524,4 +522,36 @@ class TestNetworkStatusDocument(unittest.TestCase):
     
     document = NetworkStatusDocument(content, False, default_params = False)
     self.assertEquals({"unrecognized": -122, "bwauthpid": 1}, document.params)
+  
+  def test_footer_consensus_method_requirement(self):
+    """
+    Check that validation will notice if a footer appears before it was
+    introduced.
+    """
+    
+    content = get_network_status_document({"consensus-method": "8"})
+    self.assertRaises(ValueError, NetworkStatusDocument, content)
+    
+    document = NetworkStatusDocument(content, False)
+    self.assertEqual([SIG], document.directory_signatures)
+    self.assertEqual([], document.get_unrecognized_lines())
+    
+    # excludes a footer from a version that shouldn't have it
+    
+    content = get_network_status_document({"consensus-method": "8"}, ("directory-footer", "directory-signature"))
+    document = NetworkStatusDocument(content)
+    self.assertEqual([], document.directory_signatures)
+    self.assertEqual([], document.get_unrecognized_lines())
+  
+  def test_footer_with_value(self):
+    """
+    Tries to parse a descriptor with content on the 'directory-footer' line.
+    """
+    
+    content = get_network_status_document({"directory-footer": "blarg"})
+    self.assertRaises(ValueError, NetworkStatusDocument, content)
+    
+    document = NetworkStatusDocument(content, False)
+    self.assertEqual([SIG], document.directory_signatures)
+    self.assertEqual([], document.get_unrecognized_lines())