1724774 - Extend nsICookieManager with a method to query cookies by hostname across OriginAttributes

Status: RESOLVED WORKSFORME

(Core :: Networking: Cookies, task, P2)

Description

[Paul Zühlcke [:pbz]]

For Bug 1669716 the extension API needs to be able to query cookies by hostname, indepenently of OriginAttributes, specifically the partitionKey field.
It's already possible to filter cookies this way currently, however the performance is bad since consumers need to iterate over all cookies.

I suggest that we add a method that is passed a host an and OriginAttributesPattern. Getting cookies of example.com for all OA, would mean passing example.com, { }.

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Hi Paul, do you intend to work on this yourself?

Comment 2

[Paul Zühlcke [:pbz]]

Yes, the anti-tracking team can work on this as part of our dfpi high quality project. The target is 93.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Thanks!

Comment 4

[Paul Zühlcke [:pbz]]

I've just realized that this is already possible via getCookiesWithOriginAttributes. It supports passing a host and an OriginAttributesPattern:
https://searchfox.org/mozilla-central/rev/bb5549df90f9b0f5b453f9d8e872a94e503c64a6/netwerk/cookie/nsICookieManager.idl#222

Rob, does this method work for you, or do you have any other requirements? If you want to query all cookies for a host and any OA, simply pass "{}" for the pattern string.

Comment 5

[Rob Wu [:robwu]]

(In reply to Paul Zühlcke [:pbz] from comment #4)

I've just realized that this is already possible via getCookiesWithOriginAttributes. It supports passing a host and an OriginAttributesPattern:
https://searchfox.org/mozilla-central/rev/bb5549df90f9b0f5b453f9d8e872a94e503c64a6/netwerk/cookie/nsICookieManager.idl#222

Rob, does this method work for you, or do you have any other requirements? If you want to query all cookies for a host and any OA, simply pass "{}" for the pattern string.

In terms of output, the method can be used (and it is already used). It is however implemented by enumerating all entries in a hash table instead of a single lookup in a hash table (details below). I was hoping to have a method that is more efficient than enumerating all cookies.

The getCookiesWithOriginAttributes is already used when we're querying cookies for any host (ext-cookies.js line 248).
That's handled by CookieService::GetCookiesWithOriginAttributes method is implemented by enumerating all known hosts and filtering by eTLD+1 and origin attributes.

When a specific host is known, getCookiesFromHost is used (ext-cookies.js line 246).
This is handled more efficiently by CookieService::GetCookiesFromHost, which looks up relevant cookies in a hashmap, keyed by eTLD+1 + OriginAttributes.

A potential way to resolve this is to change the hashtable (nsTHashtable<CookieEntry> mHostTable) to hash table keyed by hostname only, and either have another hash table to look up cookies by origin attributes. Cookie retrieval and removal methods by host (GetCookiesFromHost, RemoveCookiesFromExactHost) can then be implemented by a single lookup + enumeration over its (small) collection of cookies. The lookup of individual cookies will have a worse performance though (currently: lookup by CookieEntry; afterwards: lookup by eTLD+1, followed by enumerating the origin attributes).

Feel free to close the bug if you believe that there is no feasible way to optimize the lookup/removal of cookies for a given host name.

Comment 6

[Paul Zühlcke [:pbz]]

Before learning about getCookiesWithOriginAttributes, I thought you meant getting all cookies via Services.cookies.cookies and filtering in JS. That can obviously be really slow, since we need to pass all cookies through the interface and JS code will be substantially slower doing the filtering for large amounts of cookies.
I can see how iterating over the collection of hosts in the backend isn't ideal either. However, are we seeing a real world performance impact here, which would justify refactoring the hash tables (potentially at the performance cost of other, more common operations)?

Comment 7

[Rob Wu [:robwu]]

(In reply to Paul Zühlcke [:pbz] from comment #6)

Before learning about getCookiesWithOriginAttributes, I thought you meant getting all cookies via Services.cookies.cookies and filtering in JS. That can obviously be really slow, since we need to pass all cookies through the interface and JS code will be substantially slower doing the filtering for large amounts of cookies.
I can see how iterating over the collection of hosts in the backend isn't ideal either. However, are we seeing a real world performance impact here, which would justify refactoring the hash tables (potentially at the performance cost of other, more common operations)?

All things considered, I think that it's not worth the effort in refactoring the hash tables at the expense of the more common cookie lookups.

In bug 1669716 I'll just use an empty partitionKey by default (and somehow come up with a sensible API to distinguish between "empty" (first-party) and "empty" (any partition)):

• cookies.get (one cookie): default to an empty partitionKey
• cookies.getAll (with explicit host name): default to empty partitionKey, unless partitionKey is specified.
• cookies.remove: default to empty partitionKey

If performance in the cookies extension API ever becomes a concern, then it would probably make more sense to move the filter-and-match logic from https://searchfox.org/mozilla-central/rev/538569c63ffb17d590c51df66f9fff790dd7431e/toolkit/components/extensions/parent/ext-cookies.js#243-330 to native code.