From 92fc3cad900b9bd8dd7aa3ba43e883fbb480bc55 Mon Sep 17 00:00:00 2001
From: liquidsec <paul.mueller08@gmail.com>
Date: Thu, 24 Sep 2026 14:00:55 -0400
Subject: [PATCH] honor oversized cookie deletions and keep a replaced cookie's
 place

A Set-Cookie with Max-Age=0 was checked against the per-cookie size limit
before the deletion was handled, so a site that logs out by sending the
full session value back left the old cookie in the store. Deletions are
now handled first.

Replacing a cookie removed the old one and pushed the new one to the end,
which lost its creation-time order (RFC 6265 5.3 step 11.3) and could put
a stale same-name cookie first in the Cookie header. The replacement now
takes the old cookie's slot.

Fixes #91
Fixes #92
---
 src/cookies.rs | 65 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 9 deletions(-)

diff --git a/src/cookies.rs b/src/cookies.rs
index ad1a9ce..e673168 100644
--- a/src/cookies.rs
+++ b/src/cookies.rs
@@ -165,12 +165,6 @@ impl ChainCookies {
                 Rejected::note(&mut rejected.caller_owned, &cookie.name);
                 continue;
             }
-            // One oversized cookie is refused on its own, before it can eat
-            // the whole chain's budget.
-            if entry_size(&cookie.name, &cookie.value) > MAX_COOKIE_BYTES {
-                Rejected::note(&mut rejected.over_limit, &cookie.name);
-                continue;
-            }
             // §5.3 step 11: a new cookie replaces one with the same
             // name/domain/path rather than adding a duplicate. Find it now,
             // but leave it in place: whether the replacement is allowed to
@@ -180,12 +174,22 @@ impl ChainCookies {
             let replacing = self.cookies.iter().position(|c| {
                 c.name == cookie.name && c.domain == cookie.domain && c.path == cookie.path
             });
+            // A deletion stores nothing, so the size limits don't apply to
+            // it. It has to come before the size check: some sites log out
+            // by sending the full session value back with `Max-Age=0`, and
+            // refusing that would keep the old session cookie going out.
             if expired {
                 if let Some(i) = replacing {
                     self.cookies.remove(i);
                 }
                 continue;
             }
+            // One oversized cookie is refused on its own, before it can eat
+            // the whole chain's budget.
+            if entry_size(&cookie.name, &cookie.value) > MAX_COOKIE_BYTES {
+                Rejected::note(&mut rejected.over_limit, &cookie.name);
+                continue;
+            }
             let freed = replacing
                 .map(|i| entry_size(&self.cookies[i].name, &self.cookies[i].value))
                 .unwrap_or(0);
@@ -200,10 +204,13 @@ impl ChainCookies {
                 Rejected::note(&mut rejected.over_limit, &cookie.name);
                 continue;
             }
-            if let Some(i) = replacing {
-                self.cookies.remove(i);
+            // The replacement takes the old cookie's slot, which keeps its
+            // creation-time (§5.3 step 11.3). `header_for` relies on the
+            // order here being creation order for its tiebreak.
+            match replacing {
+                Some(i) => self.cookies[i] = cookie,
+                None => self.cookies.push(cookie),
             }
-            self.cookies.push(cookie);
         }
         rejected
     }
@@ -1063,6 +1070,46 @@ mod tests {
         assert!(chain.header_for(&uri("http://example.com/")).is_some());
     }
 
+    #[test]
+    fn oversized_deletion_is_still_honored() {
+        // Some sites log out by sending the whole session value back with
+        // `Max-Age=0`, so a deletion can be over the size limit.
+        let mut chain = ChainCookies::new();
+        set(&mut chain, "https://example.com/", &["s=realsessionvalue"]);
+        let big = format!("s={}; Max-Age=0", "x".repeat(5000));
+        set(&mut chain, "https://example.com/", &[&big]);
+        assert!(chain.header_for(&uri("https://example.com/")).is_none());
+    }
+
+    #[test]
+    fn replacing_a_cookie_keeps_its_place_in_line() {
+        let mut chain = ChainCookies::new();
+        set(&mut chain, "https://example.com/", &["a=1", "b=1"]);
+        set(&mut chain, "https://example.com/", &["a=2"]);
+        assert_eq!(
+            chain.header_for(&uri("https://example.com/")),
+            Some("a=2; b=1".to_string())
+        );
+    }
+
+    #[test]
+    fn replacing_one_of_two_same_name_cookies_keeps_their_order() {
+        // One name held host-only and domain-wide. The host-only one was set
+        // first, so it has to stay first after the site reissues it, or a
+        // server that reads the first occurrence gets the stale value.
+        let mut chain = ChainCookies::new();
+        set(
+            &mut chain,
+            "https://www.example.com/",
+            &["s=hostonly1", "s=domainwide; Domain=example.com"],
+        );
+        set(&mut chain, "https://www.example.com/", &["s=hostonly2"]);
+        assert_eq!(
+            chain.header_for(&uri("https://www.example.com/")),
+            Some("s=hostonly2; s=domainwide".to_string())
+        );
+    }
+
     #[test]
     fn storage_stops_at_the_cookie_count() {
         let mut chain = ChainCookies::new();