[tor-commits] [stem/master] Protocol error tests and fix
atagar at torproject.org
atagar at torproject.org
Wed Oct 12 17:05:57 UTC 2011
commit 05b106bb575f669487fc376a29b7710d2addb2b0
Author: Damian Johnson <atagar at torproject.org>
Date: Wed Oct 12 09:17:59 2011 -0700
Protocol error tests and fix
Expanding the ControlMessage unit tests to cover causes of protocol errors, and
fixing an issue that this revealed where it was possable for a malformed line
prefix to go undetected (if, say, a dropped character caused a valid line
divider to fall into that place).
---
stem/types.py | 5 ++
test/unit/message.py | 125 +++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 113 insertions(+), 17 deletions(-)
diff --git a/stem/types.py b/stem/types.py
index d37f9a1..730accb 100644
--- a/stem/types.py
+++ b/stem/types.py
@@ -60,6 +60,9 @@ def read_message(control_file):
if len(line) < 4:
log.log(log.WARN, "ProtocolError: line too short (%s)" % line)
raise ProtocolError("Badly formatted reply line: too short")
+ elif not re.match(r'^[a-zA-Z0-9]{3}[-+ ]', line):
+ log.log(log.WARN, "ProtocolError: malformed status code/divider (%s)" % line)
+ raise ProtocolError("Badly formatted reply line: beginning is malformed")
elif not line.endswith("\r\n"):
log.log(log.WARN, "ProtocolError: no CRLF linebreak (%s)" % line)
raise ProtocolError("All lines should end with CRLF")
@@ -105,6 +108,8 @@ def read_message(control_file):
parsed_content.append((status_code, divider, content))
else:
+ # this should never be reached due to the prefix regex, but might as well
+ # be safe...
log.log(log.WARN, "ProtocolError: unrecognized divider type (%s)" % line)
raise ProtocolError("Unrecognized type '%s': %s" % (divider, line))
diff --git a/test/unit/message.py b/test/unit/message.py
index dde2c9c..e1fb622 100644
--- a/test/unit/message.py
+++ b/test/unit/message.py
@@ -6,20 +6,27 @@ import StringIO
import unittest
import stem.types
-GETINFO_VERSION_REPLY = """250-version=0.2.2.23-alpha (git-b85eb949b528f4d7)\r
-250 OK\r
-"""
+OK_REPLY = "250 OK\r\n"
-GETINFO_INFONAMES_REPLY = """250+info/names=\r
-accounting/bytes -- Number of bytes read/written so far in the accounting interval.\r
-accounting/bytes-left -- Number of bytes left to write/read so far in the accounting interval.\r
-accounting/enabled -- Is accounting currently enabled?\r
-accounting/hibernating -- Are we hibernating or awake?\r
-stream-status -- List of current streams.\r
-version -- The current version of Tor.\r
-.\r
-250 OK\r
-"""
+EVENT_BW = "650 BW 32326 2856\r\n"
+EVENT_CIRC_TIMEOUT = "650 CIRC 5 FAILED PURPOSE=GENERAL REASON=TIMEOUT\r\n"
+EVENT_CIRC_LAUNCHED = "650 CIRC 9 LAUNCHED PURPOSE=GENERAL\r\n"
+EVENT_CIRC_EXTENDED = "650 CIRC 5 EXTENDED $A200F527C82C59A25CCA44884B49D3D65B122652=faktor PURPOSE=MEASURE_TIMEOUT\r\n"
+
+GETINFO_VERSION = """250-version=0.2.2.23-alpha (git-b85eb949b528f4d7)
+250 OK
+""".replace("\n", "\r\n")
+
+GETINFO_INFONAMES = """250+info/names=
+accounting/bytes -- Number of bytes read/written so far in the accounting interval.
+accounting/bytes-left -- Number of bytes left to write/read so far in the accounting interval.
+accounting/enabled -- Is accounting currently enabled?
+accounting/hibernating -- Are we hibernating or awake?
+stream-status -- List of current streams.
+version -- The current version of Tor.
+.
+250 OK
+""".replace("\n", "\r\n")
class TestMessageFunctions(unittest.TestCase):
"""
@@ -27,13 +34,47 @@ class TestMessageFunctions(unittest.TestCase):
StringIO to make 'files' to mock socket input.
"""
- def test_getinfo_results(self):
+ def test_ok_response(self):
+ """
+ Checks the basic 'OK' response that we get for most commands.
+ """
+
+ message = self.assert_message_parses(OK_REPLY)
+ self.assertEquals("OK", str(message))
+
+ contents = message.content()
+ self.assertEquals(1, len(contents))
+ self.assertEquals(("250", " ", "OK"), contents[0])
+
+ def test_event_response(self):
+ """
+ Checks parsing of actual events.
+ """
+
+ # BW event
+ message = self.assert_message_parses(EVENT_BW)
+ self.assertEquals("BW 32326 2856", str(message))
+
+ contents = message.content()
+ self.assertEquals(1, len(contents))
+ self.assertEquals(("650", " ", "BW 32326 2856"), contents[0])
+
+ # few types of CIRC events
+ for circ_content in (EVENT_CIRC_TIMEOUT, EVENT_CIRC_LAUNCHED, EVENT_CIRC_EXTENDED):
+ message = self.assert_message_parses(circ_content)
+ self.assertEquals(circ_content[4:-2], str(message))
+
+ contents = message.content()
+ self.assertEquals(1, len(contents))
+ self.assertEquals(("650", " ", str(message)), contents[0])
+
+ def test_getinfo_response(self):
"""
- Checks parsing against some actual GETINFO responses.
+ Checks parsing of actual GETINFO responses.
"""
# GETINFO version (basic single-line results)
- message = self.assert_message_parses(GETINFO_VERSION_REPLY)
+ message = self.assert_message_parses(GETINFO_VERSION)
self.assertEquals(2, len(list(message)))
self.assertEquals(2, len(str(message).split("\n")))
@@ -44,7 +85,7 @@ class TestMessageFunctions(unittest.TestCase):
self.assertEquals(("250", " ", "OK"), contents[1])
# GETINFO info/names (data entry)
- message = self.assert_message_parses(GETINFO_INFONAMES_REPLY)
+ message = self.assert_message_parses(GETINFO_INFONAMES)
self.assertEquals(2, len(list(message)))
self.assertEquals(8, len(str(message).split("\n")))
@@ -56,6 +97,56 @@ class TestMessageFunctions(unittest.TestCase):
self.assertEquals(("250", "+", "info/names="), first_entry)
self.assertEquals(("250", " ", "OK"), contents[1])
+ def test_no_crlf(self):
+ """
+ Checks that we get a ProtocolError when we don't have both a carrage
+ returna and newline for line endings. This doesn't really check for
+ newlines (since that's what readline would break on), but not the end of
+ the world.
+ """
+
+ # Replaces each of the CRLF entries with just LF, confirming that this
+ # causes a parsing error. This should test line endings for both data
+ # entry parsing and non-data.
+
+ infonames_lines = [line + "\n" for line in GETINFO_INFONAMES.split("\n")[:-1]]
+
+ for i in range(len(infonames_lines)):
+ # replace the CRLF for the line
+ infonames_lines[i] = infonames_lines[i].rstrip("\r\n") + "\n"
+ test_socket_file = StringIO.StringIO("".join(infonames_lines))
+ self.assertRaises(stem.types.ProtocolError, stem.types.read_message, test_socket_file)
+
+ # puts the CRLF back
+ infonames_lines[i] = infonames_lines[i].rstrip("\n") + "\r\n"
+
+ # sanity check the above test isn't broken due to leaving infonames_lines
+ # with invalid data
+
+ self.assert_message_parses("".join(infonames_lines))
+
+ def test_malformed_prefix(self):
+ """
+ Checks parsing for responses where the header is missing a digit or divider.
+ """
+
+ for i in range(len(EVENT_BW)):
+ # makes test input with that character missing or replaced
+ removal_test_input = EVENT_BW[:i] + EVENT_BW[i + 1:]
+ replacement_test_input = EVENT_BW[:i] + "#" + EVENT_BW[i + 1:]
+
+ if i < 4 or i >= (len(EVENT_BW) - 2):
+ # dropping the character should cause an error if...
+ # - this is part of the message prefix
+ # - this is disrupting the line ending
+
+ self.assertRaises(stem.types.ProtocolError, stem.types.read_message, StringIO.StringIO(removal_test_input))
+ self.assertRaises(stem.types.ProtocolError, stem.types.read_message, StringIO.StringIO(replacement_test_input))
+ else:
+ # otherwise the data will be malformed, but this goes undetected
+ self.assert_message_parses(removal_test_input)
+ self.assert_message_parses(replacement_test_input)
+
def assert_message_parses(self, controller_reply):
"""
Performs some basic sanity checks that a reply mirrors its parsed result.
More information about the tor-commits
mailing list