[tor-commits] [stem/master] Parsing the params attribute
atagar at torproject.org
atagar at torproject.org
Sat Oct 13 18:35:45 UTC 2012
commit 355e474b3d62dceb09699f1be27b5b27b925e7fa
Author: Damian Johnson <atagar at torproject.org>
Date: Mon Sep 10 09:12:19 2012 -0700
Parsing the params attribute
Being a fair bit more anal about field validation, checking the following...
* bounds on the value to be an 32 bit int
* order of the values (keys should be ascending)
* reject values like '+11' (the int() function accepts them)
This also includes unit tests for these use cases and better error messaging.
---
stem/descriptor/networkstatus.py | 50 ++++++++++++++--
test/unit/descriptor/networkstatus/document.py | 74 +++++++++++++++++++++++-
2 files changed, 117 insertions(+), 7 deletions(-)
diff --git a/stem/descriptor/networkstatus.py b/stem/descriptor/networkstatus.py
index 1efcf58..d43c1d0 100644
--- a/stem/descriptor/networkstatus.py
+++ b/stem/descriptor/networkstatus.py
@@ -196,7 +196,7 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
:var list client_versions: list of recommended client tor versions
:var list server_versions: list of recommended server tor versions
:var list known_flags: **\*** list of known router flags
- :var list params: dict of parameter(str) => value(int) mappings
+ :var list params: **\*** dict of parameter(str) => value(int) mappings
:var list directory_authorities: **\*** list of DirectoryAuthority objects that have generated this document
:var dict bandwidth_weights: **~** dict of weight(str) => value(int) mappings
:var list directory_signatures: **\*** list of signatures this document has
@@ -381,6 +381,49 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
# simply fetches the entries, excluding empty strings
self.known_flags = [entry for entry in value.split(" ") if entry]
+ elif keyword == "params":
+ # "params" [Parameters]
+ # Parameter ::= Keyword '=' Int32
+ # Int32 ::= A decimal integer between -2147483648 and 2147483647.
+ # Parameters ::= Parameter | Parameters SP Parameter
+
+ # should only appear in consensus-method 7 or later
+ if validate and not (self.consensus_method >= 7 or filter(lambda x: x >= 7, self.consensus_methods)):
+ raise ValueError("A network status document's 'params' line should only appear in consensus-method 7 or later")
+
+ # skip if this is a blank line
+ if value == "": continue
+
+ for entry in value.split(" "):
+ try:
+ if not '=' in entry:
+ raise ValueError("must only have 'key=value' entries")
+
+ entry_key, entry_value = entry.split("=", 1)
+
+ try:
+ # the int() function accepts things like '+123', but we don't want to
+ if entry_value.startswith('+'):
+ raise ValueError()
+
+ entry_value = int(entry_value)
+ except ValueError:
+ raise ValueError("'%s' is a non-numeric value" % entry_value)
+
+ if validate:
+ # check int32 range
+ if entry_value < -2147483648 or entry_value > 2147483647:
+ raise ValueError("values must be between -2147483648 and 2147483647")
+
+ # parameters should be in ascending order by their key
+ for prior_key in self.params:
+ if prior_key > entry_key:
+ raise ValueError("parameters must be sorted by their key")
+
+ self.params[entry_key] = entry_value
+ except ValueError, exc:
+ if not validate: continue
+ raise ValueError("Unable to parse network status document's 'params' line (%s): %s'" % (exc, line))
# doing this validation afterward so we know our 'is_consensus' and
# 'is_vote' attributes
@@ -407,13 +450,10 @@ class NetworkStatusDocument(stem.descriptor.Descriptor):
_read_keyword_line("client-versions", content, False, True)
_read_keyword_line("server-versions", content, False, True)
_read_keyword_line("known-flags", content, False, True)
+ _read_keyword_line("params", content, False, True)
vote = self.is_vote
- read_keyword_line("params", True)
- if self.params:
- self.params = dict([(param.split("=")[0], int(param.split("=")[1])) for param in self.params.split(" ")])
-
# authority section
while _peek_keyword(content) == "dir-source":
dirauth_data = _read_until_keywords(["dir-source", "r", "directory-footer", "directory-signature", "bandwidth-weights"], content, False, True)
diff --git a/test/unit/descriptor/networkstatus/document.py b/test/unit/descriptor/networkstatus/document.py
index 97e239a..01f08bf 100644
--- a/test/unit/descriptor/networkstatus/document.py
+++ b/test/unit/descriptor/networkstatus/document.py
@@ -116,7 +116,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
self.assertEqual([], document.client_versions)
self.assertEqual([], document.server_versions)
self.assertEqual(expected_known_flags, document.known_flags)
- self.assertEqual(None, document.params)
+ self.assertEqual({}, document.params)
self.assertEqual([], document.directory_authorities)
self.assertEqual(None, document.bandwidth_weights)
self.assertEqual([sig], document.directory_signatures)
@@ -150,7 +150,7 @@ class TestNetworkStatusDocument(unittest.TestCase):
self.assertEqual([], document.client_versions)
self.assertEqual([], document.server_versions)
self.assertEqual(expected_known_flags, document.known_flags)
- self.assertEqual(None, document.params)
+ self.assertEqual({}, document.params)
self.assertEqual([], document.directory_authorities)
self.assertEqual({}, document.bandwidth_weights)
self.assertEqual([sig], document.directory_signatures)
@@ -441,4 +441,74 @@ class TestNetworkStatusDocument(unittest.TestCase):
content = get_network_status_document({"known-flags": test_value})
document = NetworkStatusDocument(content)
self.assertEquals(expected_value, document.known_flags)
+
+ def test_params(self):
+ """
+ General testing for the 'params' line, exercising the happy cases.
+ """
+
+ content = get_network_status_document({"params": "CircuitPriorityHalflifeMsec=30000 bwauthpid=1 unrecognized=-122"})
+ document = NetworkStatusDocument(content)
+ self.assertEquals(30000, document.params["CircuitPriorityHalflifeMsec"])
+ self.assertEquals(1, document.params["bwauthpid"])
+ self.assertEquals(-122, document.params["unrecognized"])
+
+ # empty params line
+ content = get_network_status_document({"params": ""})
+ document = NetworkStatusDocument(content)
+ self.assertEquals({}, document.params)
+
+ def test_params_malformed(self):
+ """
+ Parses a 'params' line with malformed content.
+ """
+
+ test_values = (
+ "foo=",
+ "foo=abc",
+ "foo=+123",
+ "foo=12\tbar=12",
+ )
+
+ for test_value in test_values:
+ content = get_network_status_document({"params": test_value})
+ self.assertRaises(ValueError, NetworkStatusDocument, content)
+
+ document = NetworkStatusDocument(content, False)
+ self.assertEquals({}, document.params)
+
+ def test_params_range(self):
+ """
+ Check both the furthest valid 'params' values and values that are out of
+ bounds.
+ """
+
+ test_values = (
+ ("foo=2147483648", {"foo": 2147483648}, False),
+ ("foo=-2147483649", {"foo": -2147483649}, False),
+ ("foo=2147483647", {"foo": 2147483647}, True),
+ ("foo=-2147483648", {"foo": -2147483648}, True),
+ )
+
+ for test_value, expected_value, is_ok in test_values:
+ content = get_network_status_document({"params": test_value})
+
+ if is_ok:
+ document = NetworkStatusDocument(content)
+ else:
+ self.assertRaises(ValueError, NetworkStatusDocument, content)
+ document = NetworkStatusDocument(content, False)
+
+ self.assertEquals(expected_value, document.params)
+
+ def test_params_misordered(self):
+ """
+ Check that the 'params' line is rejected if out of order.
+ """
+
+ content = get_network_status_document({"params": "unrecognized=-122 bwauthpid=1"})
+ self.assertRaises(ValueError, NetworkStatusDocument, content)
+
+ document = NetworkStatusDocument(content, False)
+ self.assertEquals({"unrecognized": -122, "bwauthpid": 1}, document.params)
More information about the tor-commits
mailing list