Fix deleting a cookie that was created with secure=False (#2976)

* Add ability to delete a non-secure cookie.
Also re-use existing cookie properties where possible if deleting a cookie that is already knowin in the response headers.
Sanity-check and correct deletion cookie's secure_prefix and host_prefix based on the value of secure bool.

* Fail explicitly

* squash

* Cleanup some comments

---------

Co-authored-by: Adam Hopkins <[email protected]>

sanic/cookies/response.py

@@ -363,6 +363,7 @@ def delete_cookie(
         *,
         path: str = "/",
         domain: Optional[str] = None,
+        secure: bool = True,
         host_prefix: bool = False,
         secure_prefix: bool = False,
     ) -> None:
@@ -381,6 +382,8 @@ def delete_cookie(
         :type path: Optional[str], optional
         :param domain: Domain of the cookie, defaults to None
         :type domain: Optional[str], optional
+        :param secure: Whether to delete a secure cookie. Defaults to True.
+        :param secure: bool
         :param host_prefix: Whether to add __Host- as a prefix to the key.
             This requires that path="/", domain=None, and secure=True,
             defaults to False
@@ -389,32 +392,61 @@ def delete_cookie(
             This requires that secure=True, defaults to False
         :type secure_prefix: bool
         """
-        # remove it from header
+        if host_prefix and not (secure and path == "/" and domain is None):
+            raise ServerError(
+                "Cannot set host_prefix on a cookie without "
+                "path='/', domain=None, and secure=True"
+            )
+        if secure_prefix and not secure:
+            raise ServerError(
+                "Cannot set secure_prefix on a cookie without secure=True"
+            )
+
         cookies: List[Cookie] = self.headers.popall(self.HEADER_KEY, [])
+        existing_cookie = None
         for cookie in cookies:
             if (
                 cookie.key != Cookie.make_key(key, host_prefix, secure_prefix)
                 or cookie.path != path
                 or cookie.domain != domain
             ):
                 self.headers.add(self.HEADER_KEY, cookie)
-
+            elif existing_cookie is None:
+                existing_cookie = cookie
         # This should be removed in v24.3
         try:
             super().__delitem__(key)
         except KeyError:
             ...
 
-        self.add_cookie(
-            key=key,
-            value="",
-            path=path,
-            domain=domain,
-            max_age=0,
-            samesite=None,
-            host_prefix=host_prefix,
-            secure_prefix=secure_prefix,
-        )
+        if existing_cookie is not None:
+            # Use all the same values as the cookie to be deleted
+            # except value="" and max_age=0
+            self.add_cookie(
+                key=key,
+                value="",
+                path=existing_cookie.path,
+                domain=existing_cookie.domain,
+                secure=existing_cookie.secure,
+                max_age=0,
+                httponly=existing_cookie.httponly,
+                partitioned=existing_cookie.partitioned,
+                samesite=existing_cookie.samesite,
+                host_prefix=host_prefix,
+                secure_prefix=secure_prefix,
+            )
+        else:
+            self.add_cookie(
+                key=key,
+                value="",
+                path=path,
+                domain=domain,
+                secure=secure,
+                max_age=0,
+                samesite=None,
+                host_prefix=host_prefix,
+                secure_prefix=secure_prefix,
+            )
 
 
 # In v24.3, we should remove this as being a subclass of dict

tests/test_cookies.py

@@ -380,6 +380,67 @@ def test_cookie_jar_delete_cookie_encode():
     ]
 
 
+def test_cookie_jar_delete_nonsecure_cookie():
+    headers = Header()
+    jar = CookieJar(headers)
+    jar.delete_cookie("foo", domain="example.com", secure=False)
+
+    encoded = [cookie.encode("ascii") for cookie in jar.cookies]
+    assert encoded == [
+        b'foo=""; Path=/; Domain=example.com; Max-Age=0',
+    ]
+
+
+def test_cookie_jar_delete_existing_cookie():
+    headers = Header()
+    jar = CookieJar(headers)
+    jar.add_cookie(
+        "foo", "test", secure=True, domain="example.com", samesite="Strict"
+    )
+    jar.delete_cookie("foo", domain="example.com", secure=True)
+
+    encoded = [cookie.encode("ascii") for cookie in jar.cookies]
+    # deletion cookie contains samesite=Strict as was in original cookie
+    assert encoded == [
+        b'foo=""; Path=/; Domain=example.com; Max-Age=0; SameSite=Strict; Secure',
+    ]
+
+
+def test_cookie_jar_delete_existing_nonsecure_cookie():
+    headers = Header()
+    jar = CookieJar(headers)
+    jar.add_cookie(
+        "foo", "test", secure=False, domain="example.com", samesite="Strict"
+    )
+    jar.delete_cookie("foo", domain="example.com", secure=False)
+
+    encoded = [cookie.encode("ascii") for cookie in jar.cookies]
+    # deletion cookie contains samesite=Strict as was in original cookie
+    assert encoded == [
+        b'foo=""; Path=/; Domain=example.com; Max-Age=0; SameSite=Strict',
+    ]
+
+
+def test_cookie_jar_delete_existing_nonsecure_cookie_bad_prefix():
+    headers = Header()
+    jar = CookieJar(headers)
+    jar.add_cookie(
+        "foo", "test", secure=False, domain="example.com", samesite="Strict"
+    )
+    message = (
+        "Cannot set host_prefix on a cookie without "
+        "path='/', domain=None, and secure=True"
+    )
+    with pytest.raises(ServerError, match=message):
+        jar.delete_cookie(
+            "foo",
+            domain="example.com",
+            secure=False,
+            secure_prefix=True,
+            host_prefix=True,
+        )
+
+
 def test_cookie_jar_old_school_delete_encode():
     headers = Header()
     jar = CookieJar(headers)