commit 5c4a3ec4cb22fe0fff6c44f3eecf2a2639788ed6 Author: Damian Johnson atagar@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())