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
_______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
