Bug 107692 - [Curl] There is no way for a WebKit client to set the Curl cookie jar path
Summary: [Curl] There is no way for a WebKit client to set the Curl cookie jar path
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-23 07:39 PST by peavo
Modified: 2013-01-31 12:45 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.62 KB, patch)
2013-01-23 07:46 PST, peavo
no flags Details | Formatted Diff | Diff
Patch (1.95 KB, patch)
2013-01-30 07:39 PST, peavo
no flags Details | Formatted Diff | Diff
Patch (1.95 KB, patch)
2013-01-31 07:25 PST, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 2013-01-23 07:39:59 PST
There exists a method setCookieJarFileName() in WebCore\platform\network\curl\ResourceHandleManager.cpp,
but noone uses it.
Comment 1 peavo 2013-01-23 07:46:23 PST
Created attachment 184233 [details]
Patch
Comment 2 peavo 2013-01-25 08:01:52 PST
Setting the Curl cookie jar path makes Curl enable cookies, which is important for many sites to work.
Comment 3 peavo 2013-01-30 07:39:57 PST
Created attachment 185502 [details]
Patch
Comment 4 peavo 2013-01-30 07:42:31 PST
Modified patch to set a default cookie jar path, this will enable the Curl cookie engine by default (and persistent cookies).
Comment 5 Brent Fulgham 2013-01-30 09:54:05 PST
Comment on attachment 185502 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=185502&action=review

Thanks for working on this.  I think it would be cleaner if you encapsulated this in a helper function (like I did with the "certificatePath").  Then we can provide port-specific logic for identifying where the cookie jar should live.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:122
> +    , m_cookieJarFileName("cookies.dat")

I would prefer you write a helper function like "certificatePath" that encapsulates the identification of this cook jar.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:137
> +        m_cookieJarFileName = fastStrDup(cookieJarPath);

This logic should go in the helper function.  Then I can add a CF-variant without cluttering the code.
Comment 6 peavo 2013-01-31 07:25:55 PST
Created attachment 185780 [details]
Patch
Comment 7 peavo 2013-01-31 07:27:21 PST
Modified patch according to review comments.
Comment 8 Brent Fulgham 2013-01-31 11:41:50 PST
Comment on attachment 185780 [details]
Patch

Looks great!
Comment 9 peavo 2013-01-31 12:32:44 PST
(In reply to comment #8)
> (From update of attachment 185780 [details])
> Looks great!

Thanks for the review ;)
Comment 10 WebKit Review Bot 2013-01-31 12:45:44 PST
Comment on attachment 185780 [details]
Patch

Clearing flags on attachment: 185780

Committed r141464: <http://trac.webkit.org/changeset/141464>
Comment 11 WebKit Review Bot 2013-01-31 12:45:48 PST
All reviewed patches have been landed.  Closing bug.