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