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: ResponseDataExtractor merges per cookie attributes #3040

Open
4 tasks
floxay opened this issue Jan 28, 2024 · 1 comment · May be fixed by #3045
Open
4 tasks

Bug: ResponseDataExtractor merges per cookie attributes #3040

floxay opened this issue Jan 28, 2024 · 1 comment · May be fixed by #3045
Labels
Bug 🐛 This is something that is not working as expected

Comments

@floxay
Copy link
Contributor

floxay commented Jan 28, 2024 •
edited

Description

Currently ResponseDataExtractor merges all values of cookies into a single dictionary, however cookies in the Set-Cookie header can have different flags, paths, domains, etc. set which are currently lost.

URL to code causing the issue

litestar/litestar/data_extractors.py

Lines 407 to 410 in e5f2b64

if cookie_string := ";".join(
[x[1].decode("latin-1") for x in filter(lambda x: x[0].lower() == b"set-cookie", messages[0]["headers"])]
):
parsed_cookies = parse_cookie_string(cookie_string)
Test showing how per cookie attributes (all attributes apart from key and value) are combined right now:

litestar/tests/unit/test_data_extractors.py

Line 110 in e5f2b64

assert extracted_data.get("cookies") == {"Path": "/", "SameSite": "lax", "auth": "", "regular": ""}

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 Enhancement This is a new feature or request label Jan 28, 2024
@floxay floxay mentioned this issue Jan 28, 2024
Open
@floxay
Copy link
Contributor Author

floxay commented Jan 28, 2024

I have tagged this as an enhancement initially but in hindsight this probably should be a bug report?

@floxay floxay changed the title Enhancement: change ExtractedResponseData.cookies to a list/sequence Enhancement: change ExtractedResponseData.cookies to a list/sequence of dicts Jan 28, 2024
@floxay floxay changed the title Enhancement: change ExtractedResponseData.cookies to a list/sequence of dicts Bug: change ExtractedResponseData.cookies to a list/sequence of dicts Jan 29, 2024
@floxay floxay linked a pull request Jan 29, 2024 that will close this issue
Open
@floxay floxay changed the title Bug: change ExtractedResponseData.cookies to a list/sequence of dicts Bug: ResponseDataExtractor merges per cookie attributes Jan 29, 2024
@JacobCoffee JacobCoffee added Bug 🐛 This is something that is not working as expected and removed Enhancement This is a new feature or request labels Jan 31, 2024
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: Working on it
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix!: response cookie extraction floxay/litestar
2 participants
@JacobCoffee @floxay