Bug 193458 - Resolve confusion between HTTPCookieAcceptPolicy::OnlyFromMainDocumentDomain and HTTPCookieAcceptPolicy::ExclusivelyFromMainDocumentDomain
Summary: Resolve confusion between HTTPCookieAcceptPolicy::OnlyFromMainDocumentDomain ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-15 13:03 PST by Michael Catanzaro
Modified: 2020-07-08 07:52 PDT (History)
11 users (show)

See Also:


Attachments
Patch (32.84 KB, patch)
2019-07-05 18:23 PDT, Michael Catanzaro
mcatanzaro: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2019-01-15 13:03:25 PST
WKCookieManager.h declares these WKHTTPCookieAcceptPolicy:

enum {
    kWKHTTPCookieAcceptPolicyAlways = 0,
    kWKHTTPCookieAcceptPolicyNever = 1,
    kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain = 2,
    kWKHTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain = 3
};

I asked Jon Wilander about the distinction between AcceptPolicyOnlyFromMainDocumentDomain and kWKHTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain. He says:

"""AcceptPolicyOnlyFromMainDocumentDomain is the default policy on Cocoa platforms and it means third-party resource loads (differing eTLD+1) cannot read or write cookies unless they have pre-existing cookies. This effectively means the eTLD+1 needs to become a top frame resource to seed its cookie jar before it can use it as third-party.

AcceptPolicyExclusivelyFromMainDocumentDomain means only first-party cookies."""

GTK and WPE expose an WebKitCookieAcceptPolicy enum with values WEBKIT_COOKIE_POLICY_ACCEPT_ALWAYS, WEBKIT_COOKIE_POLICY_ACCEPT_NEVER, and WEBKIT_COOKIE_POLICY_ACCEPT_NO_THIRD_PARTY. Internally, it is implemented by converting from WebKitCookieAcceptPolicy to WKHTTPCookieAcceptPolicy. The conversion for the always/never policies are obvious. WEBKIT_COOKIE_POLICY_ACCEPT_NO_THIRD_PARTY is converted to kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain. However, the behavior it implements actually matches kWKHTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain. So libsoup's kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain is actually equivalent to Cocoa's kWKHTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain, and libsoup ports have no equivalent to Cocoa's kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain. This sort of unexpected platform-specific differences from seemingly cross-platform code is why we wind up with strange platform-specific bugs, so we should fix it.

Now, each of our three WebKitCookieAcceptPolicys corresponds to a SoupCookieJarAcceptPolicy: SOUP_COOKIE_JAR_ACCEPT_ALWAYS, SOUP_COOKIE_JAR_ACCEPT_NEVER, and SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY. I actually wrote a libsoup patch a year ago to change the behavior of SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY to match Cocoa's kWKHTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain, but never landed it. We should resurrect that, except we should do it as a new SoupCookieJarAcceptPolicy instead of modifying the behavior of the existing policy. Then we should expose a new, corresponding WebKitCookieAcceptPolicy, and change Epiphany to use it. And lastly, we should change toWebKitCookieAcceptPolicy and toHTTPCookieAcceptPolicy in WebKit/UIProcess/API/glib/WebKitCookieManager.cpp so that WEBKIT_COOKIE_POLICY_ACCEPT_NO_THIRD_PARTY corresponds to HTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain instead of HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain, and have the new WebKitCookieAcceptPolicy correspond to HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain.

Finally, we should consider renaming HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain or HTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain if possible. These are private APIs on all ports, but Mac system applications sometimes use private APIs, so only Apple can determine if it's possible to do a rename.

CC: Basuke and Christopher, since you might want to investigate what curl ports are doing here.
Comment 1 Michael Catanzaro 2019-01-15 13:47:34 PST
(In reply to Michael Catanzaro from comment #0)
> I actually wrote a libsoup patch a year ago to change the behavior of
> SOUP_COOKIE_JAR_ACCEPT_NO_THIRD_PARTY to match Cocoa's
> kWKHTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain, but never landed
> it. We should resurrect that, except we should do it as a new
> SoupCookieJarAcceptPolicy instead of modifying the behavior of the existing
> policy.

Here is that old patch: https://bug792130.bugzilla-attachments.gnome.org/attachment.cgi?id=366620
Comment 2 Michael Catanzaro 2019-02-27 18:36:31 PST
Note I have libsoup switch from HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain to HTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain in bug #195140, so that will at least avoid the strange platform-specific difference.
Comment 3 Michael Catanzaro 2019-07-05 18:23:57 PDT
Created attachment 373558 [details]
Patch
Comment 4 Michael Catanzaro 2019-07-05 18:24:54 PDT
(In reply to Michael Catanzaro from comment #2)
> Note I have libsoup switch from
> HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain to
> HTTPCookieAcceptPolicyExclusivelyFromMainDocumentDomain in bug #195140, so
> that will at least avoid the strange platform-specific difference.

Resolved here instead.
Comment 5 EWS Watchlist 2019-07-05 18:26:41 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 6 Basuke Suzuki 2019-07-08 19:46:35 PDT
Comment on attachment 373558 [details]
Patch

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

> Source/WebCore/platform/network/curl/CookieJarDB.h:-42
> -

We agree to the basic concept of this bug, but we still need this definition for WK1. It's not good idea to refer WK2 definitions from Legacy, isn't it?
Comment 7 Michael Catanzaro 2019-07-09 07:50:38 PDT
Comment on attachment 373558 [details]
Patch

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

Oh, I see EWS is sad.

>> Source/WebCore/platform/network/curl/CookieJarDB.h:-42
>> -
> 
> We agree to the basic concept of this bug, but we still need this definition for WK1. It's not good idea to refer WK2 definitions from Legacy, isn't it?

You're right. I thought it was a WebCore type, but it's not. That's why the WinCairo build is failing.
Comment 8 Michael Catanzaro 2019-07-09 14:25:34 PDT
For macOS/iOS the problem is I missed that NSHTTPCookieAcceptPolicy and CFHTTPCookieStorageAcceptPolicy are different. CFHTTPCookieStorageAcceptPolicy is ours, and I changed it properly, but we can't change NSHTTPCookieAcceptPolicy because that is a system API:

https://developer.apple.com/documentation/foundation/nshttpcookieacceptpolicy?language=objc

Its values are NSHTTPCookieAcceptPolicyAlways, NSHTTPCookieAcceptPolicyNever, and NSHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain. This means that it's silly to rename HTTPCookieAcceptPolicy::OnlyFromMainDocumentDomain to HTTPCookieAcceptPolicy::OnlyFromMainDocumentDomainAndGrandfatheredDomains while keeping HTTPCookieAcceptPolicy::ExclusivelyFromMainDocumentDomain, because then the WebKit enum values will no longer match NSHTTPCookieAcceptPolicy. I will think about the best way to resolve this.
Comment 9 Michael Catanzaro 2020-07-08 07:52:21 PDT
Soup ports are brought into line with Apple ports in bug #213954, so now the only remaining problem is that the enum names are confusing. Since NSHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain is an Apple platform API, we should leave OnlyFromMainDocumentDomain unchanged. I think we can simply rename ExclusivelyFromMainDocumentDomain to, say, OnlyFromMainDocumentDomainWithoutGrandfathering. It's verbose, but way more clear.