[merge] FTP now has and passes regression tests

[John Arbash Meinel]

Wayne Davison wrote:
> In the split_url() function, it would be nice if it would prompt the
> user for a password if no ':' was found in the hostname.  This would
> handle the normal case safely, while forcing someone who wanted an empty
> password (and no prompt) to append a trailing ':' onto the hostname
> (which seems like a fringe case).
> 
> For instance, if something like this were added right before the return:
> 
>     if password == None:
>         password = bzrlib.ui.ui_factory.get_password(
>                     prompt='FTP %(user)s@%(host)s password',
>                     user=username, host=host)
> 
> Doing this should fix bug 34685.
> 
> ..wayne..
> 

Interesting idea, but I don't think it is really the right place. The
problem is that if you are using sftp, and specifically spawning openssh
to connect, then you don't want bzr to prompt you for a password (since
it has no way to pass it to openssh, which always prompts the user
directly).

Also, if you have anonymous ftp, you probably can download stuff
(readonly) without a password.

http doesn't (currently) use split_url, but in the future it might, and
we frequently use anonymous connections there.

I would also probably say, "Only prompt if there is a username". Though
for ssh, again I sometimes have a username, but wouldn't want bzr to
prompt me for a password (unless I was using the built-in paramiko ssh
handler).

Anyway, all this to say that what I would prefer to see is something
more like the attached patch. (paramiko already prompts for a password
at the appropriate time).

I'm not sure about http yet. I think I'll leave that until it matters.

In my little bit of testing, I wasn't able to get ftp to work, because
the return codes don't mean anything (I can't tell if mkdir fails
because the directory already exists, or because the parent directory
doesn't exist). The return strings seem completely implementation
dependent. Where the return code just indicates success/failure, not
really an error code.

John
=:->

=== modified file 'bzrlib/transport/ftp.py'
--- bzrlib/transport/ftp.py
+++ bzrlib/transport/ftp.py
@@ -54,6 +54,7 @@
     )
 import bzrlib.errors as errors
 from bzrlib.trace import mutter, warning
+import bzrlib.ui


 _FTP_cache = {}
@@ -66,6 +67,10 @@
         conn = ftplib.FTP()

         conn.connect(host=hostname, port=port)
+        if username and username != 'anonymous' and not password:
+            password = bzrlib.ui.ui_factory.get_password(
+                prompt='FTP %(user)s@%(host)s password',
+                user=username, host=hostname)
         conn.login(user=username, passwd=password)
         conn.set_pasv(not is_active)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060516/0885ab03/attachment.pgp