Bug 110147 – [WinCairo] Support for cookies is incomplete

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Peter Nelson]

Support for cookies in WinCairo is incomplete. Problems include:

* Cookies set in HTTP response are not accessible from JavaScript
* Cookies set in JavaScript have an "unknown" domain
* Cookies set in JavaScript do not expire

[Peter Nelson]

Created attachment 188930 [details]
Patch

[Brent Fulgham]

Comment on attachment 188930 [details]
Patch

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

This looks great! I just have a few suggestions to make it a little easier to read.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:45
> +    // 7. value

How about making this an enum, so we can do something like 'attribute[kDomain]' or 'attribute[kExpirationTime]'?

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:59
> +    if (attributes.size())

Do you mean "!attributes.size()" here? It looks like you bail out any time there are attributes to process.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:64
> +    Vector<String>::iterator attribute = attributes.begin();

Won't this always be empty?

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:108
> +    cookieStr.reserveCapacity(domain.length() + path.length() + cookieName.length() + cookieValue.length() + 26);

What is the magic number for? Can we give it some sort of descriptive label?

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:174
> +        if (attributes.size() != 7)

Let's make this a label, rather than a magic number. 'kNetscapeCookieAttributeCount' or some such.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:190
> +        bool allowedPath = url.path().startsWith(attributes[2]);

Could all these attribute offsets be part of an enum? That would make this easier to follow.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:191
> +        bool secure = attributes[3] == "FALSE" || url.protocolIs("https");

attributes[kSecureConnection] == 'FALSE'

[Peter Nelson]

Some of these issues seem to have been partially resolved in Changeset 143272. I will upload another patch to deal with the rest shortly.

[Peter Nelson]

Created attachment 191588 [details]
Patch

[Brent Fulgham]

Comment on attachment 191588 [details]
Patch

Looks good.  Thank you for revising the earlier patch.

[WebKit Review Bot]

Comment on attachment 191588 [details]
Patch

Clearing flags on attachment: 191588

Committed r144853: <http://trac.webkit.org/changeset/144853>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.