Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 44 additions & 12 deletions sanic/cookies/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
Expand Down
61 changes: 61 additions & 0 deletions tests/test_cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading