Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
  • 6 commits
  • 3 files changed
  • 0 commit comments
  • 1 contributor
Showing with 172 additions and 19 deletions.
  1. +3 −0  .gitignore
  2. +78 −16 twisted/protocols/ftp.py
  3. +91 −3 twisted/test/test_ftp.py
View
3  .gitignore
@@ -0,0 +1,3 @@
+*.pyc
+_trial_temp/
+twisted/plugins/dropin.cache
View
94 twisted/protocols/ftp.py
@@ -25,7 +25,7 @@
# Twisted Imports
from twisted import copyright
-from twisted.internet import reactor, interfaces, protocol, error, defer
+from twisted.internet import reactor, interfaces, protocol, task, error, defer
from twisted.protocols import basic, policies
from twisted.python import log, failure, filepath
@@ -72,7 +72,8 @@
SVC_NOT_AVAIL_CLOSING_CTRL_CNX = "421.1"
TOO_MANY_CONNECTIONS = "421.2"
-CANT_OPEN_DATA_CNX = "425"
+CANT_OPEN_DATA_CNX = "425.1"
+DTP_TIMEOUT = "425.2"
CNX_CLOSED_TXFR_ABORTED = "426"
REQ_ACTN_ABRTD_FILE_UNAVAIL = "450"
REQ_ACTN_ABRTD_LOCAL_ERR = "451"
@@ -141,6 +142,7 @@
SVC_NOT_AVAIL_CLOSING_CTRL_CNX: '421 Service not available, closing control connection.',
TOO_MANY_CONNECTIONS: '421 Too many users right now, try again in a few minutes.',
CANT_OPEN_DATA_CNX: "425 Can't open data connection.",
+ DTP_TIMEOUT: '425 Data channel initialization timed out.',
CNX_CLOSED_TXFR_ABORTED: '426 Transfer aborted. Data connection closed.',
REQ_ACTN_ABRTD_FILE_UNAVAIL: '450 Requested action aborted. File unavailable.',
@@ -648,6 +650,10 @@ class FTP(object, basic.LineReceiver, policies.TimeoutMixin):
# States an FTP can be in
UNAUTH, INAUTH, AUTHED, RENAMING = range(4)
+ # Human readeable text listing command that can be used for setting up a
+ # data connection channel.
+ DATA_CONNECTION_COMMANDS = 'PORT or PASV'
+
# how long the DTP waits for a connection
dtpTimeout = 10
@@ -777,6 +783,18 @@ def getDTPPort(self, factory):
(self.passivePortRange,))
+ def checkDataTransportStarted(self, command):
+ '''Checks that data transport is ready.
+
+ If data transport was not requeted using PORT, PASV etc it raises
+ L{BadCmdSequenceError}.
+ '''
+ if self.dtpInstance is None:
+ raise BadCmdSequenceError(
+ '%s required before %s' % (
+ self.DATA_CONNECTION_COMMANDS, command,))
+
+
def ftp_USER(self, username):
"""
First part of login. Get the username the peer wants to
@@ -884,9 +902,7 @@ def ftp_LIST(self, path=''):
file. A null argument implies the user's current working or
default directory.
"""
- # Uh, for now, do this retarded thing.
- if self.dtpInstance is None or not self.dtpInstance.isConnected:
- return defer.fail(BadCmdSequenceError('must send PORT or PASV before RETR'))
+ self.checkDataTransportStarted('LIST')
# bug in konqueror
if path == "-a":
@@ -917,6 +933,7 @@ def gotListing(results):
segments,
('size', 'directory', 'permissions', 'hardlinks',
'modified', 'owner', 'group'))
+ d.addCallback(self._cbWaitDTPConnectionWithTimeout)
d.addCallback(gotListing)
return d
@@ -936,11 +953,7 @@ def ftp_NLST(self, path):
@return: a L{Deferred} which will be fired when the listing request
is finished.
"""
- # XXX: why is this check different from ftp_RETR/ftp_STOR? See #4180
- if self.dtpInstance is None or not self.dtpInstance.isConnected:
- return defer.fail(
- BadCmdSequenceError('must send PORT or PASV before RETR'))
-
+ self.checkDataTransportStarted('NLST')
try:
segments = toSegments(self.workingDirectory, path)
except InvalidPath:
@@ -987,6 +1000,7 @@ def listErr(results):
@returns: A C{tuple} containing the status code for a successful
transfer.
"""
+ log.err(results)
self.dtpInstance.transport.loseConnection()
return (TXFR_COMPLETE_OK,)
@@ -995,14 +1009,48 @@ def listErr(results):
'*' in segments[-1] or '?' in segments[-1] or
('[' in segments[-1] and ']' in segments[-1])):
d = self.shell.list(segments[:-1])
+ d.addCallback(self._cbWaitDTPConnectionWithTimeout)
d.addCallback(cbGlob)
else:
d = self.shell.list(segments)
+ d.addCallback(self._cbWaitDTPConnectionWithTimeout)
d.addCallback(cbList)
- # self.shell.list will generate an error if the path is invalid
- d.addErrback(listErr)
+
+ # self.shell.list will generate an error if the path is invalid
+ d.addErrback(listErr)
return d
+ def _cbWaitDTPConnectionWithTimeout(self, result):
+ """Helper callback that waits for DTP instance to be connected.
+
+ It will raise a {PortConnectionError} if DTP instance is not
+ connected after the interval defined by self.factory.timeOut.
+ """
+ def cancelTimeout():
+ if timout_call is not None and timout_call.active():
+ log.msg('cancelling DTP timeout', debug=True)
+ return timout_call.cancel()
+
+ def cbCheckDTPInstance(value):
+ check_result = None
+
+ if timout_call.called:
+ self.reply(DTP_TIMEOUT)
+ check_result = defer.fail(
+ PortConnectionError(
+ defer.TimeoutError("DTP connection timeout")))
+ elif self.dtpInstance and self.dtpInstance.isConnected:
+ cancelTimeout()
+ check_result = result
+ else:
+ check_result = task.deferLater(
+ reactor, 0.1, cbCheckDTPInstance, None)
+
+ return check_result
+
+ timout_call = reactor.callLater(
+ self.factory.timeOut, lambda ignore: ignore, None)
+ return cbCheckDTPInstance(None)
def ftp_CWD(self, path):
try:
@@ -1038,8 +1086,7 @@ def ftp_RETR(self, path):
@rtype: L{Deferred}
@return: a L{Deferred} which will be fired when the transfer is done.
"""
- if self.dtpInstance is None:
- raise BadCmdSequenceError('PORT or PASV required before RETR')
+ self.checkDataTransportStarted('RETR')
try:
newsegs = toSegments(self.workingDirectory, path)
@@ -1098,8 +1145,23 @@ def ebOpened(err):
def ftp_STOR(self, path):
- if self.dtpInstance is None:
- raise BadCmdSequenceError('PORT or PASV required before STOR')
+ """
+ This command causes the server-DTP to accept the data
+ transferred via the data connection and to store the data as
+ a file at the server site. If the file specified in the
+ pathname exists at the server site, then its contents shall
+ be replaced by the data being transferred. A new file is
+ created at the server site if the file specified in the
+ pathname does not already exist.
+
+ @type path: C{str}
+ @param path: The file path where the content should be stored.
+
+ @rtype: L{Deferred}
+ @return: a L{Deferred} which will be fired when the transfer
+ is finished.
+ """
+ self.checkDataTransportStarted('STOR')
try:
newsegs = toSegments(self.workingDirectory, path)
View
94 twisted/test/test_ftp.py
@@ -399,6 +399,84 @@ def test_portRangeInheritedFromFactory(self):
protocol = self.factory.buildProtocol(None)
self.assertEqual(portRange, protocol.wrappedProtocol.passivePortRange)
+ def _startDataConnection(self):
+ '''Prepare data transport protocol to look like it was created by
+ a previous call to PASV or PORT'''
+ dtp_factory = ftp.DTPFactory(self.serverProtocol)
+ dtp_factory.buildProtocol('ignore_address')
+ dtp_transport = proto_helpers.StringTransportWithDisconnection()
+ dtp_transport.protocol = ftp.DTP()
+ self.serverProtocol.dtpInstance.transport = dtp_transport
+
+ def _checkTimeoutLog(self, result):
+ '''Check the log after a timeout error.'''
+ current_errors = self.flushLoggedErrors()
+ self.assertEqual(1, len(current_errors))
+ self.assertIsInstance(
+ current_errors[0].value, ftp.PortConnectionError)
+
+ def test_LISTWithoutDataChannel(self):
+ """
+ Calling LIST without prior setup of data connection will result in a
+ incorrect sequence of commands error.
+ """
+ d = self._anonymousLogin()
+ self.assertCommandFailed(
+ 'LIST .',
+ ["503 Incorrect sequence of commands: "
+ "PORT or PASV required before LIST"],
+ chainDeferred=d)
+ return d
+
+ def test_LISTTimeout(self):
+ """
+ LIST will timeout if setting up the DTP instance will take to long.
+ """
+ d = self._anonymousLogin()
+ self._startDataConnection()
+
+ # Set timeout to a very small value to not slow down tests.
+ self.serverProtocol.factory.timeOut = 0.01
+
+ self.assertCommandFailed(
+ 'LIST .',
+ ["425 Data channel initialization timed out."],
+ chainDeferred=d)
+
+ d.addCallback(self._checkTimeoutLog)
+ return d
+
+ def test_NLSTWithoutDataChannel(self):
+ """
+ Calling NLST without prior setup of data connection will result in a
+ incorrect sequence of commands error.
+ """
+ d = self._anonymousLogin()
+ self.assertCommandFailed(
+ 'NLST .',
+ ["503 Incorrect sequence of commands: "
+ "PORT or PASV required before NLST"],
+ chainDeferred=d)
+ return d
+
+ def test_NLSTTimeout(self):
+ """
+ NLST will timeout if setting up the DTP instance will take to long.
+ """
+ d = self._anonymousLogin()
+
+ self._startDataConnection()
+
+ # Set timeout to a very small value to not slow down tests.
+ self.serverProtocol.factory.timeOut = 0.01
+
+ self.assertCommandFailed(
+ 'NLST .',
+ ["425 Data channel initialization timed out."],
+ chainDeferred=d)
+
+ d.addCallback(self._checkTimeoutLog)
+ return d
class FTPServerTestCaseAdvancedClient(FTPServerTestCase):
@@ -604,13 +682,23 @@ def test_NLSTNonexistent(self):
"""
NLST on a non-existent file/directory returns nothing.
"""
+ def checkDownload(download):
+ self.assertEqual('', download)
+
+ def checkErrorLog(result):
+ current_errors = self.flushLoggedErrors()
+ self.assertEqual(1, len(current_errors))
+ self.assertIsInstance(
+ current_errors[0].value, ftp.FileNotFoundError)
+
# Login
d = self._anonymousLogin()
self._download('NLST nonexistent.txt', chainDeferred=d)
- def checkDownload(download):
- self.assertEqual('', download)
- return d.addCallback(checkDownload)
+
+ d.addCallback(checkDownload)
+ d.addCallback(checkErrorLog)
+ return d
def test_NLSTOnPathToFile(self):

No commit comments for this range

Something went wrong with that request. Please try again.