[Patches] [ python-Patches-664020 ] telnetlib option subnegotiation fix

[SourceForge.net]

Patches item #664020, was opened at 2003-01-07 22:51
Message generated for change (Comment added) made by sonderblade
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=664020&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Steve Reeves (sreeves)
Assigned to: Nobody/Anonymous (nobody)
Summary: telnetlib option subnegotiation fix

Initial Comment:
The telnetlib patch #630829 (committed as v1.20) for
option subnegotiation doesn't work as-is.  There are
several problems.

1) The option negotiation callback should be a method
of the Telnet class, not a separate function.  The
callback is supposed to call the read_sb_data method
when it receives SB or SE, but of which object?

I think the callback should have been a method all
along.  It should be able to change the object's state
based on the negotiation.  Generally when implementing
a new protocol extension, you'll subclass Telnet, add
the new behavior, and then have the callback set a flag
in the object to activate the new behavior after it has
been negotiated.

The default DONT/WONT behavior can be moved out of the
depths of process_rawq() into the callback method. 
Subclasses can then just forward to the parent class
when they get an option they don't care about, instead
of having to reimplement the default behavior.

If the callback is a method, the socket argument is
still available as self.sock.  The
set_option_negotiation_callback method and the checks
for "if self.option_callback:" would not be needed.

Changing the callback to a method will break
compatibility with Python 2.2, however.

2) On receipt of SB and SE, the callback is always
called with NOOPT.  The option is never passed.  (It
actually gets stored as the first part of the parameter
data and returned by read_sb_data().)

3) The callback is called on the receipt of both SB
_and_ SE.  The SB call is completely superfluous.  The
option and parameter data have not been read yet, so
there is nothing for the callback to do.

The attached patch fixes these and adds support for the
NOP command.  It also includes documentation changes
and an example of implementing some options.

Some other thoughts:

The name "option_callback" is a little misleading.  The
callback is executed in response to all telnet protocol
commands, not just the ones dealing with options.

Why define and pass NOOPT for the case when there is no
option rather than use None?

The only place the SB parameter data will (can safely?)
be used is in the callback.  Why not pass it as another
optional parameter and skip the need for read_sb_data()?

A potential name conflict: the EOR telnet option
defines a new command code also called EOR (with a much
different numeric value).  The C header file
<arpa/telnet.h> prefixes all the options with TELOPT_,
so there is no conflict there, but telnetlib drops the
prefix.


----------------------------------------------------------------------

Comment By: Björn Lindqvist (sonderblade)
Date: 2007-03-09 01:16

Message:
Logged In: YES 
user_id=51702
Originator: NO

This patch is very similar to #1520081. They both suggest replacing
set_option_negotiation_callback with a method on the Telnet object which I
think makes perfect sense. They both contain big and useful updates to the
documentation. This patch also contain a useful refactoring of the
process_rawq() method. I think a merged patch of them both should be
applied. But IMHO, it should wait to Py3k because then API can be broken
and then set_option_negotiation_callback can be eliminated. 

----------------------------------------------------------------------

Comment By: Steve Reeves (sreeves)
Date: 2003-01-07 22:57

Message:
Logged In: YES 
user_id=2647

Sorry, the patch wasn't uploaded for some reason.  Trying again.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=664020&group_id=5470