Opened 9 years ago

Last modified 3 years ago

#6384 defect new

FileTransferClient() ignores extData in constructor, error in connectionMade() sending extData

Reported by: John Popplewell Owned by:
Priority: normal Milestone:
Component: conch Keywords: FileTransferClient
Cc: z3p Branch:
Author:

Description (last modified by Thijs Triemstra)

In conch/ssh/filetransfer.py

class FileTransferClient(FileTransferBase):     
    def __init__(self, extData = {}):
        """
        @param extData: a dict of extended_name : extended_data ...
        """
        FileTransferBase.__init__(self)
        self.extData = {}

Shouldn't this be:

        self.extData = extData

Also, when I initialized extData after constructing a class derived from FileTransferClient() I found this:

class FileTransferClient(FileTransferBase):
    ....
    def connectionMade(self):
        data = struct.pack('!L', max(self.versions))
        for k,v in self.extData.itervalues():
            data += NS(k) + NS(v)
        self.sendPacket(FXP_INIT, data)

Clearly the for loop it supposed to be iterating over iteritems() like this:

        for k,v in self.extData.iteritems():
            data += NS(k) + NS(v)

I used the extension to send application version information for backwards compatibility purposes,

best regards, John.

Attachments (3)

filetransfer.py.diff (1.1 KB) - added by John Popplewell 9 years ago.
Here's a patch. Sorry about the mess in previous post.
filetransfer.py.patch (778 bytes) - added by John Popplewell 9 years ago.
Updated patch.
test_filetransfer.py.patch (1.3 KB) - added by John Popplewell 9 years ago.
Here's a test for the previous patch.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 9 years ago by DefaultCC Plugin

Cc: z3p added

Changed 9 years ago by John Popplewell

Attachment: filetransfer.py.diff added

Here's a patch. Sorry about the mess in previous post.

comment:2 Changed 9 years ago by Thijs Triemstra

Description: modified (diff)
Keywords: conch removed

Fixing ticket description markup.

The patch looks strange (might be trac), have you used these instructions to create the patch?

comment:3 in reply to:  2 Changed 9 years ago by John Popplewell

Replying to thijs:

Fixing ticket description markup.

The patch looks strange (might be trac), have you used these instructions to create the patch?

I didn't no, sorry. I seem to be making a bit of a mess of this. I'll attach a new patch. Thanks for fixing the markup.

Changed 9 years ago by John Popplewell

Attachment: filetransfer.py.patch added

Updated patch.

Changed 9 years ago by John Popplewell

Attachment: test_filetransfer.py.patch added

Here's a test for the previous patch.

comment:4 Changed 3 years ago by Wim Lewis

Looking at the IETF filexfer drafts, I found that the ability to send extData in the client version packet was a short-lived proposal, introduced in draft 00 (protocol v3) and removed in draft 03 (protocol v4) due to interoperability concerns. Instead, only the server sends an extension list (it can do this because it replies second, and can downgrade when talking to an old client); clients do not send extData during init but can invoke extensions using SSH_FXP_EXTENDED.

Twisted Conch speaks protocol v3, so I guess it's technically allowable for the client to send the extData, but I think it shouldn't. If this is a concern we should start implementing newer versions of the filexfer protocol (see Trac #498 ).

Probably what we should do is deprecate and remove the extData argument to FileTransferClient and implement extensions using an API corresponding to the mechanisms from later protocol drafts,

Note: See TracTickets for help on using tickets.