commit b931e980b62600d5cc9cd60069102c2ffa26cb84 Author: Damian Johnson <atagar@torproject.org> Date: Sat Sep 8 12:35:01 2012 -0700 Parsing vote-status attribute Changing our 'vote_status' string attribute to 'is_vote' and 'is_consensus' boolean attributes. The spec specifically says that anything else is invalid so there's little reason to allow arbitrary content in the field. --- stem/descriptor/networkstatus.py | 61 ++++++++++++------------ test/integ/descriptor/networkstatus.py | 6 ++- test/unit/descriptor/networkstatus/document.py | 27 +++++++++- test/unit/descriptor/networkstatus/entry.py | 3 +- 4 files changed, 60 insertions(+), 37 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index d554d42..f4c3b40 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -199,7 +199,8 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): :var tuple routers: RouterStatusEntry contained in the document :var str version: **\*** document version - :var str vote_status: **\*** status of the vote (is either "vote" or "consensus") + :var bool is_consensus: **\*** true if the document is a consensus + :var bool is_vote: **\*** true if the document is a vote :var int consensus_method: **~** consensus method used to generate a consensus :var list consensus_methods: **^** A list of supported consensus generation methods (integers) :var datetime published: **^** time when the document was published @@ -237,7 +238,8 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): self.directory_signatures = [] self.version = None - self.vote_status = None + self.is_consensus = True + self.is_vote = False self.consensus_methods = [] self.published = None self.consensus_method = None @@ -293,19 +295,6 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): all_entries.update(header_entries) all_entries.update(footer_entries) - if 'vote-status' in header_entries: - is_consensus = header_entries['vote-status'][0][0] == "consensus" - is_vote = not is_consensus - else: - if validate: - raise ValueError("Network status documents must have a 'vote-status' line to say if they're a vote or consensus") - - is_consensus, is_vote = True, False - - if validate: - self._check_for_missing_and_disallowed_fields(is_consensus, header_entries, footer_entries) - self._check_for_misordered_fields(is_consensus, header_entries, footer_entries) - known_fields = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS + FOOTER_STATUS_DOCUMENT_FIELDS] content = header + '\n' + footer @@ -330,6 +319,22 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): if validate and self.version != "3": raise ValueError("Expected a version 3 network status documents, got version '%s' instead" % self.version) + elif keyword == 'vote-status': + # "vote-status" type + + if value == 'consensus': + self.is_consensus, self.is_vote = True, False + elif value == 'vote': + self.is_consensus, self.is_vote = False, True + elif validate: + raise ValueError("A network status document's vote-status line can only be 'consensus' or 'vote', got '%s' instead" % value) + + # doing this validation afterward so we know our 'is_consensus' and + # 'is_vote' attributes + + if validate: + self._check_for_missing_and_disallowed_fields(header_entries, footer_entries) + self._check_for_misordered_fields(header_entries, footer_entries) def _parse_old(self, raw_content, validate): # preamble @@ -338,14 +343,9 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): # ignore things the parse() method handles _read_keyword_line("network-status-version", content, False, True) + _read_keyword_line("vote-status", content, False, True) - - map(read_keyword_line, ["vote-status"]) - - vote = False - if self.vote_status == "vote": vote = True - elif self.vote_status == "consensus": vote = False - elif validate: raise ValueError("Unrecognized vote-status") + vote = self.is_vote if vote: read_keyword_line("consensus-methods", True) @@ -431,30 +431,28 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): else: self.unrecognized_lines = [] - def _check_for_missing_and_disallowed_fields(self, is_consensus, header_entries, footer_entries): + def _check_for_missing_and_disallowed_fields(self, header_entries, footer_entries): """ Checks that we have mandatory fields for our type, and that we don't have any fields exclusive to the other (ie, no vote-only fields appear in a consensus or vice versa). - :param bool is_consensus: true if we're a conensus and false if we're a vote :param dict header_entries: ordered keyword/value mappings of the header :param dict footer_entries: ordered keyword/value mappings of the footer :raises: ValueError if we're missing mandatory fields or have fiels we shouldn't """ - is_vote = not is_consensus # aliasing inverse for readability missing_fields, disallowed_fields = [], [] 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 ((is_consensus and in_consensus) or (is_vote and in_votes)): + if 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) - elif (is_consensus and not in_consensus) or (is_vote and not in_votes): + elif (self.is_consensus and not in_consensus) or (self.is_vote and not in_votes): # field we shouldn't have, check that we don't if field in entries.keys(): disallowed_fields.append(field) @@ -465,7 +463,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): if disallowed_fields: raise ValueError("Network status document has fields that shouldn't appear in this document type: %s" % ', '.join(disallowed_fields)) - def _check_for_misordered_fields(self, is_consensus, header_entries, footer_entries): + def _check_for_misordered_fields(self, header_entries, footer_entries): """ To be valid a network status document's fiends need to appear in a specific order. Checks that known fields appear in that order (unrecognized fields @@ -824,10 +822,10 @@ class RouterStatusEntry(stem.descriptor.Descriptor): m_comp = value.split(" ") - if not (self.document and self.document.vote_status == "vote"): + if not (self.document and self.document.is_vote): if not validate: continue - vote_status = self.document.vote_status if self.document else "<undefined document>" + vote_status = "vote" if self.document else "<undefined document>" raise ValueError("Router status entry's 'm' line should only appear in votes (appeared in a %s): %s" % (vote_status, line)) elif len(m_comp) < 1: if not validate: continue @@ -870,7 +868,8 @@ class MicrodescriptorConsensus(NetworkStatusDocument): A v3 microdescriptor consensus. :var str version: **\*** a document format version. For v3 microdescriptor consensuses this is "3 microdesc" - :var str vote_status: **\*** status of the vote (is "consensus") + :var bool is_consensus: **\*** true if the document is a consensus + :var bool is_vote: **\*** true if the document is a vote :var int consensus_method: **~** consensus method used to generate a consensus :var datetime valid_after: **\*** time when the consensus becomes valid :var datetime fresh_until: **\*** time until when the consensus is considered to be fresh diff --git a/test/integ/descriptor/networkstatus.py b/test/integ/descriptor/networkstatus.py index 1777828..9087b22 100644 --- a/test/integ/descriptor/networkstatus.py +++ b/test/integ/descriptor/networkstatus.py @@ -86,7 +86,8 @@ class TestNetworkStatus(unittest.TestCase): descriptor_file.close() self.assertEquals("3", desc.version) - self.assertEquals("consensus", desc.vote_status) + self.assertEquals(True, desc.is_consensus) + self.assertEquals(False, desc.is_vote) self.assertEquals([], desc.consensus_methods) self.assertEquals(None, desc.published) self.assertEquals(12, desc.consensus_method) @@ -178,7 +179,8 @@ I/TJmV928na7RLZe2mGHCAW3VQOvV+QkCfj05VZ8CsY= descriptor_file.close() self.assertEquals("3", desc.version) - self.assertEquals("vote", desc.vote_status) + self.assertEquals(False, desc.is_consensus) + self.assertEquals(True, desc.is_vote) self.assertEquals(range(1, 13), desc.consensus_methods) self.assertEquals(_strptime("2012-07-11 23:50:01"), desc.published) self.assertEquals(None, desc.consensus_method) diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py index a099fa9..9b24a00 100644 --- a/test/unit/descriptor/networkstatus/document.py +++ b/test/unit/descriptor/networkstatus/document.py @@ -101,7 +101,8 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertEqual((), document.routers) self.assertEqual("3", document.version) - self.assertEqual("consensus", document.vote_status) + self.assertEqual(True, document.is_consensus) + self.assertEqual(False, document.is_vote) self.assertEqual(9, document.consensus_method) self.assertEqual([], document.consensus_methods) self.assertEqual(None, document.published) @@ -134,7 +135,8 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertEqual((), document.routers) self.assertEqual("3", document.version) - self.assertEqual("vote", document.vote_status) + self.assertEqual(False, document.is_consensus) + self.assertEqual(True, document.is_vote) self.assertEqual(None, document.consensus_method) self.assertEqual([9], document.consensus_methods) self.assertEqual(datetime.datetime(2012, 9, 2, 22, 0, 0), document.published) @@ -216,7 +218,7 @@ class TestNetworkStatusDocument(unittest.TestCase): def test_invalid_version(self): """ - Try parsing a different document version with the v3 parser. + Parses a different document version with the v3 parser. """ content = get_network_status_document({"network-status-version": "4"}) @@ -224,4 +226,23 @@ class TestNetworkStatusDocument(unittest.TestCase): document = NetworkStatusDocument(content, False) self.assertEquals("4", document.version) + + def test_invalid_vote_status(self): + """ + Parses an invalid vote-status field. + """ + + test_values = ( + "", + " ", + "votee", + ) + + for test_value in test_values: + content = get_network_status_document({"vote-status": test_value}) + self.assertRaises(ValueError, NetworkStatusDocument, content) + + document = NetworkStatusDocument(content, False) + self.assertEquals(True, document.is_consensus) + self.assertEquals(False, document.is_vote) diff --git a/test/unit/descriptor/networkstatus/entry.py b/test/unit/descriptor/networkstatus/entry.py index 7eb0cb8..195600c 100644 --- a/test/unit/descriptor/networkstatus/entry.py +++ b/test/unit/descriptor/networkstatus/entry.py @@ -408,7 +408,8 @@ class TestRouterStatusEntry(unittest.TestCase): # we need a document that's a vote mock_document = lambda x: x # just need anything with a __dict__ - mock_document.__dict__["vote_status"] = "vote" + mock_document.__dict__["is_vote"] = True + mock_document.__dict__["is_consensus"] = False for m_line, expected in test_values.items(): content = get_router_status_entry({'m': m_line})