[Webkit-unassigned] [Bug 166967] [SOUP] Simplify cookie storage handling

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=166967

--- Comment #9 from Michael Catanzaro <mcatanzaro at igalia.com> ---
(In reply to comment #7)
> Note that the default NetworkStorageSession is always created without a
> SoupNetworkSession, which is specially interesting in the web process
> because most of the times we don't even need a soup session at all there. If
> you ask for the cookie storage of the default NetworkStorageSession we don't
> need to have a default SoupNetworkSession at all, the same way it happens
> with current code when you call soupCookieJar(). And the only difference in
> behavior is that now the default SoupNetworkSession is created on demand
> when asked to the default NetworkStorageSession and the default cookie
> storage is set in that case, but only stored in NetworkStorageSession (of
> course the feature is set in the soup session).

OK, that makes sense to me now. The problem I see here is that nothing prevents you from accidentally creating a SoupNetworkSession in the web process or UI process. Indeed, it seems easier than before because just calling NetworkStorageSession::soupNetworkSession has the effect of creating a new SoupNetworkSession. Is there some time when a SoupSession really is needed in the web process? If so, we should add a FIXME to change that, because it prevents future sandboxing. If not, can you add an assertion to ensure NetworkStorageSession::soupNetworkSession is only ever called from the network process?

> It's true that for every
> session, if the cookie storage is asked before the soup session is created,
> when the soup session is created we have two references of the cookie jar,
> we could probably clear the NetworkStorageSession one in that case. That can
> only happen for the default session, all others are created with a
> SoupNetworkSession.

OK, I think you should indeed clear the NetworkStorageSession jar and have it return the SoupNetworkSession jar instead. Even if it's the same thing, that would make the code clearer and more robust.

> We have 3 different create methods for SoupNetworkSession where the only
> difference is the cookie jar passed to the constructor, using
> createPrivateBrowsingCookieJar() which is confusing because it's exactly the
> same as the shared one, the cookie jar itself has nothing of private. So,
> this is simplified in this patch by removing all create() methods, and
> simply allowing a nullptr jar in the constructor, meaning don't use the
> given one (the shared one normally), but create your own.

I think you should still add some function that returns SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY and use it in the different places. That was the default setting can be changed at just one point if we want to change it in the future.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170113/c8a5c553/attachment.html>