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 1 commit
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
59 changes: 48 additions & 11 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,66 @@ def delete_cookie(
This requires that secure=True, defaults to False
:type secure_prefix: bool
"""

# Host prefix can only be used on a cookie where
# path=="/", domain==None, and secure==True
host_prefix = (
False
if not (secure and path == "/" and domain is None)
else host_prefix
)
# Secure prefix can only be used on a secure cookie
secure_prefix = False if not secure else secure_prefix
ahopkins marked this conversation as resolved.
Show resolved Hide resolved

# remove it from header
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)

else:
# Keep a snapshot of the cookie-to-be-deleted
# for reference when creating the deletion cookie
if 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
63 changes: 63 additions & 0 deletions tests/test_cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,69 @@ 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"
)
jar.delete_cookie(
"foo",
domain="example.com",
secure=False,
secure_prefix=True,
host_prefix=True,
) # noqa

encoded = [cookie.encode("ascii") for cookie in jar.cookies]
# Deletion cookie has secure_prefix and host_prefix, but original cookie
# is not secure so ignore those invalid options on delete cookie.
assert encoded == [
b'foo=""; Path=/; Domain=example.com; Max-Age=0; SameSite=Strict',
]


def test_cookie_jar_old_school_delete_encode():
headers = Header()
jar = CookieJar(headers)
Expand Down
Loading