Review Request: PATCH: Fix most of the login issues with the FTP ioslave...

[Dawit Alemayehu]

> On April 27, 2011, 11:42 p.m., David Faure wrote:
> > kioslave/ftp/ftp.cpp, line 367
> > <http://git.reviewboard.kde.org/r/101173/diff/4/?file=15510#file15510line367>
> >
> >     Sorry, I still have a question.
> >     
> >     After a username change, setHost is going to be called, right? So it will disconnect kio_ftp from the host?
> >     
> >     In that case, what's the point in doing the redirection code in this method - I thought the point was to do it after connected(), but that can't be it.
> >     So it still seems to me that the redirection code could be done inside of ftpLogin, which would simplify the code quite a lot (no bool *, in particular).
> >     
> >     But I guess I'm missing something; maybe setHost isn't called, after a redirection, so the app simply reuses the already-connected slave? In that case I don't see a better solution, indeed.
> >

#1. After the user name changed redirection, setHost will be called. But why do you think kio_ftp will be disconnected from the server ? If that is because of the closeConnection() call in setHost, then rest assured that it will not be called. Why ? Because "m_user" has already been updated to the new username and since nothing else will change, the if statement in setHost will never be true.

#2. We want the redirection to be the same as any other redirection. The only difference being we only want to update the client with the new user name. As per response in #1, the ioslave already has updated its local copy of the user name (m_user) ; so eveything else is exactly the same as the other redirection.

If you attempt to move the redirection to ftpLogin, you have to throw away your successful connection to the server, because you have to return false, i.e. not logged in to ftpOpenConnection.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101173/#review2935
-----------------------------------------------------------


On April 27, 2011, 9:27 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101173/
> -----------------------------------------------------------
> 
> (Updated April 27, 2011, 9:27 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> -------
> 
> The attached patch addresses most of the FTP login related problems and is a replacement for the previous review request
> https://git.reviewboard.kde.org/r/100873/. Here are all the changes in this patch:
> 
> - Show the "Remember password" checkbox even after the failure of the first login attempt. [Bug:258888]
> - Always check for cached password before trying to login anonymously unless the "TryAnonymousLoginFirst"
>   flag was set in kio_ftprc. [Bug: 99686, 143488, 124675]
> - Avoid sending the "anonymous" username so it will not be used in the key used to store the password in kwallet.
> - When a url contains a username, but the user chooses to login with a different username in the password dialog, 
>   then use redirection to update the client of the change.
> - Store password information in persistent storage if and only if the user checked the "Remember password" checkbox.
> 
> 
> This addresses bugs 99686, 124675, 143488, and 258888.
>     http://bugs.kde.org/show_bug.cgi?id=99686
>     http://bugs.kde.org/show_bug.cgi?id=124675
>     http://bugs.kde.org/show_bug.cgi?id=143488
>     http://bugs.kde.org/show_bug.cgi?id=258888
> 
> 
> Diffs
> -----
> 
>   kioslave/ftp/ftp.h 4ccdd4c 
>   kioslave/ftp/ftp.cpp f7db42b 
> 
> Diff: http://git.reviewboard.kde.org/r/101173/diff
> 
> 
> Testing
> -------
> 
> - Attempt to login with incorrect username and validate the "Remember password" is actually shown again.
> - Corrected the username information from the password dialog to ensure the client is updated properly about the password change.
> - Clicked on the "Remember password" to store password in persistent storage and retry logging into the same server at a later point.
> 
> 
> Thanks,
> 
> Dawit
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110428/382ac153/attachment.htm>