Review Request: HTTP ioslave: do not require incoming headers to have spaces after colon

Dawit Alemayehu adawit at kde.org
Tue Apr 17 12:30:56 BST 2012



> On April 16, 2012, 11:34 p.m., Dawit Alemayehu wrote:
> > But your change now makes it possible to have a header without any spaces after the colon as well. Despite that only affecting headers from the cache, which what your patch does, I do not see the point here. Do you want to support cases where there is no space between the ':' and the header content ?
> 
> Rolf Eike Beer wrote:
>     Sorry, that sentence from the description got lost somehow:
>     
>     While we already support "any" as ">= 1, spaces or tabs" by calling trimmed() before checking we have been failing for "any" as in "nothing".
>     
>     So: yes, this is the whole purpose of this patch.

Well in that case I have no objections. Feel free to commit it. While you are at it, can you please remove the expensive toLower() operation at the beginning of that for loop and replace it with Qt::CaseInsensitive inside of the startsWith comparisons ? Also the QString usages inside of the for loop should be declared with const to force the compiler to optimize them away.


- Dawit


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


On April 16, 2012, 8:30 p.m., Rolf Eike Beer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104625/
> -----------------------------------------------------------
> 
> (Updated April 16, 2012, 8:30 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> The RfCs suggest to use a single space after the colon in HTTP headers, but any amount of whitespace is allowed. This has been fixed for Content-Disposition headers a while ago. Now also fix it for the other headers we explicitely check here.
> 
> 
> Diffs
> -----
> 
>   kioslave/http/http.cpp f7aa857 
> 
> Diff: http://git.reviewboard.kde.org/r/104625/diff/
> 
> 
> Testing
> -------
> 
> Still compiles and I see no breakage.
> 
> 
> Thanks,
> 
> Rolf Eike Beer
> 
>

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


More information about the kde-core-devel mailing list