27414 2009-07-18 07:00:14 -0700 Cairo-based Windows port does not handle cookies properly 2009-08-07 15:25:16 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC Windows XP RESOLVED FIXED P2 Normal --- 1 skyul bfulgham abarth bfulgham xhiloh oldest_to_newest 132907 0 skyul 2009-07-18 07:00:14 -0700 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. 132913 1 33026 skyul 2009-07-18 08:20:52 -0700 Created attachment 33026 Replace CookieJarWin.cpp with CookieJarCurl.cpp 137765 2 bfulgham 2009-08-06 20:40:25 -0700 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. 137805 3 bfulgham 2009-08-06 22:54:58 -0700 I can confirm that this patch lands cleanly and runs properly on my WinCairo build. 137847 4 skyul 2009-08-07 04:29:52 -0700 I think the maintainer of Windows Cairo port should review this. Who's in chare of? 138052 5 33026 eric 2009-08-07 12:46:24 -0700 Comment on attachment 33026 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. 138053 6 33026 eric 2009-08-07 12:47:17 -0700 Comment on attachment 33026 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. 138187 7 33026 abarth 2009-08-07 15:25:13 -0700 Comment on attachment 33026 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 138188 8 abarth 2009-08-07 15:25:16 -0700 All reviewed patches have been landed. Closing bug. 33026 2009-07-18 08:20:52 -0700 2009-08-07 15:25:13 -0700 Replace CookieJarWin.cpp with CookieJarCurl.cpp cookie.diff text/plain 3652 skyul SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NjA5MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTkgQEAKKzIwMDktMDctMTggIEt3YW5nIFl1bCBTZW8gIDxza3l1bEBjb21wYW55 MTAwLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBDYWlyby1iYXNlZCBXaW5kb3dzIHBvcnQgZG9lcyBub3QgaGFuZGxlIGNvb2tpZXMgcHJvcGVy bHkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI3NDE0 CisKKyAgICAgICAgY3VybCBoYW5kbGVzIGNvb2tpZXMgYnkgaXRzZWxmLCBzbyB1c2luZyBXaW5J TmV0IGZ1bmN0aW9ucworICAgICAgICB0byBnZXQgYW5kIHNldCBjb29raWVzIGlzIHdyb25nLgor CisgICAgICAgIFJlcGxhY2UgQ29va2llSmFyV2luLmNwcCB3aXRoIENvb2tpZUphckN1cmwuY3Bw IHNvIHRoYXQKKyAgICAgICAgY29va2llcyBjYW4gYmUgaW1wbGVtZW50ZWQgbGF0ZXIgb25jZSBj dXJsIHByb3ZpZGVzIGFuIEFQSSB0bworICAgICAgICBnZXQgYW5kIHNldCBjb29raWVzLgorCisg ICAgICAgICogV2ViQ29yZS52Y3Byb2ovV2ViQ29yZS52Y3Byb2o6CisKIDIwMDktMDctMTggIEpl cmVteSBPcmxvdyAgPGpvcmxvd0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUnViYmVyIHN0YW1w ZWQgYnkgQWRhbSBCYXJ0aC4KSW5kZXg6IFdlYkNvcmUvV2ViQ29yZS52Y3Byb2ovV2ViQ29yZS52 Y3Byb2oKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9XZWJDb3JlLnZjcHJvai9XZWJDb3JlLnZjcHJv agkocmV2aXNpb24gNDYwOTApCisrKyBXZWJDb3JlL1dlYkNvcmUudmNwcm9qL1dlYkNvcmUudmNw cm9qCSh3b3JraW5nIGNvcHkpCkBAIC0xOTkxMSwxOCArMTk5MTEsMTAgQEAKIAkJCQkJCTwvRmls ZUNvbmZpZ3VyYXRpb24+DQogCQkJCQk8L0ZpbGU+DQogCQkJCQk8RmlsZQ0KLQkJCQkJCVJlbGF0 aXZlUGF0aD0iLi5ccGxhdGZvcm1cbmV0d29ya1x3aW5cQ29va2llSmFyV2luLmNwcCINCisJCQkJ CQlSZWxhdGl2ZVBhdGg9Ii4uXHBsYXRmb3JtXG5ldHdvcmtcd2luXENvb2tpZVN0b3JhZ2VXaW4u Y3BwIg0KIAkJCQkJCT4NCiAJCQkJCQk8RmlsZUNvbmZpZ3VyYXRpb24NCi0JCQkJCQkJTmFtZT0i RGVidWd8V2luMzIiDQotCQkJCQkJCUV4Y2x1ZGVkRnJvbUJ1aWxkPSJ0cnVlIg0KLQkJCQkJCQk+ DQotCQkJCQkJCTxUb29sDQotCQkJCQkJCQlOYW1lPSJWQ0NMQ29tcGlsZXJUb29sIg0KLQkJCQkJ CQkvPg0KLQkJCQkJCTwvRmlsZUNvbmZpZ3VyYXRpb24+DQotCQkJCQkJPEZpbGVDb25maWd1cmF0 aW9uDQotCQkJCQkJCU5hbWU9IlJlbGVhc2V8V2luMzIiDQorCQkJCQkJCU5hbWU9IkRlYnVnX0Nh aXJvfFdpbjMyIg0KIAkJCQkJCQlFeGNsdWRlZEZyb21CdWlsZD0idHJ1ZSINCiAJCQkJCQkJPg0K IAkJCQkJCQk8VG9vbA0KQEAgLTE5OTMwLDcgKzE5OTIyLDcgQEAKIAkJCQkJCQkvPg0KIAkJCQkJ CTwvRmlsZUNvbmZpZ3VyYXRpb24+DQogCQkJCQkJPEZpbGVDb25maWd1cmF0aW9uDQotCQkJCQkJ CU5hbWU9IkRlYnVnX0ludGVybmFsfFdpbjMyIg0KKwkJCQkJCQlOYW1lPSJSZWxlYXNlX0NhaXJv fFdpbjMyIg0KIAkJCQkJCQlFeGNsdWRlZEZyb21CdWlsZD0idHJ1ZSINCiAJCQkJCQkJPg0KIAkJ CQkJCQk8VG9vbA0KQEAgLTE5OTM5LDcgKzE5OTMxLDcgQEAKIAkJCQkJCTwvRmlsZUNvbmZpZ3Vy YXRpb24+DQogCQkJCQk8L0ZpbGU+DQogCQkJCQk8RmlsZQ0KLQkJCQkJCVJlbGF0aXZlUGF0aD0i Li5ccGxhdGZvcm1cbmV0d29ya1x3aW5cQ29va2llU3RvcmFnZVdpbi5jcHAiDQorCQkJCQkJUmVs YXRpdmVQYXRoPSIuLlxwbGF0Zm9ybVxuZXR3b3JrXHdpblxDb29raWVTdG9yYWdlV2luLmgiDQog CQkJCQkJPg0KIAkJCQkJCTxGaWxlQ29uZmlndXJhdGlvbg0KIAkJCQkJCQlOYW1lPSJEZWJ1Z19D YWlyb3xXaW4zMiINCkBAIC0xOTk1OSwzNSArMTk5NTEsNDMgQEAKIAkJCQkJCTwvRmlsZUNvbmZp Z3VyYXRpb24+DQogCQkJCQk8L0ZpbGU+DQogCQkJCQk8RmlsZQ0KLQkJCQkJCVJlbGF0aXZlUGF0 aD0iLi5ccGxhdGZvcm1cbmV0d29ya1x3aW5cQ29va2llU3RvcmFnZVdpbi5oIg0KKwkJCQkJCVJl bGF0aXZlUGF0aD0iLi5ccGxhdGZvcm1cbmV0d29ya1x3aW5cTmV0d29ya1N0YXRlTm90aWZpZXJX aW4uY3BwIg0KKwkJCQkJCT4NCisJCQkJCTwvRmlsZT4NCisJCQkJPC9GaWx0ZXI+DQorCQkJCTxG aWx0ZXINCisJCQkJCU5hbWU9ImN1cmwiDQorCQkJCQk+DQorCQkJCQk8RmlsZQ0KKwkJCQkJCVJl bGF0aXZlUGF0aD0iLi5ccGxhdGZvcm1cbmV0d29ya1xjdXJsXEF1dGhlbnRpY2F0aW9uQ2hhbGxl bmdlLmgiDQogCQkJCQkJPg0KIAkJCQkJCTxGaWxlQ29uZmlndXJhdGlvbg0KLQkJCQkJCQlOYW1l PSJEZWJ1Z19DYWlyb3xXaW4zMiINCisJCQkJCQkJTmFtZT0iRGVidWd8V2luMzIiDQogCQkJCQkJ CUV4Y2x1ZGVkRnJvbUJ1aWxkPSJ0cnVlIg0KIAkJCQkJCQk+DQogCQkJCQkJCTxUb29sDQotCQkJ CQkJCQlOYW1lPSJWQ0NMQ29tcGlsZXJUb29sIg0KKwkJCQkJCQkJTmFtZT0iVkNDdXN0b21CdWls ZFRvb2wiDQogCQkJCQkJCS8+DQogCQkJCQkJPC9GaWxlQ29uZmlndXJhdGlvbj4NCiAJCQkJCQk8 RmlsZUNvbmZpZ3VyYXRpb24NCi0JCQkJCQkJTmFtZT0iUmVsZWFzZV9DYWlyb3xXaW4zMiINCisJ CQkJCQkJTmFtZT0iUmVsZWFzZXxXaW4zMiINCiAJCQkJCQkJRXhjbHVkZWRGcm9tQnVpbGQ9InRy dWUiDQogCQkJCQkJCT4NCiAJCQkJCQkJPFRvb2wNCi0JCQkJCQkJCU5hbWU9IlZDQ0xDb21waWxl clRvb2wiDQorCQkJCQkJCQlOYW1lPSJWQ0N1c3RvbUJ1aWxkVG9vbCINCisJCQkJCQkJLz4NCisJ CQkJCQk8L0ZpbGVDb25maWd1cmF0aW9uPg0KKwkJCQkJCTxGaWxlQ29uZmlndXJhdGlvbg0KKwkJ CQkJCQlOYW1lPSJEZWJ1Z19JbnRlcm5hbHxXaW4zMiINCisJCQkJCQkJRXhjbHVkZWRGcm9tQnVp bGQ9InRydWUiDQorCQkJCQkJCT4NCisJCQkJCQkJPFRvb2wNCisJCQkJCQkJCU5hbWU9IlZDQ3Vz dG9tQnVpbGRUb29sIg0KIAkJCQkJCQkvPg0KIAkJCQkJCTwvRmlsZUNvbmZpZ3VyYXRpb24+DQog CQkJCQk8L0ZpbGU+DQogCQkJCQk8RmlsZQ0KLQkJCQkJCVJlbGF0aXZlUGF0aD0iLi5ccGxhdGZv cm1cbmV0d29ya1x3aW5cTmV0d29ya1N0YXRlTm90aWZpZXJXaW4uY3BwIg0KLQkJCQkJCT4NCi0J CQkJCTwvRmlsZT4NCi0JCQkJPC9GaWx0ZXI+DQotCQkJCTxGaWx0ZXINCi0JCQkJCU5hbWU9ImN1 cmwiDQotCQkJCQk+DQotCQkJCQk8RmlsZQ0KLQkJCQkJCVJlbGF0aXZlUGF0aD0iLi5ccGxhdGZv cm1cbmV0d29ya1xjdXJsXEF1dGhlbnRpY2F0aW9uQ2hhbGxlbmdlLmgiDQorCQkJCQkJUmVsYXRp dmVQYXRoPSIuLlxwbGF0Zm9ybVxuZXR3b3JrXGN1cmxcQ29va2llSmFyQ3VybC5jcHAiDQogCQkJ CQkJPg0KIAkJCQkJCTxGaWxlQ29uZmlndXJhdGlvbg0KIAkJCQkJCQlOYW1lPSJEZWJ1Z3xXaW4z MiINCg==