[tor-commits] [stem/master] Revising ATTACHSTREAM 555 response handling

[atagar at torproject.org]

commit b36a9e3dc44adfda40390527fbb667a174a4736d
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Apr 14 14:20:10 2013 -0700

    Revising ATTACHSTREAM 555 response handling
    
    Thanks to Roger we now know that a 555 response means that a stream's in a
    state where it can't be attached. Swapping the exception we raise to
    UnsatisfiableRequest and adding a little test for this.
---
 stem/control.py                 |    8 ++------
 test/settings.cfg               |    2 ++
 test/unit/control/controller.py |   16 +++++++++++++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index 478384a..f3e6d17 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -1965,6 +1965,7 @@ class Controller(BaseController):
 
     :raises:
       * :class:`stem.InvalidRequest` if the stream or circuit id were unrecognized
+      * :class:`UnsatisfiableRequest` if the stream isn't in a state where it can be attached
       * :class:`stem.OperationFailed` if the stream couldn't be attached for any other reason
     """
 
@@ -1982,12 +1983,7 @@ class Controller(BaseController):
       elif response.code == '551':
         raise stem.OperationFailed(response.code, response.message)
       elif response.code == '555':
-        # TODO: This response has been seen in the wild, but isn't valid
-        # according to the spec...
-        #
-        # https://trac.torproject.org/8701
-
-        raise stem.OperationFailed(response.code, response.message)
+        raise stem.UnsatisfiableRequest(response.code, response.message)
       else:
         raise stem.ProtocolError("ATTACHSTREAM returned unexpected response code: %s" % response.code)
 
diff --git a/test/settings.cfg b/test/settings.cfg
index 9d28f4d..a40e1b7 100644
--- a/test/settings.cfg
+++ b/test/settings.cfg
@@ -172,6 +172,8 @@ test.unit_tests
 |test.unit.response.getinfo.TestGetInfoResponse
 |test.unit.response.getconf.TestGetConfResponse
 |test.unit.response.singleline.TestSingleLineResponse
+|test.unit.connection.authentication.TestAuthenticate
+|test.unit.control.controller.TestControl
 
 test.integ_tests
 |test.integ.util.conf.TestConf
diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py
index ccce4df..9efe4bf 100644
--- a/test/unit/control/controller.py
+++ b/test/unit/control/controller.py
@@ -6,10 +6,11 @@ integ tests, but a few bits lend themselves to unit testing.
 import unittest
 
 import stem.descriptor.router_status_entry
+import stem.response
 import stem.socket
 import stem.version
 
-from stem import InvalidArguments, InvalidRequest, ProtocolError
+from stem import InvalidArguments, InvalidRequest, ProtocolError, UnsatisfiableRequest
 from stem.control import _parse_circ_path, Controller, EventType
 from stem.exit_policy import ExitPolicy
 from test import mocking
@@ -320,6 +321,19 @@ class TestControl(unittest.TestCase):
       self.assertEqual(valid_streams[index][2], stream.circ_id)
       self.assertEqual(valid_streams[index][3], stream.target)
 
+  def test_attach_stream(self):
+    """
+    Exercises the attach_stream() method.
+    """
+
+    # Response when the stream is in a state where it can't be attached (for
+    # instance, it's already open).
+
+    response = stem.response.ControlMessage.from_str("555 Connection is not managed by controller.\r\n")
+    mocking.mock_method(Controller, "msg", mocking.return_value(response))
+
+    self.assertRaises(UnsatisfiableRequest, self.controller.attach_stream, 'stream_id', 'circ_id')
+
   def test_parse_circ_path(self):
     """
     Exercises the _parse_circ_path() helper function.