On Tuesday 04 November 2014 21:47:36 you wrote: > On Sunday 02 November 2014 13:43:50 Àlex Fiestas wrote: > > Hi there > > > > There are quite a few places where the following code is found: > > > > if (!url.path().endsWith('/')) { > > > > url.setPath(url.path() + '/'); > > > > } > > Right. > > > Given an url like: 'scheme://' KUrl will return '/' as path > > Wrong. > > http://www.davidfaure.fr/2014/kurltest.cpp.diff > > KUrl remote2("remote://"); > remote2.path() is empty, not "/". > > > while QUrl returns empty string. > > Just like KUrl, yes. There is no path in scheme://. > > Reminder, it's scheme://optional_hostname/path (where path actually includes > the leading slash). > > > This is making kio add a third slash to the url in many places (because of > > code like the one pasted before). > > Right. Indeed, the above code turns "no path" into path=='/', which is wrong > they mean different things. The canonical example is ftp://host (on a non- > anonymous ftp server) which usually redirects to ftp://host/home/user > > As a result if you open dolphin and type smb://, it will be redirected to > > smb:///. > > I forgot the details of smb urls. I know it's tricky though ;) > And yeah, a path of '/' sounds wrong. > > > Is this an intended behavior or should I start sending patches to prevent > > this from happening? > > Well, this shows that we might need something in QUrl. The repetition of the > above 3 lines is already bad enough, anything more complex will surely be > wrong in many cases... > > KUrl had AppendTrailingSlash, QUrl is missing that (e.g. in adjusted()). > > But wait. We can only append to the path after we already connected > (to give a chance to ftp://host to redirect to ftp://host/home/user). > If we start doing any sort of path concatenation at that point, we'll mess > up. Imagine the complete code was changed to > if (!url.path().isEmpty() && !url.path().endsWith('/')) { > url.setPath(url.path() + '/'); > } > and that this is followed by > url.setPath(url.path() + "file"); > then we'll end up with smb://file. > > So I guess this is only happening in cases where we want to normalize all > paths (to end with a slash) before we even call listDir ... so that would be > in KDirLister::openUrl? With a bit of luck this is basically the only place > where this logic would be needed? Well, all of KDirLister, since the whole > point is to be able to rely on the fact that the url to a directory ends > with a '/', in there. If this is true, then I would suggest a helper > function in kdirlister.cpp. If more code needs this, please explain, and it > will be a reason for putting it in QUrl.
I have been taking a look at this today and I have to confess that I am clueless. kdirlister does not have any logic regarding paths right now. KCoreDirLister (which kdirlister inhertis from) does but it is not adding any slash as far as I could see and on all places at least within openUrl the url is not modified (aka no extra slash is added). Also, if you run "dolphin smb://" a weird thing will happen, the widget showing the url will display "smb:///" while the error message (if any) will show smb://. I am going to try to figure out a testcase for this, dolphin is too big for me to even attempt to solve this bug. Cheers!
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel