#2394 closed defect (fixed)

ftp "ls" command is broken

Reported by: warner Owned by: warner
Priority: normal Milestone: 1.10.1
Component: code-frontend-ftp-sftp Version: 1.10.0
Keywords: ftpd regression Cc:
Launchpad Bug:


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
Connected to localhost.
220 Twisted 15.0.0 FTP Server
331 Password required for alice.
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


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
	  File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 491, in _startRunCallbacks
	--- <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.

Change History (7)

comment:1 Changed at 2015-03-24T18:15:56Z by daira

  • Keywords blocks-release added

comment:2 Changed at 2015-03-24T18:25:41Z by 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.

comment:3 Changed at 2015-03-26T01:03:26Z by 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! :-)

comment:4 Changed at 2015-03-26T01:10:47Z by warner

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

comment:5 Changed at 2015-03-26T01:34:38Z by warner

  • Keywords review-needed added
  • Owner set to warner
  • Status changed from new to assigned

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.

comment:6 Changed at 2015-03-31T16:45:35Z by 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.

comment:7 Changed at 2015-03-31T18:11:14Z by warner

  • Keywords blocks-release review-needed removed
  • Resolution set to fixed
  • Status changed from assigned to closed

Landed in d7b763c

Note: See TracTickets for help on using tickets.