October 2012 - tor-commits - lists.torproject.org

[stem/master] Validating params values and including defaults
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit f60e60006a54ba2fc1eba0cb2fa5fade55b670ff Author: Damian Johnson <atagar(a)torproject.org> Date: Wed Sep 12 09:14:30 2012 -0700 Validating params values and including defaults The 'params' line of a network status document has several known entries, with their own constraints on the value. Validating that the document obeys those constraints. Also, the path-spec has default values for a handfull of params so optionally defaulting our params attribute to that. --- stem/descriptor/networkstatus.py | 70 +++++++++++++++++++++--- test/unit/descriptor/networkstatus/document.py | 29 +++++++--- 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index d43c1d0..12396d0 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -88,6 +88,19 @@ FOOTER_STATUS_DOCUMENT_FIELDS = ( ("directory-signature", True, True, True), ) +DEFAULT_PARAMS = { + "cbtdisabled": 0, + "cbtnummodes": 3, + "cbtrecentcount": 20, + "cbtmaxtimeouts": 18, + "cbtmincircs": 100, + "cbtquantile": 80, + "cbtclosequantile": 95, + "cbttestfreq": 60, + "cbtmintimeout": 2000, + "cbtinitialtimeout": 60000, +} + def parse_file(document_file, validate = True, is_microdescriptor = False): """ Parses a network status and iterates over the RouterStatusEntry or @@ -206,12 +219,13 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): | **~** attribute appears only in consensuses """ - def __init__(self, raw_content, validate = True): + def __init__(self, raw_content, validate = True, default_params = True): """ Parse a v3 network status document and provide a new NetworkStatusDocument object. :param str raw_content: raw network status document data :param bool validate: True if the document is to be validated, False otherwise + :param bool default_params: includes defaults in our params dict, otherwise it just contains values from the document :raises: ValueError if the document is invalid """ @@ -235,7 +249,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): self.client_versions = [] self.server_versions = [] self.known_flags = [] - self.params = {} + self.params = dict(DEFAULT_PARAMS) if default_params else {} self.bandwidth_weights = {} document_file = StringIO(raw_content) @@ -394,6 +408,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): # skip if this is a blank line if value == "": continue + seen_keys = [] for entry in value.split(" "): try: if not '=' in entry: @@ -411,19 +426,19 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): 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: + for prior_key in seen_keys: if prior_key > entry_key: raise ValueError("parameters must be sorted by their key") self.params[entry_key] = entry_value + seen_keys.append(entry_key) except ValueError, exc: if not validate: continue raise ValueError("Unable to parse network status document's 'params' line (%s): %s'" % (exc, line)) + + if validate: + self._check_params_constraints() # doing this validation afterward so we know our 'is_consensus' and # 'is_vote' attributes @@ -544,6 +559,47 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): actual_label = ', '.join(actual) expected_label = ', '.join(expected) raise ValueError("The fields in the document's %s are misordered. It should be '%s' but was '%s'" % (label, actual_label, expected_label)) + + def _check_params_constraints(self): + """ + Checks that the params we know about are within their documented ranges. + """ + + for key, value in self.params.items(): + # all parameters are constrained to int32 range + minimum, maximum = -2147483648, 2147483647 + + if key == "circwindow": + minimum, maximum = 100, 1000 + elif key == "CircuitPriorityHalflifeMsec": + minimum = -1 + elif key in ("perconnbwrate", "perconnbwburst"): + minimum = 1 + elif key == "refuseunknownexits": + minimum, maximum = 0, 1 + elif key == "cbtdisabled": + minimum, maximum = 0, 1 + elif key == "cbtnummodes": + minimum, maximum = 1, 20 + elif key == "cbtrecentcount": + minimum, maximum = 3, 1000 + elif key == "cbtmaxtimeouts": + minimum, maximum = 3, 10000 + elif key == "cbtmincircs": + minimum, maximum = 1, 10000 + elif key == "cbtquantile": + minimum, maximum = 10, 99 + elif key == "cbtclosequantile": + minimum, maximum = self.params.get("cbtquantile", minimum), 99 + elif key == "cbttestfreq": + minimum = 1 + elif key == "cbtmintimeout": + minimum = 500 + elif key == "cbtinitialtimeout": + minimum = self.params.get("cbtmintimeout", minimum) + + if value < minimum or value > maximum: + raise ValueError("'%s' value on the params line must be in the range of %i - %i, was %i" % (key, minimum, maximum, value)) class DirectoryAuthority(stem.descriptor.Descriptor): """ diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py index 01f08bf..1fa932a 100644 --- a/test/unit/descriptor/networkstatus/document.py +++ b/test/unit/descriptor/networkstatus/document.py @@ -7,7 +7,7 @@ import unittest import stem.version from stem.descriptor import Flag -from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, NetworkStatusDocument, DirectorySignature +from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, DEFAULT_PARAMS, NetworkStatusDocument, DirectorySignature NETWORK_STATUS_DOCUMENT_ATTR = { "network-status-version": "3", @@ -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({}, document.params) + self.assertEqual(DEFAULT_PARAMS, 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({}, document.params) + self.assertEqual(DEFAULT_PARAMS, document.params) self.assertEqual([], document.directory_authorities) self.assertEqual({}, document.bandwidth_weights) self.assertEqual([sig], document.directory_signatures) @@ -455,7 +455,11 @@ class TestNetworkStatusDocument(unittest.TestCase): # empty params line content = get_network_status_document({"params": ""}) - document = NetworkStatusDocument(content) + document = NetworkStatusDocument(content, default_params = True) + self.assertEquals(DEFAULT_PARAMS, document.params) + + content = get_network_status_document({"params": ""}) + document = NetworkStatusDocument(content, default_params = False) self.assertEquals({}, document.params) def test_params_malformed(self): @@ -475,7 +479,7 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertRaises(ValueError, NetworkStatusDocument, content) document = NetworkStatusDocument(content, False) - self.assertEquals({}, document.params) + self.assertEquals(DEFAULT_PARAMS, document.params) def test_params_range(self): """ @@ -488,16 +492,25 @@ class TestNetworkStatusDocument(unittest.TestCase): ("foo=-2147483649", {"foo": -2147483649}, False), ("foo=2147483647", {"foo": 2147483647}, True), ("foo=-2147483648", {"foo": -2147483648}, True), + + # param with special range constraints + ("circwindow=99", {"circwindow": 99}, False), + ("circwindow=1001", {"circwindow": 1001}, False), + ("circwindow=500", {"circwindow": 500}, True), + + # param that relies on another param for its constraints + ("cbtclosequantile=79 cbtquantile=80", {"cbtclosequantile": 79, "cbtquantile": 80}, False), + ("cbtclosequantile=80 cbtquantile=80", {"cbtclosequantile": 80, "cbtquantile": 80}, 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) + document = NetworkStatusDocument(content, default_params = False) else: self.assertRaises(ValueError, NetworkStatusDocument, content) - document = NetworkStatusDocument(content, False) + document = NetworkStatusDocument(content, False, default_params = False) self.assertEquals(expected_value, document.params) @@ -509,6 +522,6 @@ class TestNetworkStatusDocument(unittest.TestCase): content = get_network_status_document({"params": "unrecognized=-122 bwauthpid=1"}) self.assertRaises(ValueError, NetworkStatusDocument, content) - document = NetworkStatusDocument(content, False) + document = NetworkStatusDocument(content, False, default_params = False) self.assertEquals({"unrecognized": -122, "bwauthpid": 1}, document.params)

1 0

[stem/master] Tidying up NetworkStatusDocument._parse() start
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit 84fe0b46adc684a663af8536da35648adfd841e2 Author: Damian Johnson <atagar(a)torproject.org> Date: Fri Sep 14 09:35:03 2012 -0700 Tidying up NetworkStatusDocument._parse() start Much of the _parse() method's start was unnecessary. Shortening it to improve readability. --- stem/descriptor/networkstatus.py | 14 +++++--------- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index 12396d0..2402460 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -88,6 +88,8 @@ FOOTER_STATUS_DOCUMENT_FIELDS = ( ("directory-signature", True, True, True), ) +ALL_FIELDS = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS + FOOTER_STATUS_DOCUMENT_FIELDS] + DEFAULT_PARAMS = { "cbtdisabled": 0, "cbtnummodes": 3, @@ -289,22 +291,16 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): header_entries = stem.descriptor._get_descriptor_components(header, validate)[0] footer_entries = stem.descriptor._get_descriptor_components(footer, validate)[0] - all_entries = dict() - all_entries.update(header_entries) - all_entries.update(footer_entries) - - known_fields = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS + FOOTER_STATUS_DOCUMENT_FIELDS] - content = header + '\n' + footer - - for keyword, values in all_entries.items(): + for keyword, values in header_entries.items() + footer_entries.items(): value, block_contents = values[0] line = "%s %s" % (keyword, value) # All known fields can only appear once except... # * 'directory-signature' in a consensus - if validate and len(values) > 1 and keyword in known_fields: + if validate and len(values) > 1 and keyword in ALL_FIELDS: if not (keyword == 'directory-signature' and is_consensus): + content = header + '\n' + footer raise ValueError("Network status documents can only have a single '%s' line, got %i:\n%s" % (keyword, len(values), content)) if keyword == 'network-status-version':

1 0

[stem/master] Parsing the directory-footer attribute
by atagar＠torproject.org 13 Oct '12

13 Oct '12

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

1 0

[stem/master] Rejecting all footer fields in outdated consensus-method
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit b3444ace427dca9a66a1afed1d24a09971612810 Author: Damian Johnson <atagar(a)torproject.org> Date: Sun Sep 16 17:43:46 2012 -0700 Rejecting all footer fields in outdated consensus-method We were only checking that we lacked the mandatory footer fields when our consensus-method indicated that we shouldn't have a footer. Instead checking that we have no footer at all. This also makes the code a little nicer... --- stem/descriptor/networkstatus.py | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index e262264..0a4aaeb 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -521,14 +521,18 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): missing_fields, disallowed_fields = [], [] + if not self.meets_consensus_method(9): + # footers only appear in consensus-method 9 or later + if footer_entries: + raise ValueError("Network status document's footer should only apepar in consensus-method 9 or later") + else: + # pretend to have mandatory fields to prevent validation from whining + footer_entries = {"directory-footer": "", "directory-signature": ""} + 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 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)): + 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)

1 0

[stem/master] Noting that we might want to add 'bandwidth-weights' later
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit 53f16d8546302621f786c30ef0fb1b69786444fe Author: Damian Johnson <atagar(a)torproject.org> Date: Sun Sep 16 18:10:08 2012 -0700 Noting that we might want to add 'bandwidth-weights' later The "bandwidth-weights" field mentions a 'bandwidth-weights' parameter and a default value. This isn't listed in the params section so getting confirmation of what I should be making of this param first. --- stem/descriptor/networkstatus.py | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index 0a4aaeb..ae52c2f 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -90,6 +90,9 @@ FOOTER_STATUS_DOCUMENT_FIELDS = ( ALL_FIELDS = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS + FOOTER_STATUS_DOCUMENT_FIELDS] +# Maybe we should add 'bandwidth-weights'? +# https://trac.torproject.org/6872 + DEFAULT_PARAMS = { "cbtdisabled": 0, "cbtnummodes": 3,

1 0

[stem/master] Breaking up the header and footer from NetworkStatusDocument
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit 641bed527c019a92e47dd769bca23fa04411d9c3 Author: Damian Johnson <atagar(a)torproject.org> Date: Fri Sep 21 09:00:19 2012 -0700 Breaking up the header and footer from NetworkStatusDocument The NetworkStatusDocument class was starting to get monsterous, and it was just gonna get worse. A network status document consists of four sections... - header - authorities - router status entries - footer Making the NetworkStatusDocument a thin container for these four, and making separate classes for them. This has made the code much nicer. The only disadvantage that I've seen is that validation is done in pieces so if, for instance, you're missing mandatory fields from both the header and footer you now won't be told about both in a single error message. Instead the header will be parsed first, fail, and just tell you about those. That said, this is a pretty minor regression and well worth the improved maintainability. --- stem/descriptor/networkstatus.py | 428 ++++++++++++++++++++------------------ 1 files changed, 223 insertions(+), 205 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index a7bca7a..33b0440 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -85,7 +85,8 @@ FOOTER_STATUS_DOCUMENT_FIELDS = ( ("directory-signature", True, True, True), ) -ALL_FIELDS = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS + FOOTER_STATUS_DOCUMENT_FIELDS] +HEADER_FIELDS = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS] +FOOTER_FIELDS = [attr[0] for attr in FOOTER_STATUS_DOCUMENT_FIELDS] DEFAULT_PARAMS = { "bwweightscale": 10000, @@ -243,32 +244,21 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): super(NetworkStatusDocument, self).__init__(raw_content) self.directory_authorities = [] - self.signatures = [] - - self.version = None - self.is_consensus = True - self.is_vote = False - self.consensus_methods = [] - self.published = None - self.consensus_method = None - self.valid_after = None - self.fresh_until = None - self.valid_until = None - self.vote_delay = None - self.dist_delay = None - self.client_versions = [] - self.server_versions = [] - self.known_flags = [] - self.params = dict(DEFAULT_PARAMS) if default_params else {} - self.bandwidth_weights = {} - self._unrecognized_lines = [] document_file = StringIO(raw_content) - header, footer, routers_end = _get_document_content(document_file, validate) + header_content, footer_content, routers_end = _get_document_content(document_file, validate) + + self._header = _DocumentHeader(header_content, validate, default_params) + self._footer = _DocumentFooter(footer_content, validate, self._header) + + for attr, value in vars(self._header).items() + vars(self._footer).items(): + if attr != "_unrecognized_lines": + setattr(self, attr, value) + else: + self._unrecognized_lines += value - self._parse(header, footer, validate) - self._parse_old(header + footer, validate) + self._parse_old(header_content + footer_content, validate) if document_file.tell() < routers_end: self.routers = tuple(_get_routers(document_file, validate, self, routers_end, self._get_router_type())) @@ -289,36 +279,82 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): :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)) + return self._header.meets_consensus_method(method) def get_unrecognized_lines(self): return list(self._unrecognized_lines) - def _parse(self, header, footer, validate): - """ - Parses the given content and applies the attributes. + def _parse_old(self, raw_content, validate): + # preamble + content = StringIO(raw_content) + read_keyword_line = lambda keyword, optional = False: setattr(self, keyword.replace("-", "_"), _read_keyword_line(keyword, content, validate, optional)) - :param str header_content: content of the descriptor header - :param str footer_content: content of the descriptor footer - :param bool validate: checks validity if True + # ignore things the parse() method handles + _read_keyword_line("network-status-version", content, False, True) + _read_keyword_line("vote-status", content, False, True) + _read_keyword_line("consensus-methods", content, False, True) + _read_keyword_line("consensus-method", content, False, True) + _read_keyword_line("published", content, False, True) + _read_keyword_line("valid-after", content, False, True) + _read_keyword_line("fresh-until", content, False, True) + _read_keyword_line("valid-until", content, False, True) + _read_keyword_line("voting-delay", content, False, True) + _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) - :raises: ValueError if a validity check fails - """ + # 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) + dirauth_data = "".join(dirauth_data).rstrip() + self.directory_authorities.append(DirectoryAuthority(dirauth_data, self.is_vote, validate)) + + _read_keyword_line("directory-footer", content, False, True) + _read_keyword_line("bandwidth-weights", content, False, True) + _read_keyword_line("directory-signature", content, False, True) + +class _DocumentHeader(object): + def __init__(self, content, validate, default_params): + self.version = None + self.is_consensus = True + self.is_vote = False + self.consensus_methods = [] + self.published = None + self.consensus_method = None + self.valid_after = None + self.fresh_until = None + self.valid_until = None + self.vote_delay = None + self.dist_delay = None + self.client_versions = [] + self.server_versions = [] + self.known_flags = [] + self.params = dict(DEFAULT_PARAMS) if default_params else {} + + self._unrecognized_lines = [] - header_entries = stem.descriptor._get_descriptor_components(header, validate)[0] - footer_entries = stem.descriptor._get_descriptor_components(footer, validate)[0] + entries = stem.descriptor._get_descriptor_components(content, validate)[0] + self._parse(entries, validate) - for keyword, values in header_entries.items() + footer_entries.items(): + # doing this validation afterward so we know our 'is_consensus' and + # 'is_vote' attributes + + if validate: + _check_for_missing_and_disallowed_fields(self, entries, HEADER_STATUS_DOCUMENT_FIELDS) + _check_for_misordered_fields(entries, HEADER_FIELDS) + + def meets_consensus_method(self, method): + return bool(self.consensus_method >= method or filter(lambda x: x >= method, self.consensus_methods)) + + def _parse(self, entries, validate): + for keyword, values in entries.items(): value, block_contents = values[0] line = "%s %s" % (keyword, value) - # All known fields can only appear once except... - # * 'directory-signature' in a consensus - - if validate and len(values) > 1 and keyword in ALL_FIELDS: - if not (keyword == 'directory-signature' and is_consensus): - content = header + '\n' + footer - raise ValueError("Network status documents can only have a single '%s' line, got %i:\n%s" % (keyword, len(values), content)) + # all known header fields can only appear once except + if validate and len(values) > 1 and keyword in HEADER_FIELDS: + raise ValueError("Network status documents can only have a single '%s' line, got %i" % (keyword, len(values))) if keyword == 'network-status-version': # "network-status-version" version @@ -421,144 +457,12 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): # skip if this is a blank line if value == "": continue - self.params.update(self._parse_int_mappings(keyword, value, validate)) + self.params.update(_parse_int_mappings(keyword, value, validate)) 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) - elif keyword == "bandwidth-weights": - self.bandwidth_weights = self._parse_int_mappings(keyword, value, validate) - - if validate: - weight_keys = tuple(sorted(self.bandwidth_weights.keys())) - - if weight_keys != BANDWIDTH_WEIGHT_ENTRIES: - expected_label = ', '.join(BANDWIDTH_WEIGHT_ENTRIES) - actual_label = ', '.join(weight_keys) - - raise ValueError("A network status document's 'bandwidth-weights' entries should be '%s', got '%s'" % (expected_label, actual_label)) - elif keyword == "directory-signature": - if not " " in value or not block_contents: - if not validate: continue - raise ValueError("Authority signatures in a network status document are expected to be of the form 'directory-signature FINGERPRINT KEY_DIGEST\\nSIGNATURE', got:\n%s" % line) - - fingerprint, key_digest = value.split(" ", 1) - self.signatures.append(DocumentSignature(fingerprint, key_digest, block_contents, validate)) else: self._unrecognized_lines.append(line) - - # 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 - content = StringIO(raw_content) - read_keyword_line = lambda keyword, optional = False: setattr(self, keyword.replace("-", "_"), _read_keyword_line(keyword, content, validate, optional)) - - # ignore things the parse() method handles - _read_keyword_line("network-status-version", content, False, True) - _read_keyword_line("vote-status", content, False, True) - _read_keyword_line("consensus-methods", content, False, True) - _read_keyword_line("consensus-method", content, False, True) - _read_keyword_line("published", content, False, True) - _read_keyword_line("valid-after", content, False, True) - _read_keyword_line("fresh-until", content, False, True) - _read_keyword_line("valid-until", content, False, True) - _read_keyword_line("voting-delay", content, False, True) - _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 - - # 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) - dirauth_data = "".join(dirauth_data).rstrip() - self.directory_authorities.append(DirectoryAuthority(dirauth_data, vote, validate)) - - _read_keyword_line("directory-footer", content, False, True) - _read_keyword_line("bandwidth-weights", content, False, True) - _read_keyword_line("directory-signature", content, False, True) - - 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 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 - """ - - missing_fields, disallowed_fields = [], [] - - if not self.meets_consensus_method(9): - # footers only appear in consensus-method 9 or later - if footer_entries: - raise ValueError("Network status document's footer should only apepar in consensus-method 9 or later") - else: - # pretend to have mandatory fields to prevent validation from whining - footer_entries = {"directory-footer": "", "directory-signature": ""} - - 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)): - # mandatory field, check that we have it - if not field in entries.keys(): - missing_fields.append(field) - 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) - - if missing_fields: - 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 or version: %s" % ', '.join(disallowed_fields)) - - 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 - are ignored). - """ - - # Earlier validation has ensured that our fields either belong to our - # document type or are unknown. Remove the unknown fields since they - # reflect a spec change and can appear anywhere in the document. - - expected_header = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS] - expected_footer = [attr[0] for attr in FOOTER_STATUS_DOCUMENT_FIELDS] - - actual_header = filter(lambda field: field in expected_header, header_entries.keys()) - actual_footer = filter(lambda field: field in expected_footer, footer_entries.keys()) - - # Narrow the expected_header and expected_footer to just what we have. If - # the lists then match then the order's valid. - - expected_header = filter(lambda field: field in actual_header, expected_header) - expected_footer = filter(lambda field: field in actual_footer, expected_footer) - - for label, actual, expected in (('header', actual_header, expected_header), - ('footer', actual_footer, expected_footer)): - if actual != expected: - actual_label = ', '.join(actual) - expected_label = ', '.join(expected) - raise ValueError("The fields in the document's %s are misordered. It should be '%s' but was '%s'" % (label, actual_label, expected_label)) def _check_params_constraints(self): """ @@ -602,42 +506,156 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): if value < minimum or value > maximum: raise ValueError("'%s' value on the params line must be in the range of %i - %i, was %i" % (key, minimum, maximum, value)) - - def _parse_int_mappings(self, keyword, value, validate): - # Parse a series of 'key=value' entries, checking the following: - # - values are integers - # - keys are sorted in lexical order + +class _DocumentFooter(object): + def __init__(self, content, validate, header): + self.signatures = [] + self.bandwidth_weights = {} - results, seen_keys = {}, [] - for entry in value.split(" "): - try: - if not '=' in entry: - raise ValueError("must only have 'key=value' entries") + self._unrecognized_lines = [] + + if validate and content and not header.meets_consensus_method(9): + raise ValueError("Network status document's footer should only apepar in consensus-method 9 or later") + elif not content and not header.meets_consensus_method(9): + return # footer is optional and there's nothing to parse + + entries = stem.descriptor._get_descriptor_components(content, validate)[0] + self._parse(entries, validate, header) + + if validate: + _check_for_missing_and_disallowed_fields(header, entries, FOOTER_STATUS_DOCUMENT_FIELDS) + _check_for_misordered_fields(entries, FOOTER_FIELDS) + + def _parse(self, entries, validate, header): + for keyword, values in entries.items(): + value, block_contents = values[0] + line = "%s %s" % (keyword, value) + + # all known footer fields can only appear once except... + # * 'directory-signature' in a consensus + + if validate and len(values) > 1 and keyword in FOOTER_FIELDS: + if not (keyword == 'directory-signature' and header.is_consensus): + raise ValueError("Network status documents can only have a single '%s' line, got %i" % (keyword, len(values))) + + if keyword == "directory-footer": + # nothing to parse, simply checking that we don't have a value - entry_key, entry_value = entry.split("=", 1) + if validate and value: + raise ValueError("A network status document's 'directory-footer' line shouldn't have any content, got '%s'" % line) + elif keyword == "bandwidth-weights": + self.bandwidth_weights = _parse_int_mappings(keyword, value, validate) - try: - # the int() function accepts things like '+123', but we don't want to - if entry_value.startswith('+'): - raise ValueError() + if validate: + weight_keys = tuple(sorted(self.bandwidth_weights.keys())) - entry_value = int(entry_value) - except ValueError: - raise ValueError("'%s' is a non-numeric value" % entry_value) + if weight_keys != BANDWIDTH_WEIGHT_ENTRIES: + expected_label = ', '.join(BANDWIDTH_WEIGHT_ENTRIES) + actual_label = ', '.join(weight_keys) + + raise ValueError("A network status document's 'bandwidth-weights' entries should be '%s', got '%s'" % (expected_label, actual_label)) + elif keyword == "directory-signature": + if not " " in value or not block_contents: + if not validate: continue + raise ValueError("Authority signatures in a network status document are expected to be of the form 'directory-signature FINGERPRINT KEY_DIGEST\\nSIGNATURE', got:\n%s" % line) - if validate: - # parameters should be in ascending order by their key - for prior_key in seen_keys: - if prior_key > entry_key: - raise ValueError("parameters must be sorted by their key") + fingerprint, key_digest = value.split(" ", 1) + self.signatures.append(DocumentSignature(fingerprint, key_digest, block_contents, validate)) + +def _check_for_missing_and_disallowed_fields(header, entries, fields): + """ + 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 _DocumentHeader header: document header + :param dict entries: ordered keyword/value mappings of the header or footer + :param list fields: expected field attributes (either HEADER_STATUS_DOCUMENT_FIELDS or FOOTER_STATUS_DOCUMENT_FIELDS) + + :raises: ValueError if we're missing mandatory fields or have fiels we shouldn't + """ + + missing_fields, disallowed_fields = [], [] + + for field, in_votes, in_consensus, mandatory in fields: + if mandatory and ((header.is_consensus and in_consensus) or (header.is_vote and in_votes)): + # mandatory field, check that we have it + if not field in entries.keys(): + missing_fields.append(field) + elif (header.is_consensus and not in_consensus) or (header.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) + + if missing_fields: + 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 or version: %s" % ', '.join(disallowed_fields)) + +def _check_for_misordered_fields(entries, expected): + """ + 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 + are ignored). + + :param dict entries: ordered keyword/value mappings of the header or footer + :param list expected: ordered list of expected fields (either HEADER_FIELDS or FOOTER_FIELDS) + + :raises: ValueError if entries aren't properly ordered + """ + + # Earlier validation has ensured that our fields either belong to our + # document type or are unknown. Remove the unknown fields since they + # reflect a spec change and can appear anywhere in the document. + + actual = filter(lambda field: field in expected, entries.keys()) + + # Narrow the expected to just what we have. If the lists then match then the + # order's valid. + + expected = filter(lambda field: field in actual, expected) + + if actual != expected: + actual_label = ', '.join(actual) + expected_label = ', '.join(expected) + raise ValueError("The fields in a section of the document are misordered. It should be '%s' but was '%s'" % (actual_label, expected_label)) + +def _parse_int_mappings(keyword, value, validate): + # Parse a series of 'key=value' entries, checking the following: + # - values are integers + # - keys are sorted in lexical order + + results, seen_keys = {}, [] + 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() - results[entry_key] = entry_value - seen_keys.append(entry_key) - except ValueError, exc: - if not validate: continue - raise ValueError("Unable to parse network status document's '%s' line (%s): %s'" % (keyword, exc, value)) - - return results + entry_value = int(entry_value) + except ValueError: + raise ValueError("'%s' is a non-numeric value" % entry_value) + + if validate: + # parameters should be in ascending order by their key + for prior_key in seen_keys: + if prior_key > entry_key: + raise ValueError("parameters must be sorted by their key") + + results[entry_key] = entry_value + seen_keys.append(entry_key) + except ValueError, exc: + if not validate: continue + raise ValueError("Unable to parse network status document's '%s' line (%s): %s'" % (keyword, exc, value)) + + return results class DirectoryAuthority(stem.descriptor.Descriptor): """

1 0

[stem/master] Parsing the directory-signature and unrecognized lines
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit 1f868090e2d641ddcb49d02bd15b5894f5bf6923 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Sep 18 09:36:56 2012 -0700 Parsing the directory-signature and unrecognized lines Finishing up with the footer. It doesn't make sense for the DirectorySignature or DirectoryAuthority to be Descriptor subclasses (cuz... well, they aren't descriptors). However, I like having this struct class rather than providing our callers with a tuple list. I should probably do this for other descriptor documents too... --- stem/descriptor/networkstatus.py | 111 ++++++++++-------------- test/integ/descriptor/networkstatus.py | 16 ++-- test/unit/descriptor/networkstatus/document.py | 59 ++++++++++--- 3 files changed, 98 insertions(+), 88 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index e142728..a7bca7a 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -36,7 +36,7 @@ The documents can be obtained from any of the following sources... +- MicrodescriptorConsensus - Microdescriptor flavoured consensus documents RouterStatusEntry - Router descriptor; contains information about a Tor relay +- RouterMicrodescriptor - Router microdescriptor; contains information that doesn't change frequently - DirectorySignature - Network status document's directory signature + DocumentSignature - Signature of a document by a directory authority DirectoryAuthority - Directory authority defined in a v3 network status document """ @@ -216,7 +216,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): :var list known_flags: **\*** list of known router flags :var list params: **\*** dict of parameter(str) => value(int) mappings :var list directory_authorities: **\*** list of DirectoryAuthority objects that have generated this document - :var list directory_signatures: **\*** list of signatures this document has + :var list signatures: **\*** DocumentSignature of the authorities that have signed the document **Consensus Attributes:** :var int consensus_method: method version used to generate this consensus @@ -243,7 +243,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): super(NetworkStatusDocument, self).__init__(raw_content) self.directory_authorities = [] - self.directory_signatures = [] + self.signatures = [] self.version = None self.is_consensus = True @@ -262,6 +262,8 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): self.params = dict(DEFAULT_PARAMS) if default_params else {} self.bandwidth_weights = {} + self._unrecognized_lines = [] + document_file = StringIO(raw_content) header, footer, routers_end = _get_document_content(document_file, validate) @@ -290,13 +292,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): 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. - - :returns: a list of unrecognized trailing lines - """ - - return self.unrecognized_lines + return list(self._unrecognized_lines) def _parse(self, header, footer, validate): """ @@ -445,6 +441,15 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): actual_label = ', '.join(weight_keys) raise ValueError("A network status document's 'bandwidth-weights' entries should be '%s', got '%s'" % (expected_label, actual_label)) + elif keyword == "directory-signature": + if not " " in value or not block_contents: + if not validate: continue + raise ValueError("Authority signatures in a network status document are expected to be of the form 'directory-signature FINGERPRINT KEY_DIGEST\\nSIGNATURE', got:\n%s" % line) + + fingerprint, key_digest = value.split(" ", 1) + self.signatures.append(DocumentSignature(fingerprint, key_digest, block_contents, validate)) + else: + self._unrecognized_lines.append(line) # doing this validation afterward so we know our 'is_consensus' and # 'is_vote' attributes @@ -483,17 +488,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): _read_keyword_line("directory-footer", content, False, True) _read_keyword_line("bandwidth-weights", content, False, True) - - while _peek_keyword(content) == "directory-signature": - signature_data = _read_until_keywords("directory-signature", content, False, True) - self.directory_signatures.append(DirectorySignature("".join(signature_data))) - - remainder = content.read() - - if remainder: - self.unrecognized_lines = remainder.split("\n") - else: - self.unrecognized_lines = [] + _read_keyword_line("directory-signature", content, False, True) def _check_for_missing_and_disallowed_fields(self, header_entries, footer_entries): """ @@ -706,60 +701,44 @@ class DirectoryAuthority(stem.descriptor.Descriptor): return self.unrecognized_lines -class DirectorySignature(stem.descriptor.Descriptor): +# TODO: microdescriptors have a slightly different format (including a +# 'method') - should probably be a subclass +class DocumentSignature(object): """ - Contains directory signatures in a v3 network status document. + Directory signature of a v3 network status document. - :var str identity: signature identity - :var str key_digest: signature key digest - :var str method: method used to generate the signature - :var str signature: the signature data - """ + :var str identity: fingerprint of the authority that made the signature + :var str key_digest: digest of the signing key + :var str signature: document signature + :param bool validate: checks validity if True - def __init__(self, raw_content, validate = True): - """ - Parse a directory signature entry in a v3 network status document and - provide a DirectorySignature object. - - :param str raw_content: raw directory signature entry information - :param bool validate: True if the document is to be validated, False otherwise - - :raises: ValueError if the raw data is invalid - """ - - super(DirectorySignature, self).__init__(raw_content) - self.identity, self.key_digest, self.method, self.signature = None, None, None, None - content = raw_content.splitlines() - - signature_line = _read_keyword_line_str("directory-signature", content, validate).split(" ") - - if len(signature_line) == 2: - self.identity, self.key_digest = signature_line - if len(signature_line) == 3: - # for microdescriptor consensuses - # This 'method' seems to be undocumented 8-8-12 - self.method, self.identity, self.key_digest = signature_line - - self.signature = _get_pseudo_pgp_block(content) - self.unrecognized_lines = content - if self.unrecognized_lines and validate: - raise ValueError("Unrecognized trailing data in directory signature") + :raises: ValueError if a validity check fails + """ - def get_unrecognized_lines(self): - """ - Returns any unrecognized lines. + def __init__(self, identity, key_digest, signature, validate = True): + # Checking that these attributes are valid. Technically the key + # digest isn't a fingerprint, but it has the same characteristics. - :returns: a list of unrecognized lines - """ + if validate: + if not stem.util.tor_tools.is_valid_fingerprint(identity): + raise ValueError("Malformed fingerprint (%s) in the document signature" % (identity)) + + if not stem.util.tor_tools.is_valid_fingerprint(key_digest): + raise ValueError("Malformed key digest (%s) in the document signature" % (key_digest)) - return self.unrecognized_lines + self.identity = identity + self.key_digest = key_digest + self.signature = signature def __cmp__(self, other): - if not isinstance(other, DirectorySignature): + if not isinstance(other, DocumentSignature): return 1 - # attributes are all derived from content, so we can simply use that to check - return str(self) > str(other) + for attr in ("identity", "key_digest", "signature"): + if getattr(self, attr) > getattr(other, attr): return 1 + elif getattr(self, attr) < getattr(other, attr): return -1 + + return 0 class RouterStatusEntry(stem.descriptor.Descriptor): """ @@ -1033,7 +1012,7 @@ class MicrodescriptorConsensus(NetworkStatusDocument): :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 + :var list signatures: **\*** list of signatures this document has | **\*** attribute is either required when we're parsed with validation or has a default value, others are left as None if undefined | **~** attribute appears only in consensuses diff --git a/test/integ/descriptor/networkstatus.py b/test/integ/descriptor/networkstatus.py index d77a36f..aa91ab3 100644 --- a/test/integ/descriptor/networkstatus.py +++ b/test/integ/descriptor/networkstatus.py @@ -143,10 +143,10 @@ HFXB4497LzESysYJ/4jJY83E5vLjhv+igIxD9LU6lf6ftkGeF+lNmIAIEKaMts8H mfWcW0b+jsrXcJoCxV5IrwCDF3u1aC3diwZY6yiG186pwWbOwE41188XI2DeYPwE I/TJmV928na7RLZe2mGHCAW3VQOvV+QkCfj05VZ8CsY= -----END SIGNATURE-----""" - self.assertEquals(8, len(desc.directory_signatures)) - self.assertEquals("14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4", desc.directory_signatures[0].identity) - self.assertEquals("BF112F1C6D5543CFD0A32215ACABD4197B5279AD", desc.directory_signatures[0].key_digest) - self.assertEquals(expected_signature, desc.directory_signatures[0].signature) + self.assertEquals(8, len(desc.signatures)) + self.assertEquals("14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4", desc.signatures[0].identity) + self.assertEquals("BF112F1C6D5543CFD0A32215ACABD4197B5279AD", desc.signatures[0].key_digest) + self.assertEquals(expected_signature, desc.signatures[0].signature) def test_metrics_vote(self): """ @@ -261,10 +261,10 @@ fskXN84wB3mXfo+yKGSt0AcDaaPuU3NwMR3ROxWgLN0KjAaVi2eV9PkPCsQkcgw3 JZ/1HL9sHyZfo6bwaC6YSM9PNiiY6L7rnGpS7UkHiFI+M96VCMorvjm5YPs3FioJ DnN5aFtYKiTc19qIC7Nmo+afPdDEf0MlJvEOP5EWl3w= -----END SIGNATURE-----""" - self.assertEquals(1, len(desc.directory_signatures)) - self.assertEquals("27B6B5996C426270A5C95488AA5BCEB6BCC86956", desc.directory_signatures[0].identity) - self.assertEquals("D5C30C15BB3F1DA27669C2D88439939E8F418FCF", desc.directory_signatures[0].key_digest) - self.assertEquals(expected_signature, desc.directory_signatures[0].signature) + self.assertEquals(1, len(desc.signatures)) + self.assertEquals("27B6B5996C426270A5C95488AA5BCEB6BCC86956", desc.signatures[0].identity) + self.assertEquals("D5C30C15BB3F1DA27669C2D88439939E8F418FCF", desc.signatures[0].key_digest) + self.assertEquals(expected_signature, desc.signatures[0].signature) def test_cached_microdesc_consensus(self): """ diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py index 8ed1f0e..a0fe3a8 100644 --- a/test/unit/descriptor/networkstatus/document.py +++ b/test/unit/descriptor/networkstatus/document.py @@ -7,7 +7,16 @@ import unittest import stem.version from stem.descriptor import Flag -from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, DEFAULT_PARAMS, BANDWIDTH_WEIGHT_ENTRIES, NetworkStatusDocument, DirectorySignature +from stem.descriptor.networkstatus import HEADER_STATUS_DOCUMENT_FIELDS, FOOTER_STATUS_DOCUMENT_FIELDS, DEFAULT_PARAMS, BANDWIDTH_WEIGHT_ENTRIES, NetworkStatusDocument, DocumentSignature + +sig_block = """\ +-----BEGIN SIGNATURE----- +e1XH33ITaUYzXu+dK04F2dZwR4PhcOQgIuK859KGpU77/6lRuggiX/INk/4FJanJ +ysCTE1K4xk4fH3N1Tzcv/x/gS4LUlIZz3yKfBnj+Xh3w12Enn9V1Gm1Vrhl+/YWH +eweONYRZTTvgsB+aYsCoBuoBBpbr4Swlu64+85F44o4= +-----END SIGNATURE-----""" + +SIG = DocumentSignature("14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4", "BF112F1C6D5543CFD0A32215ACABD4197B5279AD", sig_block) NETWORK_STATUS_DOCUMENT_ATTR = { "network-status-version": "3", @@ -21,16 +30,9 @@ NETWORK_STATUS_DOCUMENT_ATTR = { "voting-delay": "300 300", "known-flags": "Authority BadExit Exit Fast Guard HSDir Named Running Stable Unnamed V2Dir Valid", "directory-footer": "", - "directory-signature": "\n".join(( - "14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4 BF112F1C6D5543CFD0A32215ACABD4197B5279AD", - "-----BEGIN SIGNATURE-----", - "e1XH33ITaUYzXu+dK04F2dZwR4PhcOQgIuK859KGpU77/6lRuggiX/INk/4FJanJ", - "ysCTE1K4xk4fH3N1Tzcv/x/gS4LUlIZz3yKfBnj+Xh3w12Enn9V1Gm1Vrhl+/YWH", - "eweONYRZTTvgsB+aYsCoBuoBBpbr4Swlu64+85F44o4=", - "-----END SIGNATURE-----")), + "directory-signature": "%s %s\n%s" % (SIG.identity, SIG.key_digest, SIG.signature), } -SIG = DirectorySignature("directory-signature " + NETWORK_STATUS_DOCUMENT_ATTR["directory-signature"]) def get_network_status_document(attr = None, exclude = None, routers = None): """ @@ -119,7 +121,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.signatures) self.assertEqual([], document.get_unrecognized_lines()) def test_minimal_vote(self): @@ -151,7 +153,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.signatures) self.assertEqual([], document.get_unrecognized_lines()) def test_missing_fields(self): @@ -170,6 +172,15 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertRaises(ValueError, NetworkStatusDocument, content) NetworkStatusDocument(content, False) # constructs without validation + def test_unrecognized_line(self): + """ + Includes unrecognized content in the document. + """ + + content = get_network_status_document({"pepperjack": "is oh so tasty!"}) + document = NetworkStatusDocument(content) + self.assertEquals(["pepperjack is oh so tasty!"], document.get_unrecognized_lines()) + def test_misordered_fields(self): """ Rearranges our descriptor fields. @@ -533,14 +544,14 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertRaises(ValueError, NetworkStatusDocument, content) document = NetworkStatusDocument(content, False) - self.assertEqual([SIG], document.directory_signatures) + self.assertEqual([SIG], document.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.signatures) self.assertEqual([], document.get_unrecognized_lines()) def test_footer_with_value(self): @@ -552,7 +563,7 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertRaises(ValueError, NetworkStatusDocument, content) document = NetworkStatusDocument(content, False) - self.assertEqual([SIG], document.directory_signatures) + self.assertEqual([SIG], document.signatures) self.assertEqual([], document.get_unrecognized_lines()) def test_bandwidth_wights_ok(self): @@ -649,4 +660,24 @@ class TestNetworkStatusDocument(unittest.TestCase): document = NetworkStatusDocument(content, False) self.assertEquals(expected, document.bandwidth_weights) + + def test_malformed_signature(self): + """ + Provides malformed or missing content in the 'directory-signature' line. + """ + + test_values = ( + "", + "\n", + "blarg", + ) + + for test_value in test_values: + for test_attr in xrange(3): + attrs = [SIG.identity, SIG.key_digest, SIG.signature] + attrs[test_attr] = test_value + + content = get_network_status_document({"directory-signature": "%s %s\n%s" % tuple(attrs)}) + self.assertRaises(ValueError, NetworkStatusDocument, content) + NetworkStatusDocument(content, False) # checks that it's still parseable without validation

1 0

[stem/master] Getting rid of the _get_document_content() helper
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit e4185194801b6afd10671e6e7a1a53977a598c29 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Sep 22 13:18:16 2012 -0700 Getting rid of the _get_document_content() helper Too many helper functions and the code becomes fragmented. The _get_document_content() was only used a couple places, and both of those were actually better with their own slightly different implementations. --- stem/descriptor/networkstatus.py | 90 +++++++++++++------------------------ 1 files changed, 32 insertions(+), 58 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index f279805..030d413 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -131,59 +131,28 @@ def parse_file(document_file, validate = True, is_microdescriptor = False): * IOError if the file can't be read """ - header, authorities, footer, routers_end = _get_document_content(document_file, validate) - document_data = header + authorities + footer + # getting the document without the routers section - if not is_microdescriptor: - document = NetworkStatusDocument(document_data, validate) - router_type = RouterStatusEntry - else: - document = MicrodescriptorConsensus(document_data, validate) - router_type = RouterMicrodescriptor - - for desc in _get_routers(document_file, validate, document, routers_end, router_type): - yield desc - -def _get_document_content(document_file, validate): - """ - Network status documents consist of four sections: - * header - * authority entries - * router entries - * footer - - This provides back a tuple with the following... - (header, authorities, footer, routers_end) - - This leaves the document_file at the start of the router entries. - - :param file document_file: file with network status document content - :param bool validate: checks the validity of the document's contents if True, skips these checks otherwise - - :returns: tuple with the network status document content and ending position of the routers - - :raises: - * ValueError if the contents is malformed and validate is True - * IOError if the file can't be read - """ - - # parse until the first record of a following section - header = _read_until_keywords((AUTH_START, ROUTERS_START, FOOTER_START), document_file) - authorities = _read_until_keywords((ROUTERS_START, FOOTER_START), document_file) + header = _read_until_keywords((ROUTERS_START, FOOTER_START), document_file) - # skip router section, just taking note of the position routers_start = document_file.tell() _read_until_keywords(FOOTER_START, document_file, skip = True) routers_end = document_file.tell() footer = document_file.readlines() + document_content = header + footer - # leave our position at the start of the router section - document_file.seek(routers_start) + if not is_microdescriptor: + document = NetworkStatusDocument(document_content, validate) + router_type = RouterStatusEntry + else: + document = MicrodescriptorConsensus(document_content, validate) + router_type = RouterMicrodescriptor - return ("".join(header), "".join(authorities), "".join(footer), routers_end) + for desc in _get_routers(document_file, validate, document, routers_start, routers_end, router_type): + yield desc -def _get_routers(document_file, validate, document, end_position, router_type): +def _get_routers(document_file, validate, document, start_position, end_position, router_type): """ Iterates over the router entries in a given document. The document_file is expected to be at the start of the router section and the end_position @@ -192,7 +161,8 @@ def _get_routers(document_file, validate, document, end_position, router_type): :param file document_file: file with network status document content :param bool validate: checks the validity of the document's contents if True, skips these checks otherwise :param object document: document the descriptors originate from - :param int end_position: location in the document_file where the router section ends + :param int start_position: start of the routers section + :param int end_position: end of the routers section :param class router_type: router class to construct :returns: iterator over router_type instances @@ -202,15 +172,16 @@ def _get_routers(document_file, validate, document, end_position, router_type): * IOError if the file can't be read """ + document_file.seek(start_position) while document_file.tell() < end_position: desc_content = "".join(_read_until_keywords("r", document_file, ignore_first = True, end_position = end_position)) yield router_type(desc_content, document, validate) -def _get_authorities(authority_lines, is_vote, validate): +def _get_authorities(authorities, is_vote, validate): """ Iterates over the authoritiy entries in given content. - :param list authority_lines: lines of content to be parsed + :param str authority_lines: content of the authorities section :param bool is_vote: indicates if this is for a vote or contensus document :param bool validate: True if the document is to be validated, False otherwise @@ -221,7 +192,7 @@ def _get_authorities(authority_lines, is_vote, validate): auth_buffer = [] - for line in authority_lines: + for line in authorities.split("\n"): if not line: continue elif line.startswith(AUTH_START) and auth_buffer: yield DirectoryAuthority("\n".join(auth_buffer), is_vote, validate) @@ -276,27 +247,30 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): """ super(NetworkStatusDocument, self).__init__(raw_content) + document_file = StringIO(raw_content) - self.directory_authorities = [] - self._unrecognized_lines = [] + header = _read_until_keywords((AUTH_START, ROUTERS_START, FOOTER_START), document_file) + self._header = _DocumentHeader("".join(header), validate, default_params) - document_file = StringIO(raw_content) - header_content, authority_content, footer_content, routers_end = _get_document_content(document_file, validate) + authorities = _read_until_keywords((ROUTERS_START, FOOTER_START), document_file) + self.directory_authorities = list(_get_authorities("".join(authorities), self._header.is_vote, validate)) - self._header = _DocumentHeader(header_content, validate, default_params) - self._footer = _DocumentFooter(footer_content, validate, self._header) - self.directory_authorities = list(_get_authorities(authority_content.split("\n"), self._header.is_vote, validate)) + routers_start = document_file.tell() + _read_until_keywords(FOOTER_START, document_file, skip = True) + routers_end = document_file.tell() + self._footer = _DocumentFooter(document_file.read(), validate, self._header) + + self._unrecognized_lines = [] + + # copy the header and footer attributes into us for attr, value in vars(self._header).items() + vars(self._footer).items(): if attr != "_unrecognized_lines": setattr(self, attr, value) else: self._unrecognized_lines += value - if document_file.tell() < routers_end: - self.routers = tuple(_get_routers(document_file, validate, self, routers_end, self._get_router_type())) - else: - self.routers = () + self.routers = tuple(_get_routers(document_file, validate, self, routers_start, routers_end, self._get_router_type())) def _get_router_type(self): return RouterStatusEntry

1 0

[stem/master] Adding bwweightscale parameter constraints
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit ea3102387729daa17587f12b47f249ff49baf1bd Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Sep 18 08:03:12 2012 -0700 Adding bwweightscale parameter constraints My spec fix to clarify the parameter was merged [1] so enforcing the default and constraints in our parser. [1] https://trac.torproject.org/6872 --- stem/descriptor/networkstatus.py | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index 2061bd5..e142728 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -87,10 +87,8 @@ FOOTER_STATUS_DOCUMENT_FIELDS = ( ALL_FIELDS = [attr[0] for attr in HEADER_STATUS_DOCUMENT_FIELDS + FOOTER_STATUS_DOCUMENT_FIELDS] -# Maybe we should add 'bandwidth-weights'? -# https://trac.torproject.org/6872 - DEFAULT_PARAMS = { + "bwweightscale": 10000, "cbtdisabled": 0, "cbtnummodes": 3, "cbtrecentcount": 20, @@ -584,6 +582,8 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): minimum = 1 elif key == "refuseunknownexits": minimum, maximum = 0, 1 + elif key == "bwweightscale": + minimum = 1 elif key == "cbtdisabled": minimum, maximum = 0, 1 elif key == "cbtnummodes":

1 0

[stem/master] Generalizing how router entries and authorities are parsed
by atagar＠torproject.org 13 Oct '12

13 Oct '12

commit a5babce203784eb45f7c585edbcee4caf2362212 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Sep 22 14:12:21 2012 -0700 Generalizing how router entries and authorities are parsed The _get_routers() and _get_authorities() were essentially doing the same thing. Replacing both with a more general _get_entries() helper that reads a range of the document and constructs instances for it. Taking advantage of this nicer helper's keyword arguments to make the code more readable (functions that take a ton of positional args are is a pita). I'm a bit surprised (and concerned) at how easily this passed unit tests. No doubt I've broken the integ tests but I'm not putting any effort there until I've finished the document parser rewrite. --- stem/descriptor/networkstatus.py | 119 +++++++++++++----------- test/unit/descriptor/networkstatus/document.py | 4 +- test/unit/descriptor/networkstatus/entry.py | 22 ++-- 3 files changed, 76 insertions(+), 69 deletions(-) diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py index 030d413..8774b95 100644 --- a/stem/descriptor/networkstatus.py +++ b/stem/descriptor/networkstatus.py @@ -149,59 +149,58 @@ def parse_file(document_file, validate = True, is_microdescriptor = False): document = MicrodescriptorConsensus(document_content, validate) router_type = RouterMicrodescriptor - for desc in _get_routers(document_file, validate, document, routers_start, routers_end, router_type): + desc_iterator = _get_entries( + document_file, + validate, + entry_class = router_type, + entry_keyword = ROUTERS_START, + start_position = routers_start, + end_position = routers_end, + extra_args = (document,), + ) + + for desc in desc_iterator: yield desc -def _get_routers(document_file, validate, document, start_position, end_position, router_type): +def _get_entries(document_file, validate, entry_class, entry_keyword, start_position = None, end_position = None, section_end_keywords = (), extra_args = ()): """ - Iterates over the router entries in a given document. The document_file is - expected to be at the start of the router section and the end_position - desigates where that section ends. + Reads a range of the document_file containing some number of entry_class + instances. We deliminate the entry_class entries by the keyword on their + first line (entry_keyword). When finished the document is left at the + end_position. + + Either a end_position or section_end_keywords must be provided. :param file document_file: file with network status document content :param bool validate: checks the validity of the document's contents if True, skips these checks otherwise - :param object document: document the descriptors originate from - :param int start_position: start of the routers section - :param int end_position: end of the routers section - :param class router_type: router class to construct + :param class entry_class: class to construct instance for + :param str entry_keyword: first keyword for the entry instances + :param int start_position: start of the section, default is the current position + :param int end_position: end of the section + :param tuple section_end_keywords: keyword(s) that deliminate the end of the section if no end_position was provided + :param tuple extra_args: extra arguments for the entry_class (after the content and validate flag) - :returns: iterator over router_type instances + :returns: iterator over entry_class instances :raises: * ValueError if the contents is malformed and validate is True * IOError if the file can't be read """ - document_file.seek(start_position) - while document_file.tell() < end_position: - desc_content = "".join(_read_until_keywords("r", document_file, ignore_first = True, end_position = end_position)) - yield router_type(desc_content, document, validate) - -def _get_authorities(authorities, is_vote, validate): - """ - Iterates over the authoritiy entries in given content. - - :param str authority_lines: content of the authorities section - :param bool is_vote: indicates if this is for a vote or contensus document - :param bool validate: True if the document is to be validated, False otherwise - - :returns: DirectoryAuthority entries represented by the content - - :raises: ValueError if the document is invalid - """ + if start_position is None: + start_position = document_file.tell() - auth_buffer = [] - - for line in authorities.split("\n"): - if not line: continue - elif line.startswith(AUTH_START) and auth_buffer: - yield DirectoryAuthority("\n".join(auth_buffer), is_vote, validate) - auth_buffer = [] - - auth_buffer.append(line) + if end_position is None: + if section_end_keywords: + _read_until_keywords(section_end_keywords, document_file, skip = True) + end_position = document_file.tell() + else: + raise ValueError("Either a end_position or section_end_keywords must be provided") - if auth_buffer: - yield DirectoryAuthority("\n".join(auth_buffer), is_vote, validate) + document_file.seek(start_position) + while document_file.tell() < end_position: + desc_content = "".join(_read_until_keywords(entry_keyword, document_file, ignore_first = True, end_position = end_position)) + yield router_type(desc_content, validate, *extra_args) class NetworkStatusDocument(stem.descriptor.Descriptor): """ @@ -249,18 +248,27 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): super(NetworkStatusDocument, self).__init__(raw_content) document_file = StringIO(raw_content) - header = _read_until_keywords((AUTH_START, ROUTERS_START, FOOTER_START), document_file) - self._header = _DocumentHeader("".join(header), validate, default_params) - - authorities = _read_until_keywords((ROUTERS_START, FOOTER_START), document_file) - self.directory_authorities = list(_get_authorities("".join(authorities), self._header.is_vote, validate)) + self._header = _DocumentHeader(document_file, validate, default_params) - routers_start = document_file.tell() - _read_until_keywords(FOOTER_START, document_file, skip = True) - routers_end = document_file.tell() + self.directory_authorities = tuple(_get_entries( + document_file, + validate, + entry_class = DirectoryAuthority, + entry_keyword = AUTH_START, + section_end_keywords = (ROUTERS_START, FOOTER_START), + extra_args = (self._header.is_vote,), + )) - self._footer = _DocumentFooter(document_file.read(), validate, self._header) + self.routers = tuple(_get_entries( + document_file, + validate, + entry_class = self._get_router_type(), + entry_keyword = ROUTERS_START, + section_end_keywords = FOOTER_START, + extra_args = (self,), + )) + self._footer = _DocumentFooter(document_file, validate, self._header) self._unrecognized_lines = [] # copy the header and footer attributes into us @@ -269,8 +277,6 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): setattr(self, attr, value) else: self._unrecognized_lines += value - - self.routers = tuple(_get_routers(document_file, validate, self, routers_start, routers_end, self._get_router_type())) def _get_router_type(self): return RouterStatusEntry @@ -292,7 +298,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor): return list(self._unrecognized_lines) class _DocumentHeader(object): - def __init__(self, content, validate, default_params): + def __init__(self, document_file, validate, default_params): self.version = None self.is_consensus = True self.is_vote = False @@ -311,6 +317,7 @@ class _DocumentHeader(object): self._unrecognized_lines = [] + content = "".join(_read_until_keywords((AUTH_START, ROUTERS_START, FOOTER_START), document_file)) entries = stem.descriptor._get_descriptor_components(content, validate)[0] self._parse(entries, validate) @@ -485,12 +492,12 @@ class _DocumentHeader(object): raise ValueError("'%s' value on the params line must be in the range of %i - %i, was %i" % (key, minimum, maximum, value)) class _DocumentFooter(object): - def __init__(self, content, validate, header): + def __init__(self, document_file, validate, header): self.signatures = [] self.bandwidth_weights = {} - self._unrecognized_lines = [] + content = document_file.read() if validate and content and not header.meets_consensus_method(9): raise ValueError("Network status document's footer should only apepar in consensus-method 9 or later") elif not content and not header.meets_consensus_method(9): @@ -655,7 +662,7 @@ class DirectoryAuthority(stem.descriptor.Descriptor): | legacy_dir_key is the only optional attribute """ - def __init__(self, raw_content, vote = True, validate = True): + def __init__(self, raw_content, validate, vote = True): """ Parse a directory authority entry in a v3 network status document and provide a DirectoryAuthority object. @@ -765,7 +772,7 @@ class RouterStatusEntry(stem.descriptor.Descriptor): **\*** attribute is either required when we're parsed with validation or has a default value, others are left as None if undefined """ - def __init__(self, raw_contents, document, validate = True): + def __init__(self, raw_contents, validate = True, document = None): """ Parse a router descriptor in a v3 network status document. @@ -1045,7 +1052,7 @@ class RouterMicrodescriptor(RouterStatusEntry): | **\*** attribute is either required when we're parsed with validation or has a default value, others are left as None if undefined """ - def __init__(self, raw_contents, document, validate = True): + def __init__(self, raw_contents, validate, document): """ Parse a router descriptor in a v3 microdescriptor consensus and provide a new RouterMicrodescriptor object. @@ -1057,7 +1064,7 @@ class RouterMicrodescriptor(RouterStatusEntry): :raises: ValueError if the descriptor data is invalid """ - super(RouterMicrodescriptor, self).__init__(raw_contents, document, validate) + super(RouterMicrodescriptor, self).__init__(raw_contents, validate, document) self.document = document diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py index a0fe3a8..ad79dde 100644 --- a/test/unit/descriptor/networkstatus/document.py +++ b/test/unit/descriptor/networkstatus/document.py @@ -119,7 +119,7 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertEqual([], document.server_versions) self.assertEqual(expected_known_flags, document.known_flags) self.assertEqual(DEFAULT_PARAMS, document.params) - self.assertEqual([], document.directory_authorities) + self.assertEqual((), document.directory_authorities) self.assertEqual({}, document.bandwidth_weights) self.assertEqual([SIG], document.signatures) self.assertEqual([], document.get_unrecognized_lines()) @@ -151,7 +151,7 @@ class TestNetworkStatusDocument(unittest.TestCase): self.assertEqual([], document.server_versions) self.assertEqual(expected_known_flags, document.known_flags) self.assertEqual(DEFAULT_PARAMS, document.params) - self.assertEqual([], document.directory_authorities) + self.assertEqual((), document.directory_authorities) self.assertEqual({}, document.bandwidth_weights) self.assertEqual([SIG], document.signatures) self.assertEqual([], document.get_unrecognized_lines()) diff --git a/test/unit/descriptor/networkstatus/entry.py b/test/unit/descriptor/networkstatus/entry.py index 7c9e051..6380c62 100644 --- a/test/unit/descriptor/networkstatus/entry.py +++ b/test/unit/descriptor/networkstatus/entry.py @@ -73,7 +73,7 @@ class TestRouterStatusEntry(unittest.TestCase): Parses a minimal router status entry. """ - entry = RouterStatusEntry(get_router_status_entry(), None) + entry = RouterStatusEntry(get_router_status_entry()) expected_flags = set([Flag.FAST, Flag.NAMED, Flag.RUNNING, Flag.STABLE, Flag.VALID]) self.assertEqual(None, entry.document) @@ -114,7 +114,7 @@ class TestRouterStatusEntry(unittest.TestCase): """ content = get_router_status_entry({'z': 'New tor feature: sparkly unicorns!'}) - entry = RouterStatusEntry(content, None) + entry = RouterStatusEntry(content) self.assertEquals(['z New tor feature: sparkly unicorns!'], entry.get_unrecognized_lines()) def test_proceeding_line(self): @@ -131,7 +131,7 @@ class TestRouterStatusEntry(unittest.TestCase): """ content = get_router_status_entry() + "\n\nv Tor 0.2.2.35\n\n" - entry = RouterStatusEntry(content, None) + entry = RouterStatusEntry(content) self.assertEqual("Tor 0.2.2.35", entry.version_line) def test_missing_r_field(self): @@ -293,7 +293,7 @@ class TestRouterStatusEntry(unittest.TestCase): for s_line, expected in test_values.items(): content = get_router_status_entry({'s': s_line}) - entry = RouterStatusEntry(content, None) + entry = RouterStatusEntry(content) self.assertEquals(expected, entry.flags) # tries some invalid inputs @@ -321,7 +321,7 @@ class TestRouterStatusEntry(unittest.TestCase): for v_line, expected in test_values.items(): content = get_router_status_entry({'v': v_line}) - entry = RouterStatusEntry(content, None) + entry = RouterStatusEntry(content) self.assertEquals(expected, entry.version) self.assertEquals(v_line, entry.version_line) @@ -343,7 +343,7 @@ class TestRouterStatusEntry(unittest.TestCase): for w_line, expected in test_values.items(): content = get_router_status_entry({'w': w_line}) - entry = RouterStatusEntry(content, None) + entry = RouterStatusEntry(content) self.assertEquals(expected[0], entry.bandwidth) self.assertEquals(expected[1], entry.measured) self.assertEquals(expected[2], entry.unrecognized_bandwidth_entries) @@ -378,7 +378,7 @@ class TestRouterStatusEntry(unittest.TestCase): for p_line, expected in test_values.items(): content = get_router_status_entry({'p': p_line}) - entry = RouterStatusEntry(content, None) + entry = RouterStatusEntry(content) self.assertEquals(expected, entry.exit_policy) # tries some invalid inputs @@ -414,7 +414,7 @@ class TestRouterStatusEntry(unittest.TestCase): for m_line, expected in test_values.items(): content = get_router_status_entry({'m': m_line}) - entry = RouterStatusEntry(content, mock_document) + entry = RouterStatusEntry(content, document = mock_document) self.assertEquals(expected, entry.microdescriptor_hashes) # try without a document @@ -430,7 +430,7 @@ class TestRouterStatusEntry(unittest.TestCase): for m_line in test_values: content = get_router_status_entry({'m': m_line}) - self.assertRaises(ValueError, RouterStatusEntry, content, mock_document) + self.assertRaises(ValueError, RouterStatusEntry, content, True, mock_document) def _expect_invalid_attr(self, content, attr = None, expected_value = None): """ @@ -439,8 +439,8 @@ class TestRouterStatusEntry(unittest.TestCase): value when we're constructed without validation. """ - self.assertRaises(ValueError, RouterStatusEntry, content, None) - entry = RouterStatusEntry(content, None, False) + self.assertRaises(ValueError, RouterStatusEntry, content) + entry = RouterStatusEntry(content, False) if attr: self.assertEquals(expected_value, getattr(entry, attr))

1 0