Bug 27414 – Cairo-based Windows port does not handle cookies properly

• Component: Platform
• Status: RESOLVED FIXED

[Kwang Yul Seo]

Cairo-based windows port uses curl as its http backend. curl handles cookies by itself and it does not share cookies with WinINet. However, CookieJarWin.cpp uses WinINet functions, InternetGetCookie and InternetSetCookie to get and set cookies respectively. 

Using CookieJarCurl.cpp is also wrong because it does not interact with curl's cookie manager. It a simple hash map to get and set cookies. It is inevitable because there is no such API to get and set cookies in curl.

I think the right direction is to use CookieJarCurl.cpp and ask curl developers to provide a public API to manipulate cookies. Then we can implement cookie code correctly.

In the meantime, replacing CookieJarWin.cpp with CookieJarCurl.cpp has no side effect unless our intention is to share cookies with Internet Explorer or other WinINet-based clients.

[Kwang Yul Seo]

Created attachment 33026 [details]
Replace CookieJarWin.cpp with CookieJarCurl.cpp

[Brent Fulgham]

I am not a reviewer, but I think that this patch is correct.  I threw together the existing Windows Cookie logic in an effort to get things building and running, without much concern about correctness or proper utility.

I am in the process of trying to flesh out more of the cURL support, and think this is a good step in that direction.  I support this patch being approved and landed.

[Brent Fulgham]

I can confirm that this patch lands cleanly and runs properly on my WinCairo build.

[Kwang Yul Seo]

I think the maintainer of Windows Cairo port should review this. Who's in chare of?

[Eric Seidel (no email)]

Comment on attachment 33026 [details]
Replace CookieJarWin.cpp with CookieJarCurl.cpp

Assuming this *only* affects the Cairo build and does not break the regular Windows build, this is fine.  I can't tell from this patch if it affects the Apple Win build or not.

[Eric Seidel (no email)]

Comment on attachment 33026 [details]
Replace CookieJarWin.cpp with CookieJarCurl.cpp

Based on the above comments, I'll r+ this.  We can roll it out if it breaks the Apple windows build.

[Adam Barth]

Comment on attachment 33026 [details]
Replace CookieJarWin.cpp with CookieJarCurl.cpp

Clearing review flag on attachment: 33026

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.vcproj/WebCore.vcproj
Committed r46918
	M	WebCore/ChangeLog
	M	WebCore/WebCore.vcproj/WebCore.vcproj
r46918 = c4b233180d13184eb3bdafeae92b4f8bb02b022c (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46918

[Adam Barth]

All reviewed patches have been landed.  Closing bug.