ftp "ls" command is broken

[warner]

While testing the fix for #2388, I noticed that FTP's "ls" command is broken. Apparently it was broken in 1.10.0 too, so I'm marking this as a release-blocking regression. I suspect it has to do with a change in Twisted, though, so maybe it worked at one point and we just got caught by an API change.

(ve)206:warner@brian-office-mini% ftp -P 8021 alice@localhost
Trying 127.0.0.1...
Connected to localhost.
220 Twisted 15.0.0 FTP Server
331 Password required for alice.
Password:
230 User logged in, proceed
Remote system type is UNIX.
Using binary mode to transfer files.
ftp> ls
227 Entering Passive Mode (127,0,0,1,220,56).
125 Data connection already open, starting transfer
<HANG>

twistd.log:

2015-03-24 10:16:13-0700 [-] Unexpected FTP error
2015-03-24 10:16:13-0700 [-] Unhandled Error
	Traceback (most recent call last):
	  File "/usr/local/lib/python2.7/site-packages/twisted/internet/base.py", line 824, in runUntilCurrent
	    call.func(*call.args, **call.kw)
	  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/foolscap-0.7.0-py2.7.egg/foolscap/eventual.py", line 26, in _turn
	    cb(*args, **kwargs)
	  File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 383, in callback
	    self._startRunCallbacks(result)
	  File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 491, in _startRunCallbacks
	    self._runCallbacks()
	--- <exception caught here> ---
	  File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 578, in _runCallbacks
	    current.result = callback(current.result, *args, **kw)
	  File "/usr/local/lib/python2.7/site-packages/twisted/protocols/ftp.py", line 996, in gotListing
	    self.dtpInstance.sendListResponse(name, attrs)
	  File "/usr/local/lib/python2.7/site-packages/twisted/protocols/ftp.py", line 474, in sendListResponse
	    self.sendLine(self._formatOneListResponse(name, *response))
	  File "/usr/local/lib/python2.7/site-packages/twisted/protocols/ftp.py", line 464, in _formatOneListResponse
	    'permissions': permissions.shorthand(),
	exceptions.AttributeError: 'int' object has no attribute 'shorthand'

I'm guessing this involves allmydata.frontends.ftpd.Handler._populate_row, where it returns a fake (int) 0600 as the "permissions" key for all rows. I'm further guessing that Twisted's changed the API to require some sort of Permissions object.

We need to check the range of Twisteds with which we claim compatibility, look at the variety of ftp.IFTPShell interfaces required by that set, and find a way to be compatible with all of them.

[daira]

This looks like it might be due to ​https://twistedmatrix.com/trac/changeset/42473/trunk/twisted/protocols/ftp.py, which changed the permissions to be represented as a twisted.python.filepath.Permissions object.

[warner]

Yeah, I think that's it. Twisted-14.0.2 expected an int, Twisted-15.0.0 expects a Permissions (and the docstring is still out of date: ​https://twistedmatrix.com/trac/ticket/7833).

I don't see any clean way to test which version of Twisted is calling us, so I think I'm going to use an ugly hack. The old Twisted uses this function to render the integer mode value (​https://github.com/twisted/twisted/blob/twisted-14.0.2/twisted/protocols/ftp.py#L428):

def formatMode(mode):
    return ''.join([mode & (256 >> n) and 'rwx'[n % 3] or '-' for n in range(9)])

and the new Twisted uses this (​https://github.com/twisted/twisted/blob/twisted-15.0.0/twisted/protocols/ftp.py#L464)

'permissions': permissions.shorthand(),

So I'm thinking we create a subclass of Permissions that overrides the __and__ operator to return an int for the first form, but has a .shorthand() for the second form.

Evil! :-)

[warner]

Hrm, we support back to Twisted-11.0.0 (on windows), which didn't have Permissions (it was introduced in 11.1.0).

[warner]

​https://github.com/tahoe-lafs/tahoe-lafs/pull/148 has a branch for review. The evil hack seems to work, but if someone could also test it on a box with Twisted-11.0.0 (which will require windows, I think), I'd appreciate it.

[daira]

We decided to simplify the hack by requiring Twisted >= 11.1.0 on Windows. This also simplifies the test in test_ftp.test_list, because we don't need to account for the permissions value being either an IntishPermissions or an int.

[warner]

Landed in d7b763c