[webkit-dev] Curl Cookie Handling

[ARaj]

Hi,
    There is a small problem with that patch.
In the function CookieManager::getCookie(const KURL& url), a shallow local
copy of the cookie map is being created at this line:
                       *HashMap<String, Cookie*> curMap =
it->second->getCookieMap();*

So, when you do a "take" on this, you are essentially removing the
pointer(and subsequently deleting it) from this local copy only. Since the
original m_cookieMap still holds the deleted pointer, the next time this is
accessed, it results in a crash.

An ideal fix for this would be to get a reference from *getCookieMap() *and
use that reference in this function.

Regards,

Raj.

On Wed, Feb 11, 2009 at 1:27 AM, Kevin Ollivier <kevino at theolliviers.com>
wrote:
> Hi Julien,
>
> On Feb 10, 2009, at 7:08 AM, Julien Chaffraix wrote:
>
>> Hi Kevin,
>>
>>> Is this patch still valid, i.e. not made obsolete by another approach?
>>
>> As far as I know, there is no work towards storing the cookies inside
>> the database as it is done in Firefox. Furthermore I know no work in
>> cURL toward exposing the cookies so that we could add / remove /
>> update them simply in our database.
>>
>> This patch may be a bit special because it is not really cURL
>> specific. The only dependency to cURL is a method to parse a date (the
>> 'max-age' field), which I found later is already in JavaScriptCore and
>> would need to be exported to WebCore and maybe need to be adapted.
>> Thus it could be a base toward a cross-platform cookie implementation.
>> I do not know if it would be ok for all ports to replicate their
>> network library's cookie engine.
>
> Well, each port could of course decide for themselves. I know wx would
like
> to use this. :-) I wonder if the GTK or other ports are also interested?
> Maybe they should voice their support if so.
>
>>
>>> Also,
>>> was it a complete patch (sans any bugs, of course) for cookie support
>>> using
>>> SQLite? I could only test it using wx right now but I would definitely
be
>>> interested in using the SQLite approach.
>>
>> It is a complete patch in the sense that it stores and fetch the
>> cookies from the database. About the bug-free part, we have an answer
>> in this thread I guess :-)
>> Basically I could not find a test suite for cookies to validate the
>> approach when I started so I tested it with real world websites
>> (mostly google app suite, yahoo, mapquest and a few other sites) but
>> it did not get the testing it deserves and I will not hide it.
>
> Yes, I did look at your patch earlier but I myself wasn't confident that
I'd
> know how to properly test cookie support, so I hoped someone more
> knowledgeable would step in. :(
>
>> To get it moving, I think a rewrite is necessary because I made tons
>> of small mistakes (I started as a contributor at that time and did not
>> know the code and coding conventions as I do now) that could be more
>> easily tackled by a rewrite. It will also need a test suite to fully
>> valide the changes.
>> All in all, it means a lot of work that I am willing to do it *if* I
>> see enough interest in a cross-platform cookie engine for WebKit.
>
> Well, knowing this project, I doubt you'll find anyone who would object to
> starting a series of unit tests for cookies. :-) From there, it is a
matter
> of reworking your patch, but I can say if there is a test suite I can run
> that handles common usage, I'll feel a lot more comfortable reviewing your
> patch.
>
>>
>>> I know someone a while back proposed a strategy for dealing with port
>>> enhancements that end up bit rotting in the review tree, whatever
>>> happened
>>> to that? Sometimes new feature patches cannot be broken down into
smaller
>>> pieces, and I realize large patches tend to be intimidating especially
to
>>> people who can't test them themselves, but there has to be some strategy
>>> for
>>> dealing with that so that important new stuff doesn't just sit in a
patch
>>> tracker for months or years...
>>
>> I think you refer to the Gtk port here? IIRC the main issue for Gtk
>> was validating the API which is not relevant here.
>
> That sounds familiar, I guess I mistook the discussion to be about "port
> specific" rather than "API specific" patches. Still, I think there ought
to
> be some recourse when a patch submitter cannot find a willing reviewer
after
> some long period of time.
>
> Thanks,
>
> Kevin
>
>>
>> Regards,
>> Julien
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20090304/56a5b239/attachment.html>