On Tue, May 21, 2013 at 4:21 AM, jeff quast <
con...@jeffquast.com> wrote:
> Hello,
>
> I'd like to share a Telnet Server implementation using tulip and request any feedback anybody anybody would like to share. I feel there may be facilities in tulip that I should be making use of... though it appears to function just fine. Any guidance of any wrong- or should-be-doings here appreciated.
Hi Jeff,
Thanks for giving Tulip such a thorough try! It's a lot of code to
take in, so I'll focus on issues that directly affect Tulip.
>
https://github.com/jquast/telnetlib3/blob/master/telnetlib3.py
>
> For instance, Implementing SLC_XON (^s) and XOFF (^q) required some sort of flow control on the writer.I subclassed both_SelectorSocketTransport and SelectorEventLoop to implement this. Is this correct? The _prefix indicates it isn't, but I couldn't find another way or example. Interesting that the reader has a pause() and resume() but not the writer. It looks like i've implemented the same on the other end of the socketpair, using a different pseudonym.
You found a hole in the spec and implementation. I was always aware
that we needed flow control in both directions, but didn't have much
of a use case for the other direction, so I waited for one to pop up.
You provided one. The PEP has this to say:
"""
TBD: Provide flow control the other way -- the transport may need to
suspend the protocol if the amount of data buffered becomes a burden.
Proposal: let the transport call ``protocol.pause()`` and
``protocol.resume()`` if they exist; if they don't exist, the
protocol doesn't support flow control. (Perhaps different names
to avoid confusion between protocols and transports?)
"""
However, it seems you've chosen a different implementation strategy,
probably because of the way telnet works: instead of the transport
asking the protocol to (temporarily) stop calling write(), in your
case the protocol asks the transport to to temporarily change
write()'s behavior to direct all data directly to the buffer and stop
flushing the buffer to the socket. That's actually an interesting
concept, as is the abort_output() method.
I propose that we add this functionality to the standard Tulip
StreamTransport classes, under different names. Could you come up with
a patch for that? I propose the following names: xon -> pause_writing,
xoff -> resume_writing, abort_output -> discard_buffer. (You can leave
the Proactor version unimplemented, although it should be pretty
simple to add it there too, if you have access to a Windows box to
test.)
In general, the _underscore_prefix classes are definitely not part of
the public interface, and subclassing them is the wrong way to go
about this -- if we weren't going to add these things to Tulip, I
would have recommended that you write some kind of adaptor class, or
add a buffer to the protocol class.
Which reminds me, I'm also not that keen on subclassing StreamReader.
In fact, I'm not keen on subclassing *any* classes that are provided
by a different library that aren't specifically written and documented
to support subclassing. Subclassing has its place (especially *within*
a library, as it is used by Tulip), but several decades of OO
programming have taught us that using subclassing as an approach to
API design is usually a bad idea. I suspect that if, instead of
subclassing StreamReader, you just write a separate class that takes a
StreamReader instance as an argument (or instantiates one, your
choice) your code wouldn't be much different, and there's much less
coupling between your implementation and the StreamReader
implementation.
> Telnet Servers are responsible for requesting various negotiation states for a wide variety of basic and extended options. If you connect to the example server with a bsd telnet client, the logger will blow up with a fascinating variety of option negotiations. Any generic interface would necessarily be rich with callbacks, and this library is no exception.
It's impressive. Did you just implement the entire RFC?
> However, there is a general lack of fancy tulip expressions being used. No coroutines or futures, no yield from, just a very simple _negotiate() loop on connect that uses call_soon and call_later. Is this because I'm missing out on some interesting opportunities? Or because the telnet protocol lends itself to, or at least my implementation is strictly, byte-at-a-time processing? I feel as though these features are not necessary here ..
I think it's fine. You're still using async I/O and an event loop.
> As mentioned about asyncore and pyftpdlib in prior discussions, With SO_OOBINLINE and MSG_OOB not implemented in tulip, it looks like the Protocol does not comply with the telnet RFC for the "Synch" mechanism.
Another thing for which I didn't have a use case. IIRC I asked Glyph
about these and he didn't think they were much used either. But it
sounds like full implementation of the telnet RFC requires supporting
these, and I'd be happy to comply -- except I don't know a thing about
how they are typically used (never having used them myself). But if
you send me even just a sketch for a patch, I'd be happy to look into
it and (probably) incorporate some version of it.
> If you escape ^] in bsd telnet client, and type "send synch" to the example server, the IAC byte is not received, and the "May have received DM ...." debug line is printed, by matching only DM, which, in BINARY mode, could just as well be an extended ascii character for whatever encoding.
Heh. I have no idea what you just wrote. :-(
> I may use such a check to "snuff" it quietly, or receive it as part of input for the rare occasions it is received while in BINARY mode. Does this sound safe enough?
Again, I'm not sure, because I don't understand. Is this feature
actually used? (TBH I haven't used telnet in ages -- it's all SSH
nowadays. :-)
> Although I haven't finished CHARSET negotiation, some support for byte encoding is offered. The default linemode will echo back any bytes received and may appear to be correct on output. For this same reason, if your "KEY_UP" sequence is equal to your visual "move_up" sequence, it appears like you may be moving your cursor in a full screen editor, but it isn't. You would expect the same behavior from a tcp echo server.
Again, I don't really understand what you are saying here.
> For now, typing "set CHARSET=utf-8" at the prompt will change the decoding to 'utf-8'. I soon plan to wire in CHARSET rfc, which I've seen some MUD clients reply "UTF-8" to (though not bsd telnet client).
Ah, I guess MUDs are the reason Telnet still exists?
> Having previously authored a "Telnet BBS" system in python that behaves in "character at a time" mode, and previously implemented the same with twisted, and having extended for various NAWS, TTYPE, NEW_ENVIRON, etc, and having studied several other implementations out there over the past 10 years, esp. recently in a growing "Python Mud Server" community, I'd like to have this polished off onto pypy land and get these projects more compatible and more capable. I really want to see them stop being confused by encodings ! Some of these projects already use Twisted, so there is hopes they could "plug-in", as it goes.
Sounds like a great case for Tulip!
> I do hope to provide an examples folder with a perhaps a chat server, a basic mud game (that implements some additional mud-only telnet extensions), and maybe something like dgamelaunch that front-ends pty.fork() to demonstrate character-at-a-time, even if it is just /usr/games/trek or vi or something. I also aim to create a (headless) Telnet Client based on the same Protocol. Most of the Protocol code is shared between client and server, and enforced by 'is_server' assertions already. There is also a great deal of warnings and assertions regarding RFC compliance in place, as I hope to create a server fingerprinting tool, a capability-compliance testing tool, and perhaps a client-fingerprinting telnet server for honeypots.
That sounds awesome.
--
--Guido van Rossum (
python.org/~guido)