1224579 - Setting cookie with leading dot in domain doesn't work

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[Jan Skrasek]

Setting cookie with domain which has the leading dot doesn't work.

var domain = ".example.com";
chrome.cookies.set({"url": url, "domain": domain, "path": "/", "name": "api_debug", "value": "1"});

It will be probably caused by readding antoher in https://dxr.mozilla.org/mozilla-central/rev/0c648a1efbe06b5ec866ba058d18256b80808b46/toolkit/components/extensions/ext-cookies.js#180 (sorry, the link currently does not work, e500, the general https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/ext-cookies.js#180)

Comment 1

[Tom Schuster [:evilpie]]

When I read the documentation I somehow assumed that all domains should be the opposite host-only, but it only says when "domain" is committed that cookie should become host only. "The domain of the cookie. If omitted, the cookie becomes a host-only cookie."

So I have to test Chrome again if it always adds a "." before domains. The two leading dots are definitely wrong.

Comment 3

[Tom Schuster [:evilpie]]

Attachment: Allow leading dot in domain in cookies.set

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Does the patch in bug 1210996 fix this?

Comment 5

[Kris Maglione [:kmag]]

(In reply to Bill McCloskey (:billm) from comment #4)
> Does the patch in bug 1210996 fix this?

I think it does, but it would be good to have the additional test either way.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8700613 [details] [diff] [review]
Allow leading dot in domain in cookies.set

Review of attachment 8700613 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, that's true. Tom, once Kris lands his patch, can you check that the test passes without your ext-cookies.js change and then just land the test?

Comment 7

[Tom Schuster [:evilpie]]

The error went away, but the result is wrong. It seems to create a hostOnly cookie now. If 'domain' is specified the result is never a hostOnly cookie. As far as I can tell you can only get such a cookie when omitting it.

Comment 8

[Kris Maglione [:kmag]]

Attachment: [webext] Fix the handling of domain cookies

You're right, I got that bit of logic wrong.

Comment 9

[Tom Schuster [:evilpie]]

Comment on attachment 8701501 [details] [diff] [review]
[webext] Fix the handling of domain cookies

Review of attachment 8701501 [details] [diff] [review]:
-----------------------------------------------------------------

As requested, we should also land my test. The new code is really nice btw :)

::: toolkit/components/extensions/ext-cookies.js
@@ +70,5 @@
>    if (!extension.whiteListedHosts.matchesIgnoringPath(uri)) {
>      return false;
>    }
>  
> +  if (!cookie.host) {

Comment that this will be a host only cookie.

Comment 10

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/fx-team/rev/8338cdb566e28bd2dcaf7ef726cdd49529ae431d
Bug 1224579: [webext] Fix the handling of domain cookies. r=evilpie

Comment 11

[Kris Maglione [:kmag]]

I fixed the conflicts, and checked that your tests pass after that patch. Let me know if you want me to check it in.

Comment 12

[Tom Schuster [:evilpie]]

Sure, whenever you want.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8338cdb566e2

Comment 14

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/fx-team/rev/e25c5bf2da7411a58e2a6e5cb37bf67be23052ff
Bug 1224579 - Allow leading dot in domain in cookies.set. r=billm

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e25c5bf2da74