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.

Sign up for GitHub

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

Jump to bottom

Bug: cookie string parsing incorrectly handles attributes without values #3023

Open
4 tasks
floxay opened this issue Jan 24, 2024 · 4 comments · May be fixed by #3034
Open
4 tasks

Bug: cookie string parsing incorrectly handles attributes without values #3023

floxay opened this issue Jan 24, 2024 · 4 comments · May be fixed by #3034
Labels
Bug 🐛 This is something that is not working as expected

Comments

@floxay
Copy link
Contributor

floxay commented Jan 24, 2024 •
edited by polar-sh bot

Description

The parse_cookie_string() function incorrectly handles attributes without values and will keep only one due to the dictionary conversion.
For example if the cookie contains the HttpOnly and the Secure attributes these will both have an empty string as key and eventually only one of these will be returned.
image

URL to code causing the issue

litestar/litestar/_parsers.py

Lines 50 to 67 in 3dfd6fe

def parse_cookie_string(cookie_string: str) -> dict[str, str]:
"""Parse a cookie string into a dictionary of values.
Args:
cookie_string: A cookie string.
Returns:
A string keyed dictionary of values
"""
cookies = [cookie.split("=", 1) if "=" in cookie else ("", cookie) for cookie in cookie_string.split(";")]
output: dict[str, str] = {
k: unquote(unquote_cookie(v))
for k, v in filter(
lambda x: x[0] or x[1],
((k.strip(), v.strip()) for k, v in cookies),
)
}
return output

MCVE

No response

Steps to reproduce

No response

Screenshots

No response

Logs

No response

Litestar Version

v2.5.1

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@floxay floxay added the Bug 🐛 This is something that is not working as expected label Jan 24, 2024
@floxay
Copy link
Contributor Author

floxay commented Jan 25, 2024 •
edited

I imagine simply swapping ("", cookie) to (cookie, "") in

litestar/litestar/_parsers.py

Line 59 in 3dfd6fe

cookies = [cookie.split("=", 1) if "=" in cookie else ("", cookie) for cookie in cookie_string.split(";")]
would most likely break some user code accessing connection.cookies so I assume that's not a valid way to fix it, although that format would match the way these attributes work more.
What about (cookie, cookie)? Can't think of anything else that'd be simple and keeps the signature.

@peterschutt
Copy link
Contributor

What about (cookie, cookie)? Can't think of anything else that'd be simple and keeps the signature.

For the 'else' clause right? Sounds right to me.

@floxay floxay linked a pull request Jan 27, 2024 that will close this issue
Open
@JeanArhancet JeanArhancet mentioned this issue Jan 27, 2024
Merged
@floxay
Copy link
Contributor Author

floxay commented Jan 27, 2024 •
edited

This seems to have been closed on accident as PR #3035 was merged and incorrectly linked to this issue.
Actual bug report which #3035 resolves is #3029

#3034 is still pending to resolve this issue, should I open a new bug report or is there a way to "unlink" and re-open this one? Or just leave as-is now? @provinzkraut

@provinzkraut provinzkraut reopened this Jan 27, 2024
@provinzkraut
Copy link
Member

This seems to have been closed on accident as PR #3035 was merged and incorrectly linked to this issue. Actual bug report which #3035 resolves is #3029

#3034 is still pending to resolve this issue, should I open a new bug report or is there a way to "unlink" and re-open this one? Or just leave as-is now? @provinzkraut

Should be fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug 🐛 This is something that is not working as expected
Projects
Overview
Status: Triage
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix!: cookie string parser floxay/litestar
3 participants
@peterschutt @provinzkraut @floxay