On 2014-06-21, 10:24, exar...@twistedmatrix.com wrote:
> If bypassing the reactor this way is bad, how bad is it and what are
the consequences or effects?
This use is untested. There's no reason to expect it will continue to
work with future Twisted releases (or, really, that it fully works
now; since `writeSomeData` bypasses the transport's buffering layer,
it seems like you're risking an out-of-order or partial send; probably
these will only arise under load so you may not have observed them in
your testing).
Thanks for the very helpful reply! This is exactly the sort of thing I
was hoping to find out.
Is there a better way to get a working solution?
The proper way to do this would be for OpenSSH to acknowledge the
operation. At this point you would know it's safe to proceed to the
next operation. Since you didn't mention anything about
acknowledgements, I'm guessing there are none.
Correct, the order of events is:
0. client opens OpenSSH socket, sends hello, receives reply
1. user calls the protocol function to create a new OpenSSH session over
the existing socket
2. client sends MUX_C_ALIVE_CHECK to OpenSSH
3. OpenSSH replies with MUX_S_ALIVE
4. client sends MUX_C_NEW_SESSION to OpenSSH, then immediately forwards
the file descriptors
5. OpenSSH replies with either MUX_S_SESSION_OPENED,
MUX_S_PERMISSION_DENIED, or MUX_S_FAILURE.
6. ...session is now created...
7. OpenSSH sends MUX_S_EXIT_MESSAGE when the session has finished
8. client can now close the OpenSSH socket
It would make sense for OpenSSH to acknowledge the MUX_C_NEW_SESSION in
step 4 before the file descriptors get sent, but I've read the source
code and OpenSSH does not do this -- it expects the file descriptors to
be sent as soon as the new session message is sent.
Since you're already relying on `self.transport.socket.fileno()` and
`send1msg` (basically, bypassing the transport abstraction and just
doing socket operations yourself) one improvement you could make would
be just to rely on that for the whole thing. Don't use
`writeSomeData`. Use `socket.send(command)`. At least this way you're
only relying on being able to treat a transport like a UNIX socket -
not on the particulars of the transport's buffering implementation.
A different approach you could take would be to implement this
connection sharing feature for Conch. I can pretty much guarantee
it's possible to implement since an older version of Conch actually
did implement it. :) The implementation was removed because it was
fragile, complicated, and poorly tested. It would be great to
re-introduce the functionality with a higher quality implementation.
I'll look into what it might take to implement this in a robust way with
tests for Conch. I think that's definitely a preferred solution, since
it sounds like such a feature might be accepted (I wasn't considering
this initially since Conch is a generic SSH transport, and piggybacking
on top of an existing OpenSSH socket is a highly-specific thing.) If I
can come up with a good way to do this, I'll submit a patch, otherwise
I'll keep the functionality as it is now, in a standalone class that
doesn't use Conch at all.
--
Mark Montague
m...@catseye.org
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
