Rev 4728: Fix test failure at the root without cleaning up ftp APPE. in file:///home/vila/src/bzr/bugs/443041-ftp-append-bytes/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Tue Oct 6 09:24:14 BST 2009
At file:///home/vila/src/bzr/bugs/443041-ftp-append-bytes/
------------------------------------------------------------
revno: 4728
revision-id: v.ladeuil+lp at free.fr-20091006082414-2gw236e7n81hppxp
parent: v.ladeuil+lp at free.fr-20091005143235-bjra3xnhacfismc4
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 443041-ftp-append-bytes
timestamp: Tue 2009-10-06 10:24:14 +0200
message:
Fix test failure at the root without cleaning up ftp APPE.
* bzrlib/transport/ftp/_gssapi.py:
(GSSAPIFtpTransport.connection_class, GSSAPIFtpTransport._login):
Reduce the duplication that led to bug #443041: the connection
wasn't set to bianry mode after its creation.
* bzrlib/transport/ftp/__init__.py:
(FtpTransport._create_connection): Made more pluggable for GSSAPI
purposes.
(FtpTransport.connection_class, FtpTransport._login): The parts
that GSSAPI want to override.
(FtpTransport._try_append): Revert controversial change.
* bzrlib/tests/ftp_server/pyftpdlib_based.py:
(BzrConformingFTPHandler.ftp_NLST): Fix typo.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2009-10-05 14:29:46 +0000
+++ b/NEWS 2009-10-06 08:24:14 +0000
@@ -101,8 +101,9 @@
with some combinations of remote and local formats. This was causing
"unknown object type identifier 60" errors. (Andrew Bennetts, #427736)
-* ftp APPE command was not cleanly handled in all cases and buffered too
- much too. (Vincent Ladeuil, #443041)
+* ftp transports were built differently when the kerberos python module was
+ present leading to obscure failures related to ASCII/BINARY modes.
+ (Vincent Ladeuil, #443041)
* Network streams now decode adjacent records of the same type into a
single stream, reducing layering churn. (Robert Collins)
=== modified file 'bzrlib/tests/ftp_server/pyftpdlib_based.py'
--- a/bzrlib/tests/ftp_server/pyftpdlib_based.py 2009-10-05 14:32:35 +0000
+++ b/bzrlib/tests/ftp_server/pyftpdlib_based.py 2009-10-06 08:24:14 +0000
@@ -84,7 +84,7 @@
line = self.fs.fs2ftp(path)
if self.fs.isfile(self.fs.realpath(path)):
why = "Not a directory: %s" % line
- self.log('FAIL SIZE "%s". %s.' % (line, why))
+ self.log('FAIL NLST "%s". %s.' % (line, why))
self.respond("550 %s." %why)
else:
ftpserver.FTPHandler.ftp_NLST(self, path)
=== modified file 'bzrlib/transport/ftp/__init__.py'
--- a/bzrlib/transport/ftp/__init__.py 2009-10-05 14:29:46 +0000
+++ b/bzrlib/transport/ftp/__init__.py 2009-10-06 08:24:14 +0000
@@ -108,6 +108,8 @@
self._set_connection(connection, credentials)
return connection
+ connection_class = ftplib.FTP
+
def _create_connection(self, credentials=None):
"""Create a new connection with the provided credentials.
@@ -133,13 +135,9 @@
((self._host, self._port, user, '********',
self.is_active),))
try:
- connection = ftplib.FTP()
+ connection = self.connection_class()
connection.connect(host=self._host, port=self._port)
- if user and user != 'anonymous' and \
- password is None: # '' is a valid password
- password = auth.get_password('ftp', self._host, user,
- port=self._port)
- connection.login(user=user, passwd=password)
+ self._login(connection, auth, user, password)
connection.set_pasv(not self.is_active)
# binary mode is the default
connection.voidcmd('TYPE I')
@@ -152,6 +150,13 @@
" %s" % str(e), orig_error=e)
return connection, (user, password)
+ def _login(self, connection, auth, user, password):
+ # '' is a valid password
+ if user and user != 'anonymous' and password is None:
+ password = auth.get_password('ftp', self._host,
+ user, port=self._port)
+ connection.login(user=user, passwd=password)
+
def _reconnect(self):
"""Create a new connection with the previously used credentials"""
credentials = self._get_credentials()
@@ -385,6 +390,7 @@
"""Append the text in the file-like object into the final
location.
"""
+ text = f.read()
abspath = self._remote_path(relpath)
if self.has(relpath):
ftp = self._get_FTP()
@@ -394,13 +400,13 @@
if self._has_append:
mutter("FTP appe to %s", abspath)
- self._try_append(relpath, f, mode)
+ self._try_append(relpath, text, mode)
else:
- self._fallback_append(relpath, f, mode)
+ self._fallback_append(relpath, text, mode)
return result
- def _try_append(self, relpath, fp, mode=None, retries=0):
+ def _try_append(self, relpath, text, mode=None, retries=0):
"""Try repeatedly to append the given text to the file at relpath.
This is a recursive function. On errors, it will be called until the
@@ -410,18 +416,19 @@
abspath = self._remote_path(relpath)
mutter("FTP appe (try %d) to %s", retries, abspath)
ftp = self._get_FTP()
- starting_at = fp.tell()
- ftp.storbinary("APPE %s" % abspath, fp)
+ cmd = "APPE %s" % abspath
+ conn = ftp.transfercmd(cmd)
+ conn.sendall(text)
+ conn.close()
self._setmode(relpath, mode)
+ ftp.getresp()
except ftplib.error_perm, e:
# Check whether the command is not supported (reply code 502)
if str(e).startswith('502 '):
- warning(
- "FTP server does not support file appending natively. "
- "Performance may be severely degraded! (%s)", e)
+ warning("FTP server does not support file appending natively. "
+ "Performance may be severely degraded! (%s)", e)
self._has_append = False
- fp.seek(starting_at)
- self._fallback_append(relpath, fp, mode)
+ self._fallback_append(relpath, text, mode)
else:
self._translate_perm_error(e, abspath, extra='error appending',
unknown_exc=errors.NoSuchFile)
@@ -433,13 +440,12 @@
else:
warning("FTP temporary error: %s. Retrying.", str(e))
self._reconnect()
- fp.seek(starting_at)
- self._try_append(relpath, fp, mode, retries+1)
+ self._try_append(relpath, text, mode, retries+1)
- def _fallback_append(self, relpath, fp, mode = None):
+ def _fallback_append(self, relpath, text, mode = None):
remote = self.get(relpath)
remote.seek(0, os.SEEK_END)
- osutils.pumpfile(fp, remote)
+ remote.write(text)
remote.seek(0)
return self.put_file(relpath, remote, mode)
=== modified file 'bzrlib/transport/ftp/_gssapi.py'
--- a/bzrlib/transport/ftp/_gssapi.py 2009-09-18 01:25:08 +0000
+++ b/bzrlib/transport/ftp/_gssapi.py 2009-10-06 08:24:14 +0000
@@ -97,52 +97,22 @@
"""
- def _create_connection(self, credentials=None):
- """Create a new connection with the provided credentials.
-
- :param credentials: The credentials needed to establish the connection.
-
- :return: The created connection and its associated credentials.
-
- The credentials are a tuple with the username and password. The
- password is used if GSSAPI Authentication is not available.
+ connection_class = GSSAPIFtp
+
+ def _login(self, connection, auth, user, password):
+ """Login with GSSAPI Authentication.
+
+ The password is used if GSSAPI Authentication is not available.
The username and password can both be None, in which case the
credentials specified in the URL or provided by the
AuthenticationConfig() are used.
"""
- if credentials is None:
- user, password = self._user, self._password
- else:
- user, password = credentials
-
- auth = config.AuthenticationConfig()
- if user is None:
- user = auth.get_user('ftp', self._host, port=self._port,
- default=getpass.getuser())
- mutter("Constructing FTP instance against %r" %
- ((self._host, self._port, user, '********',
- self.is_active),))
try:
- connection = GSSAPIFtp()
- connection.connect(host=self._host, port=self._port)
- try:
- connection.gssapi_login(user=user)
- except ftplib.error_perm, e:
- if user and user != 'anonymous' and \
- password is None: # '' is a valid password
- password = auth.get_password('ftp', self._host, user,
- port=self._port)
- connection.login(user=user, passwd=password)
- connection.set_pasv(not self.is_active)
- except socket.error, e:
- raise errors.SocketConnectionError(self._host, self._port,
- msg='Unable to connect to',
- orig_error= e)
+ connection.gssapi_login(user=user)
except ftplib.error_perm, e:
- raise errors.TransportError(msg="Error setting up connection:"
- " %s" % str(e), orig_error=e)
- return connection, (user, password)
+ super(GSSAPIFtpTransport, self)._login(connection, auth,
+ user, password)
def get_test_permutations():
More information about the bazaar-commits
mailing list