Review Request: Broken redirections in handling of webdav when server requests a redirection from http to https

[Dawit Alemayehu]

> On 2010-01-29 19:57:39, Dawit Alemayehu wrote:
> > Looks ok... You probably need the fallback in case the protocol of the redirection/response url is "webdav" and not "http" ??  Alternatively you can simply replace "http" with "webdav" to avoid having to deal with those checks:
> > 
> > QString protocol (u.protocol());
> > u.setProtocol(protocol.replace(QLatin1String("http"), QLatin1String("webdav"));
> > 
> > Either way it seems fine and should be back ported since it is a fix...
> 
> Matthew Dawson wrote:
>     From the information I've seen, the web server doesn't actually see a webdav://... url, but just a normal looking http request with extra data (like how a POST request works).  So on redirects, it ask to redirect to another http(s) url.  The fallback exist so in case the server sends some other protocol.  My question asked if this is possible.  It would have been caught in the old behaviour, and force webdav://. For example, can the web server redirect me to a telnet:// or ftp:// url?  And is it desirable to force it to webdav?  Or should instead the code ignore other protocols and only change http(s) to webdav(s)?
>     
>     It also catches if the web server doesn't respond with any protocol.  But I don't think that is legal(?).

It should only change http and nothing else. There is nothing that prevents a server from redirecting your request to any other protocol. Whether or not any redirection request to any protocol gets honored or allowed is something that is dealt with in KIO itself. As such, to me, only changing "http" redirects to "webdav" ones makes sense in this case... 


- Dawit


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


On 2010-01-29 15:52:47, Matthew Dawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2760/
> -----------------------------------------------------------
> 
> (Updated 2010-01-29 15:52:47)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> When a website holding a webdav repository automatically redirects all http requests to https, the webdav handler in kio_http forces the request back to http.  The problem occurs as the code assumes the webdav protocol will not change.
> 
> This patch fixes the issue by analyzing the returned url, and sets webdav/webdavs based on whether the server requested a redirection to http/https respectively (it also will handle redirects from webdavs to webdav).  It also includes a fallback to the old behaviour if no protocol is specified, or the protocol is not http/https (is that even a legal request?  If not I can remove the fallback).
> 
> Also, is it ok to backport this patch for KDE SC 4.4?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kioslave/http/http.cpp 1082000 
> 
> Diff: http://reviewboard.kde.org/r/2760/diff
> 
> 
> Testing
> -------
> 
> Locally against a remote server running lighttpd that auto redirects all requests from http to https.  The webdav implementation used comes from webcit, included in the citadel mail server.  Url was:
> 
> webdav://mail.mjdsystems.ca/groupdav
> 
> Tested using kioclient (running kioclient get $url and checking debug output).
> 
> 
> Thanks,
> 
> Matthew
> 
>