Stem doesn't recognize error 555 on attach_stream

When trying to attach a stream that is not controlled by the controller using Controller.attach_stream, Tor will reply with error 555. Stem does not recognize this and will throw a generic exception:

stem.ProtocolError: ATTACHSTREAM returned unexpected response code: 555

I think an InvalidArguments exception would be best suited for this error code.

@@ -1981,6 +1981,8 @@ class Controller(BaseController):
         raise stem.InvalidRequest(response.code, response.message)
       elif response.code == '551':
         raise stem.OperationFailed(response.code, response.message)
+      elif response.code == '555':
+        raise stem.InvalidArguments(response.code, response.message)
       else:
         raise stem.ProtocolError("ATTACHSTREAM returned unexpected response code: %s" % response.code)

changed milestone to %Tor: 0.2.4.x-final

added component::core tor/tor milestone::Tor: 0.2.4.x-final owner::atagar priority::medium resolution::fixed status::closed tor-client tor-spec type::defect labels

Thanks! You uncovered not just a Stem issue but one with Tor. According to the spec ATTACHSTREAM should only respond with 250, 552, or 551.

I think an InvalidArguments exception would be best suited for this error code.

I'm not entirely clear what a 555 response means. I'd rather not have attach_stream() raise a new kind of exception before the spec is updated, so raising a OperationFailed for the time being (stem change).

Would you mind printing raw_content() for this ControlMessage? I'd love to include it in our unit tests. :)

Reassigning this to the tor component for the spec fix.

Trac:
Component: Stem to Tor

  if (ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONTROLLER_WAIT &&
      ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONNECT_WAIT &&
      ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_RESOLVE_WAIT) {
    connection_write_str_to_buf(
                    "555 Connection is not managed by controller.\r\n",
                    conn);
    return 0;
  }

This happens when you try to attachstream a stream that isn't in a state where we think you should be able to do so. For example, if you try to attachstream an open stream, it should fail.

That said, there may be a bug here: the 'if' I just quoted doesn't consider AP_CONN_STATE_CIRCUIT_WAIT. That is, you can't ask to attachstream a stream that is waiting for Tor to produce a suitable circuit. That is probably because we assumed an implicit invariant that there is no acceptable circuit for a stream in this state (if there were one, the stream wouldn't still be in this state). But a) we might be wrong, and b) why constrain controllers?

diff --git a/control-spec.txt b/control-spec.txt
index 4498fbe..adf4fd1 100644
--- a/control-spec.txt
+++ b/control-spec.txt
@@ -837,8 +837,9 @@
   Hops are 1-indexed; generally, it is not permitted to attach to hop 1.
 
   Tor responds with "250 OK" if it can attach the stream, 552 if the circuit
-  or stream didn't exist, or 551 if the stream couldn't be attached for
-  another reason.
+  or stream didn't exist, 555 if the stream isn't in an appropriate
+  state to be attached (e.g. it's already open), or 551 if the stream
+  couldn't be attached for another reason.
 
   {Implementation note: Tor will close unattached streams by itself,
   roughly two minutes after they are born. Let the developers know if

is probably the spec change you had in mind

Thanks Roger! Revised stem's handling to raise a UnsatisfiableRequest instead and added this to our tests (change). Spec change looks good to me.

Trac:
Keywords: N/A deleted, tor-spec added
Milestone: N/A to Tor: 0.2.4.x-final
Status: new to needs_review

Roger, that kind of inline patch is annoying for me to apply, since I have to copy-and-paste it and write a commit message and attribute it to you. "git format-patch" or a mergeable branch is much easier on my end.

Merged it.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

I think there remains a bug here. See my "That said, there may be a bug here" paragraph. Why do we prevent the controller from attachstreaming a circuit that's currently in state AP_CONN_STATE_CIRCUIT_WAIT ?

Trac:
Resolution: fixed to N/A
Status: closed to reopened
Keywords: tor-spec deleted, N/A added

Replying to arma:

I think there remains a bug here. See my "That said, there may be a bug here" paragraph. Why do we prevent the controller from attachstreaming a circuit that's currently in state AP_CONN_STATE_CIRCUIT_WAIT ?

Because there's probably a race condition in your program if you expect that to work.

This can happen if you are not setting LeaveStreamsUnattached, and you are trying to call attachstream on user streams. But that case indicates a questionable controller program, since there's no way for you to make sure that your choice of circuit will be honored. Instead, the circuit that the stream is waiting for might complete, and the stream might attach to that one instead of yours.

It can also happen if you are setting LeaveStreamsUnattached, and you are trying to call attachstream on Tor-internal streams, like directory requests. But that's even more unsupported.

Trac:
Keywords: N/A deleted, tor-spec tor-client added

Ok. In the interest of not adding more work for me, I agree. :) Closing.

Trac:
Resolution: N/A to fixed
Status: reopened to closed

closed

moved to tpo/core/tor#8701 (closed)