Review Request: Fix whitespace related bugs when listing directories in kio_ftp

[Dawit Alemayehu]

> On Oct. 16, 2012, 11:50 a.m., David Faure wrote:
> > kioslave/ftp/ftp.cpp, line 1491
> > <http://git.reviewboard.kde.org/r/106636/diff/3/?file=88065#file88065line1491>
> >
> >     This is slow iteration, since it has to modify the container every time. There's no reentrancy here, why use this complicated approach, when a simple Q_FOREACH could do?

I did not want to use Q_FOREACH here because the items we are iterating over need to be modified by calls to fixupEntryName. I wanted to avoid copying each item on the list if I could, but your point is valid. Modifying the list would be more expensive ; so I have opted for using a standard for-loop to iterate over the loop and access the list items using the [] operator. That way we perform neither on of the more expensive operations.


> On Oct. 16, 2012, 11:50 a.m., David Faure wrote:
> > kioslave/ftp/ftp.cpp, line 1493
> > <http://git.reviewboard.kde.org/r/106636/diff/3/?file=88065#file88065line1493>
> >
> >     Once bFound is true, we're done. Why not break out of the loop, then? (and remove this if()) ?
> >     
> >     This is unlike the loop you're copying from, which has to keep processing the directory listing from the server.


- Dawit


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


On Oct. 10, 2012, 3:55 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106636/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2012, 3:55 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Description
> -------
> 
> The attached patch fixes a regression caused by a commit to fix bug# 88575. Namely, it fixed a problem where filenames with whitespaces in them were not handled correctly by kio_ftp. That is because the filenames were automatically trimmed when read from the directory. However, the fix then re-introduced the original bug and the reason why names were automatically trimmed in the first place. Some ftp servers add bogus whitespace between the date and filename in their listings. Hence, we need need to fix both of these opposing issues without breaking the other. This patch tries to do just that by actually validating each name entry that starts with a whitespace. That way the correct name is sent to the client.
> 
> 
> This addresses bug 300988.
>     http://bugs.kde.org/show_bug.cgi?id=300988
> 
> 
> Diffs
> -----
> 
>   kioslave/ftp/ftp.h 2465a4b 
>   kioslave/ftp/ftp.cpp 26be283 
> 
> Diff: http://git.reviewboard.kde.org/r/106636/diff/
> 
> 
> Testing
> -------
> 
> - Create 150 folders and 150 text files whose name starts with a whitespace in a directory.
> - Start your preferred ftp server (I tested with vsftpd) on your local machine and login.
> - Make sure you can browse the directory that contains the text files and folders you created above just as quickly as you can other ftp sites.
> - Rename some of the folders and text files by remove the leading whitespace from them and check if that is handled properly.
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

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