After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 677923 - Add soup_cookie_jar_set_cookie_with_first_party() alternative that takes a SoupCookie in argument
Add soup_cookie_jar_set_cookie_with_first_party() alternative that takes a So...
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: API
2.39.x
Other Linux
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2012-06-12 08:01 UTC by Christophe Dumez
Modified: 2012-06-12 17:02 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch (4.36 KB, patch)
2012-06-12 08:04 UTC, Christophe Dumez 		reviewed Details | Review
Patch (4.88 KB, patch)
2012-06-12 13:26 UTC, Christophe Dumez 		needs-work Details | Review
Patch (4.95 KB, patch)
2012-06-12 15:35 UTC, Christophe Dumez 		none Details | Review

Description Christophe Dumez 2012-06-12 08:01:44 UTC 
Currently, the only way to add a cookie while checking first_party is to use soup_cookie_jar_set_cookie_with_first_party(). However, this function takes an unparsed cookie in argument.

In the event the client has already parsed the cookie (e.g. to do some pre-processing), it would be useful to have an alternative to soup_cookie_jar_set_cookie_with_first_party() which takes a parsed SoupCookie in argument. This would be more efficient.

The alternative for the client would be to duplicate the policy check code and then call soup_cookie_jar_add_cookie(). However, duplicating the policy check code is not nice or maintainable.

This new API function would be useful to WebKit [1].

[1] https://bugs.webkit.org/show_bug.cgi?id=88760
Comment 1 Christophe Dumez 2012-06-12 08:04:15 UTC 
Created attachment 216172 [details] [review]
Patch
Comment 2 Dan Winship 2012-06-12 12:42:54 UTC 
Comment on attachment 216172 [details] [review]
Patch

as with the other patch, the new function needs to be added to the docs too

>-	if (!uri->host)
>-		return;

you need to keep this check in soup_cookie_jar_add_cookie_with_first_party(), in particular so that webkit won't crash when adding a cookie from a file: URI.
Comment 3 Christophe Dumez 2012-06-12 13:26:06 UTC 
Created attachment 216203 [details] [review]
Patch

Take the feedback into consideration, sorry I missed that.
Comment 4 Dan Winship 2012-06-12 14:45:23 UTC 
Comment on attachment 216203 [details] [review]
Patch

no, you want the check in _add_cookie_, not _set_cookie_, so it gets checked in both cases.

ok to commit with that change
Comment 5 Christophe Dumez 2012-06-12 15:35:26 UTC 
Created attachment 216214 [details] [review]
Patch

I added the cookie->domain check in soup_cookie_jar_add_cookie_with_first_party(), hopefully this is what you meant.

Note however that I kept the uri->host check in soup_cookie_jar_set_cookie_with_first_party(). Unless I'm mistaken, we still want it there because of the soup_cookie_parse() call. soup_cookie_parse() has a g_return_val_if_fail() check, not of an "if" check.
Comment 6 Dan Winship 2012-06-12 16:52:43 UTC 
oh, whoops, my bad. The previous patch (attachment 216203 [details] [review] / git hash 668c6ceb) was correct. Please commit that.