tahoe-lafs/trac-2024-07-25

ftp "ls" command is broken #2394

New issue

Closed

opened 2015-03-24 17:25:18 +00:00 by warner · 6 comments

warner commented

2015-03-24 17:25:18 +00:00

Owner

(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.

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.

tahoe-lafs added the

code-frontend-ftp-sftp

normal

defect

1.10.0

labels 2015-03-24 17:25:18 +00:00

tahoe-lafs added this to the 1.10.1 milestone 2015-03-24 17:25:18 +00:00

daira commented

2015-03-24 18:25:41 +00:00

Author

Owner

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.

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 commented

2015-03-26 01:03:26 +00:00

Author

Owner

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! :-)

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 commented

2015-03-26 01:10:47 +00:00

Author

Owner

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

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

warner commented

2015-03-26 01:34:38 +00:00

Author

Owner

(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 commented

2015-03-31 16:45:35 +00:00

Author

Owner

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.

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 commented

2015-03-31 18:11:14 +00:00

Author

Owner

Landed in d7b763c

tahoe-lafs added the

fixed

label 2015-03-31 18:11:14 +00:00

warner closed this issue

2015-03-31 18:11:14 +00:00