Review Request: Fix cookie expiration date parsing in KCookieJar...

[Benjamin Meyer]

Looking at the current code there are plenty of real cases that are being missed.

Earlier this year I went through and re-wrote the expiration date parser for QCookieJar based upon bug reports I received in Arora.  Investigating the source code in Firefox I created a new date parser which can be found in the parseDateString() function in qnetworkcookie.cpp.  It is compatible with the behavior of other browsers, while still being readable code.  I went through a number of revisions and am proud of the final result.

http://qt.gitorious.org/qt/qt/blobs/HEAD/src/network/access/qnetworkcookie.cpp

I wrote an extensive set of autotests including some that I saw in bug reports

http://qt.gitorious.org/qt/qt/blobs/master/tests/auto/qnetworkcookie/tst_qnetworkcookie.cpp

I would suggest running these autotests on this parser as there are many sites out there that do things like "31 11 06" or "31 11 01" where you need to be compatible with not the spec but the behavior of major browsers. 

-Benjamin Meyer

On Dec 17, 2009, at 6:37 PM, adawit at kde.org wrote:

> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2404/
> -----------------------------------------------------------
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This patch addresses the issue of parsing cookie expiration date correctly. Failing to parse expiration dates properly results in the cookie being treated as a session cookie which results in many unintended side effects similar to those reported in the bug reports listed above.
> 
> 
> This addresses bugs 19318, 145244, 176731, and 187792.
>    https://bugs.kde.org/show_bug.cgi?id=19318
>    https://bugs.kde.org/show_bug.cgi?id=145244
>    https://bugs.kde.org/show_bug.cgi?id=176731
>    https://bugs.kde.org/show_bug.cgi?id=187792
> 
> 
> Diffs
> -----
> 
>  /trunk/KDE/kdelibs/kioslave/http/kcookiejar/kcookiejar.cpp 1063375 
>  /trunk/KDE/kdelibs/kioslave/http/kcookiejar/tests/cookie.test 1063374 
> 
> Diff: http://reviewboard.kde.org/r/2404/diff
> 
> 
> Testing
> -------
> 
> * kcookiejar unit test.
> * Spot check with few sites that set cookies based on a "remember me" option.
> 
> 
> Thanks,
> 
> adawit
>