Cookie order in header does not follow RFC6265

[jjatria]

RFC6265 § 5.4 states:

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.

However, Cro::HTTP::Client::CookieJar does not do this:

#!/usr/bin/env raku

use Cro::HTTP::Client;

my @cookies = ( 
    'a=1; Domain=example.com; Path=/bar',
    'c=1; Domain=example.com; Path=/',
    'b=2; Domain=sub.example.com; Path=/bar',
    'd=2; Domain=sub.example.com; Path=/',
);

my $jar = Cro::HTTP::Client::CookieJar.new;
my $uri = Cro::Uri.parse: 'http://sub.example.com/bar';

my $res = Cro::HTTP::Response.new: :200status;
$res.append-header: "Set-Cookie: $_" for @cookies;
$jar.add-from-response: $res, $uri;

my $req = Cro::HTTP::Request.new;
$jar.add-to-request: $req, $uri;

say "{ .name }: { .value }" for $req.headers;
# OUTPUT:
# Cookie: d=2; b=2; c=1; a=1

Note that the order seems to be reverse chronological, rather than the one stated by the spec.

Note also that the cookie jar code does appear to be trying to sort according to the spec:

@cookie-list.sort({ .cookie.path.comb.elems, .creation-time });
@cookie-list.map({ $req.add-cookie($_.cookie) });

but this has a couple of problems.

1. It sorts by path length ascendingly (so longer paths appear later, which is against the spec).

2. The sort order gets lost anyway, since the sorted list is never used or assigned to anything (so the cookies remain in the order they were added).

3. This order is later reversed because $req.add-cookie does

   my @cookies = self!unpack-cookie;
   my @all-other = @cookies.grep({ not $_.name eq $c.name });
   self!pack-cookie($c, |@all-other);

   which (for each cookie that gets added) (!)

   1. Parses the request's Cookie header into Cro::HTTP::Cookie objects (with !unpack-cookie)
   2. Makes a list with the new cookie first and then all the other ones
   3. Serialises this list into a new Cookie header string (with !pack-cookie)

A change like the following addresses these issues:

diff --git a/lib/Cro/HTTP/Client/CookieJar.pm6 b/lib/Cro/HTTP/Client/CookieJar.pm6
index e3e9450..68c37e6 100644
--- a/lib/Cro/HTTP/Client/CookieJar.pm6
+++ b/lib/Cro/HTTP/Client/CookieJar.pm6
@@ -135,8 +135,10 @@ monitor Cro::HTTP::Client::CookieJar {
             };
         });
         # Sorting
-        @cookie-list.sort({ .cookie.path.comb.elems, .creation-time });
-        @cookie-list.map({ $req.add-cookie($_.cookie) });
+        @cookie-list.sort({
+               $^b.cookie.path.comb.elems <=> $^a.cookie.path.comb.elems
+            || $^a.creation-time          <=> $^b.creation-time
+        }).map: { $req.add-cookie: .cookie }
     };
 
     #| Get the contents of the cookie jar
diff --git a/lib/Cro/HTTP/Request.pm6 b/lib/Cro/HTTP/Request.pm6
index eb5e721..e8f859e 100644
--- a/lib/Cro/HTTP/Request.pm6
+++ b/lib/Cro/HTTP/Request.pm6
@@ -206,7 +206,7 @@ class Cro::HTTP::Request does Cro::HTTP::Message {
     multi method add-cookie(Cro::HTTP::Cookie $c --> Bool) {
         my @cookies = self!unpack-cookie;
         my @all-other = @cookies.grep({ not $_.name eq $c.name });
-        self!pack-cookie($c, |@all-other);
+        self!pack-cookie(|@all-other, $c);
         @cookies.elems !== @all-other.elems
     }

That said, RFC6265 § 4.2.2 also says:

Although cookies are serialized linearly in the Cookie header, servers SHOULD NOT rely upon the serialization order. In particular, if the Cookie header contains two cookies with the same name (e.g., that were set with different Path or Domain attributes), servers SHOULD NOT rely upon the order in which these cookies appear in the header.

so this might be a little academic.