[webkit-reviews] review granted: [Bug 88760] [soup] Prevent setting or editing httpOnly cookies from JavaScript : [Attachment 147124] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Christophe Dumez
<christophe.dumez at intel.com>'s request for review:
Bug 88760: [soup] Prevent setting or editing httpOnly cookies from JavaScript
https://bugs.webkit.org/show_bug.cgi?id=88760

Attachment 147124: Patch
https://bugs.webkit.org/attachment.cgi?id=147124&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=147124&action=review


Since I have a small suggestion I'm not setting cq+, thanks, great work!

> Source/WebCore/platform/network/soup/CookieJarSoup.cpp:75
> +static inline SoupCookie* findCookieInList(const GSList* cookies, const
gchar* name, const gchar* path)

I think you could make this helper contain the full logic, so instead of
finding the cookie in the list and then verifying it's http-only at the caller
you can instead have httpOnlyCookiExists(...).

> Source/WebCore/platform/network/soup/CookieJarSoup.cpp:113
> +	   // Make sure we do not overwrite httpOnly cookies from JavaScript.
> +	   SoupCookie* existingCookie = findCookieInList(existingCookies,
soup_cookie_get_name(cookie.get()), soup_cookie_get_path(cookie.get()));
> +	   if (existingCookie && soup_cookie_get_http_only(existingCookie))
> +	       continue;

Would make this more readable =).