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 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,,