close
The Wayback Machine - https://web.archive.org/web/20210816115202/https://github.com/aio-libs/aiohttp/issues/5634
Skip to content

/ aiohttp

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

Jump to bottom

[server] Add warning when a cookie's length would exceed the RFC 6265 minimum user-agent support #5634

Open
panagiks opened this issue Apr 20, 2021 · 3 comments
Open

[server] Add warning when a cookie's length would exceed the RFC 6265 minimum user-agent support #5634

panagiks opened this issue Apr 20, 2021 · 3 comments
Labels
enhancement good first issue Hacktoberfest

Comments

@panagiks
Copy link
Contributor

@panagiks panagiks commented Apr 20, 2021

🐣 Is your feature request related to a problem? Please describe.
We had a report in aiohttp-session (see aio-libs/aiohttp-session#574) that a user submitting a 'large' cookie had the cookie dropped by the receiving browser.

According to RFC 6265 - section 6.1:

General-use user agents SHOULD
provide each of the following minimum capabilities:
o At least 4096 bytes per cookie (as measured by the sum of the
length of the cookie's name, value, and attributes).

This means that cookies that exceed 4096 bytes in storage (not transmission) have a chance that the user-agent will drop them which is a case that is hard to 'guess' for a developer.

💡 Describe the solution you'd like
A warning log should be emitted if a cookie that would exceed the RFC minimum support is created in aiohttp server.

A challenge here is the definition of 'exceed' because aiohttp knows the transmitted size but the RFC refers to (and the user-agents seem to implement) a check on the final stored size of the cookie string, including all field names and values regardless of whether they were transmitted or filled in as defaults by the user-agent.

❓ Describe alternatives you've considered
It was briefly considered implementing this check in aiohttp-session but was rejected since this is a general issue with cookies and not specific to aiohttp-session so such a check could benefit all aiohttp users.

📋 Additional context
None.
@Dreamsorcerer
Copy link
Contributor

@Dreamsorcerer Dreamsorcerer commented Apr 20, 2021 •
edited

The warning would go somewhere around:

aiohttp/aiohttp/helpers.py

Line 852 in 1a4126a

self._cookies[name] = value

Maybe requires a little trial and error with a browser forcing this limit, to help figure out exactly what is included in the size limit (the linked issue mentions this being reproducible in Chrome 89).

But, at a guess, this might look something like: len(name) + len(value) + sum(len(k) + len(v) for k, v in c.items())
@anesabml
Copy link
Contributor

@anesabml anesabml commented Aug 9, 2021

@Dreamsorcerer I came across a similar bug report in Django, eventually, they didn't implement a warning or anything like that (they added a warning to the documentation), because of performance reasons (calculating the whole length of every cookie in the cookies set can introduce a performance overhead).

https://code.djangoproject.com/ticket/22242
@Dreamsorcerer
Copy link
Contributor

@Dreamsorcerer Dreamsorcerer commented Aug 9, 2021

Well, that's what dev mode is for, then atleast it will catch errors for people during development.
Just wrap it in if sys.flags.dev_mode:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement good first issue Hacktoberfest
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@Dreamsorcerer @panagiks @anesabml