[tor-commits] [stem/master] Parsing the params attribute

atagar at torproject.org atagar at torproject.org
Sat Oct 13 18:35:45 UTC 2012


commit 355e474b3d62dceb09699f1be27b5b27b925e7fa
Author: Damian Johnson <atagar at torproject.org>
Date:   Mon Sep 10 09:12:19 2012 -0700

    Parsing the params attribute
    
    Being a fair bit more anal about field validation, checking the following...
    
    * bounds on the value to be an 32 bit int
    * order of the values (keys should be ascending)
    * reject values like '+11' (the int() function accepts them)
    
    This also includes unit tests for these use cases and better error messaging.
---
 stem/descriptor/networkstatus.py               |   50 ++++++++++++++--
 test/unit/descriptor/networkstatus/document.py |   74 +++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 7 deletions(-)

diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py
index 1efcf58..d43c1d0 100644
--- a/stem/descriptor/networkstatus.py
+++ b/stem/descriptor/networkstatus.py
@@ -196,7 +196,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
   :var list client_versions: list of recommended client tor versions
   :var list server_versions: list of recommended server tor versions
   :var list known_flags: **\*** list of known router flags
-  :var list params: dict of parameter(str) => value(int) mappings
+  :var list params: **\*** dict of parameter(str) => value(int) mappings
   :var list directory_authorities: **\*** list of DirectoryAuthority objects that have generated this document
   :var dict bandwidth_weights: **~** dict of weight(str) => value(int) mappings
   :var list directory_signatures: **\*** list of signatures this document has
@@ -381,6 +381,49 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
         
         # simply fetches the entries, excluding empty strings
         self.known_flags = [entry for entry in value.split(" ") if entry]
+      elif keyword == "params":
+        # "params" [Parameters]
+        # Parameter ::= Keyword '=' Int32
+        # Int32 ::= A decimal integer between -2147483648 and 2147483647.
+        # 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)):
+          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
+        if value == "": continue
+        
+        for entry in value.split(" "):
+          try:
+            if not '=' in entry:
+              raise ValueError("must only have 'key=value' entries")
+            
+            entry_key, entry_value = entry.split("=", 1)
+            
+            try:
+              # the int() function accepts things like '+123', but we don't want to
+              if entry_value.startswith('+'):
+                raise ValueError()
+              
+              entry_value = int(entry_value)
+            except ValueError:
+              raise ValueError("'%s' is a non-numeric value" % entry_value)
+            
+            if validate:
+              # check int32 range
+              if entry_value < -2147483648 or entry_value > 2147483647:
+                raise ValueError("values must be between -2147483648 and 2147483647")
+              
+              # parameters should be in ascending order by their key
+              for prior_key in self.params:
+                if prior_key > entry_key:
+                  raise ValueError("parameters must be sorted by their key")
+            
+            self.params[entry_key] = entry_value
+          except ValueError, exc:
+            if not validate: continue
+            raise ValueError("Unable to parse network status document's 'params' line (%s): %s'" % (exc, line))
     
     # doing this validation afterward so we know our 'is_consensus' and
     # 'is_vote' attributes
@@ -407,13 +450,10 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
     _read_keyword_line("client-versions", content, False, True)
     _read_keyword_line("server-versions", content, False, True)
     _read_keyword_line("known-flags", content, False, True)
+    _read_keyword_line("params", content, False, True)
     
     vote = self.is_vote
     
-    read_keyword_line("params", True)
-    if self.params:
-      self.params = dict([(param.split("=")[0], int(param.split("=")[1])) for param in self.params.split(" ")])
-    
     # authority section
     while _peek_keyword(content) == "dir-source":
       dirauth_data = _read_until_keywords(["dir-source", "r", "directory-footer", "directory-signature", "bandwidth-weights"], content, False, True)
diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py
index 97e239a..01f08bf 100644
--- a/test/unit/descriptor/networkstatus/document.py
+++ b/test/unit/descriptor/networkstatus/document.py
@@ -116,7 +116,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
     self.assertEqual([], document.client_versions)
     self.assertEqual([], document.server_versions)
     self.assertEqual(expected_known_flags, document.known_flags)
-    self.assertEqual(None, document.params)
+    self.assertEqual({}, document.params)
     self.assertEqual([], document.directory_authorities)
     self.assertEqual(None, document.bandwidth_weights)
     self.assertEqual([sig], document.directory_signatures)
@@ -150,7 +150,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
     self.assertEqual([], document.client_versions)
     self.assertEqual([], document.server_versions)
     self.assertEqual(expected_known_flags, document.known_flags)
-    self.assertEqual(None, document.params)
+    self.assertEqual({}, document.params)
     self.assertEqual([], document.directory_authorities)
     self.assertEqual({}, document.bandwidth_weights)
     self.assertEqual([sig], document.directory_signatures)
@@ -441,4 +441,74 @@ class TestNetworkStatusDocument(unittest.TestCase):
       content = get_network_status_document({"known-flags": test_value})
       document = NetworkStatusDocument(content)
       self.assertEquals(expected_value, document.known_flags)
+  
+  def test_params(self):
+    """
+    General testing for the 'params' line, exercising the happy cases.
+    """
+    
+    content = get_network_status_document({"params": "CircuitPriorityHalflifeMsec=30000 bwauthpid=1 unrecognized=-122"})
+    document = NetworkStatusDocument(content)
+    self.assertEquals(30000, document.params["CircuitPriorityHalflifeMsec"])
+    self.assertEquals(1, document.params["bwauthpid"])
+    self.assertEquals(-122, document.params["unrecognized"])
+    
+    # empty params line
+    content = get_network_status_document({"params": ""})
+    document = NetworkStatusDocument(content)
+    self.assertEquals({}, document.params)
+  
+  def test_params_malformed(self):
+    """
+    Parses a 'params' line with malformed content.
+    """
+    
+    test_values = (
+      "foo=",
+      "foo=abc",
+      "foo=+123",
+      "foo=12\tbar=12",
+    )
+    
+    for test_value in test_values:
+      content = get_network_status_document({"params": test_value})
+      self.assertRaises(ValueError, NetworkStatusDocument, content)
+      
+      document = NetworkStatusDocument(content, False)
+      self.assertEquals({}, document.params)
+  
+  def test_params_range(self):
+    """
+    Check both the furthest valid 'params' values and values that are out of
+    bounds.
+    """
+    
+    test_values = (
+      ("foo=2147483648", {"foo": 2147483648}, False),
+      ("foo=-2147483649", {"foo": -2147483649}, False),
+      ("foo=2147483647", {"foo": 2147483647}, True),
+      ("foo=-2147483648", {"foo": -2147483648}, True),
+    )
+    
+    for test_value, expected_value, is_ok in test_values:
+      content = get_network_status_document({"params": test_value})
+      
+      if is_ok:
+        document = NetworkStatusDocument(content)
+      else:
+        self.assertRaises(ValueError, NetworkStatusDocument, content)
+        document = NetworkStatusDocument(content, False)
+      
+      self.assertEquals(expected_value, document.params)
+  
+  def test_params_misordered(self):
+    """
+    Check that the 'params' line is rejected if out of order.
+    """
+    
+    content = get_network_status_document({"params": "unrecognized=-122 bwauthpid=1"})
+    self.assertRaises(ValueError, NetworkStatusDocument, content)
+    
+    document = NetworkStatusDocument(content, False)
+    self.assertEquals({"unrecognized": -122, "bwauthpid": 1}, document.params)
 





More information about the tor-commits mailing list