1818968 - cookies.get() should return the cookie with the closest matching path

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[Rob Wu [:robwu]]

ext-cookies.js has a comment in the get implementation with "// FIXME: We don't sort by length of path and creation time." (added in bug 1197417). This expectation follows from the usual behavior of cookie access on the web and is also part of RFC 6265 (cited below).

Note: while this report is about cookies.get, the underlying cookie-lookup implementation and issue is shared by all cookie methods:

• cookies.get - test case below.
• cookies.getAll - test case below.
• cookies.set - internally uses cookies.get to return the just-created cookie, so any bug in cookies.get will be reflected here.
• cookies.remove - bug 1387957 shows that the unexpected cookie is removed when there are multiple cookies.

Test case:

1. Visit https://example.com and create cookies with multiple paths:

document.cookie = "a=1; path=/";
document.cookie = "a=2; path=/sub";
document.cookie = "a=3; path=/sub/dir";
document.cookie = "a=4; path=/sub/dir/deeper";

2. From an extension with the "cookies" permission and host_permissions for example.com, run the following and look at the result:

chrome.cookies.get({ url: "https://example.com/sub/file", name: "a" }, cookie => {
  console.log(cookie.value, cookie.path);
});

3. For comparison, run: chrome.cookies.getAll({ name: "a" }, console.log);

Expected (observed in Chrome):

• At step 2: 1 /sub
• At step 3: cookies sorted by path length (longest first), i.e. 4 3 2 1

Actual (Firefox):

• At step 2: 1 /
• At step 3: cookies in order of creation: 1 2 3 4

The expected order is observed in web pages (e.g. when document.cookie is used at https://example.com/sub/dir/deeper/end), and is specified by RFC 6265, section 5.4:

2. The user agent SHOULD sort the cookie-list in the following
   order:

   • Cookies with longer paths are listed before cookies with
     shorter paths.

   • Among cookies that have equal-length path fields, cookies with
     earlier creation-times are listed before cookies with later
     creation-times.

   NOTE: Not all user agents sort the cookie-list in this order, but
   this order reflects common practice when this document was
   written, and, historically, there have been servers that
   (erroneously) depended on this order.

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1818968 - Change "FIXME" to "TODO bug 1818968"

Comment 2

[Rob Wu [:robwu]]

The above patch does not fix the issue, but links to this bug so that the relevant bug is linked from the source code.

Comment 3

[Rob Wu [:robwu]]

The work-around, however terrible it is, is to call cookies.getAll() to get all cookies and then find the right cookie among them.

For the cookies.remove method, the work-around is to get all cookies and restore them again if they had accidentally been removed. This is an even worse work-around, because the cookie still has internal fields that would change (e.g. those mentioned in bug 1480046).

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f152b9989882
Change "FIXME" to "TODO bug 1818968" r=zombie DONTBUILD

Comment 5

[Serban Stanca [:SerbanS]]

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

Comment 6

[CoolCmd]

Someone should add to MDN that get() and getAll() returns expired (bug 1388873) and unsorted (bug 1818968) cookies.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:robwu, maybe it's time to close this bug?
For more information, please visit BugBot documentation.

Comment 8

[Rob Wu [:robwu]]

The landed patch was merely a TODO fixup. This bug should still stay open.

Comment 9

[Andrea Marchesini [:baku]]

Attachment: Bug 1818968 - WebExtension - Sort cookies as defined by RFC6265, r=robwu,edgul

Comment 10

[Rob Wu [:robwu]]

dev-doc-needed: should document that the cookies API now orders cookies according to RFC 6265. This is mainly relevant when there are multiple cookies that are the same except for the path component. Previously, the first created cookie was matched by cookies.get, cookies.remove, cookies.set and cookies.getAll, now the cookie with the longest matching path is returned instead.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/061dc31cb68d
WebExtension - Sort cookies as defined by RFC6265, r=robwu,edgul,cookie-reviewers

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/061dc31cb68d

Comment 13

[Richard Bloor]

Changes ready for review:

• content, PR cookies.get orders cookies according to RFC-6265 #36193
• BCD, PR cookies.get orders cookies according to RFC-6265 #24634