﻿id	summary	reporter	owner	description	type	status	priority	milestone	component	version	resolution	keywords	cc	launchpad_bug
941	SFTP frontend fails when listing a directory containing a mutable file, because it relies on node.get_size() to be an integer	davidsarah	davidsarah	"When the SFTP frontend lists a directory, it sometimes raises a {{{struct.error}}} exception:
{{{
ssh-connection on SSHServerTransport,11,127.0.0.1] OPENDIRECTORY /
2010-02-06 19:09:37-0700 [SSHChannel session (0) on SSHService
ssh-connection on SSHServerTransport,11,127.0.0.1] CONVERT  []
2010-02-06 19:09:37-0700 [SSHChannel session (0) on SSHService
ssh-connection on SSHServerTransport,11,127.0.0.1] ROOT <DirectoryNode
RW-MUT oen7k>
2010-02-06 19:09:37-0700 [SSHChannel session (0) on SSHService
ssh-connection on SSHServerTransport,11,127.0.0.1] PATH []
2010-02-06 19:09:37-0700 [SSHChannel session (0) on SSHService
ssh-connection on SSHServerTransport,11,127.0.0.1] Unhandled Error
        Traceback (most recent call last):
[...]
File ""/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py"", line 312, in _cbSendDirectory
            data += self._packAttributes(attrs)
File ""/usr/lib/python2.6/dist-packages/twisted/conch/ssh/filetransfer.py"", line 95, in _packAttributes
            data += struct.pack('!Q', attrs['size'])
        struct.error: cannot convert argument to long
}}}
(full report by Jody Harris [http://allmydata.org/pipermail/tahoe-dev/2010-February/003831.html here]).

This happens because {{{childnode.get_size()}}} does not always return an integer. At least for mutable files, it can't, because the size isn't immediately available and the return value would have to be a Deferred. (For directories, the size is reported as 1 byte.)

There are two places where sftpd.py can end up putting a result of {{{get_size()}}} in a field that expects an integer: {{{attrs['size']}}} in [source:src/allmydata/frontends/sftpd.py?rev=4119#L328 _populate_attrs], and {{{s.st_size}}} in [source:src/allmydata/frontends/sftpd.py?rev=4119#L254 openDirectory].

It is possible to use {{{get_current_size()}}} instead of {{{get_size()}}}, which does return a Deferred -- but that would involve extra network round trips to get the size of each mutable file, significantly slowing down directory listings even though the sizes are not usually very important (as mentioned in ticket:677#comment:2).

This is essentially the same problem as ticket #680 for FTP. However  the SFTP protocol, unlike FTP, has the option [http://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#section-7.1 not to include] the size in the attributes for each directory entry. We should try that first and see whether SFTP clients handle it correctly. (It appears that sshfs will treat the size as zero anyway -- see line 619 [http://fuse.cvs.sourceforge.net/viewvc/fuse/sshfs/sshfs.c?revision=1.104&view=markup here] -- but at least we're allowing other clients the opportunity to avoid showing the user an incorrect size.)

For stat requests that apply to a single file ({{{SSH_FXP_STAT}}}/{{{LSTAT}}}/{{{FSTAT}}}), we should get the current size -- this is similar to getting the current size for {{{t=json}}} webapi requests (#677).

It seems based on [source:src/allmydata/frontends/ftpd.py?rev=4210#L188 _populate_row] in ftpd.py that FTP is likely to have the same issue. Probably all uses of {{{get_size()}}} in non-test code need to be checked.

(Thanks to Zooko and Brian for thorough IRC discussion of this bug and #680.)"	defect	closed	critical	1.7.0	code-frontend	1.6.0	fixed	reliability performance sftp docs		
