Skip to content

gh-92936: update http.cookies docs post GH-113663 #137566

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nburns
Copy link
Contributor

@nburns nburns commented Aug 8, 2025

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add a What's New entry (Doc/whatsnew/3.15.rst).

@picnixz picnixz changed the title gh-92936: update http.cookie docs gh-92936: update http.cookies docs post GH-113663 Aug 8, 2025
@nburns nburns requested a review from AA-Turner as a code owner August 8, 2025 20:35
@nburns nburns force-pushed the update-http-cookie-docs-example branch from 1416a5f to 8598956 Compare August 8, 2025 20:37
@nburns nburns force-pushed the update-http-cookie-docs-example branch from 8598956 to 75803bc Compare August 8, 2025 20:57
@picnixz
Copy link
Member

picnixz commented Aug 8, 2025

Please avoid force-pushing. It makes incremental review harder. We squash-merge commits at the end.

@nburns
Copy link
Contributor Author

nburns commented Aug 8, 2025

I sure will, thanks for all your help!

>>> C.load('cookies=7; mixins="{"chips": "dark chocolate"}"; state="gooey"')
>>> print(C)
Set-Cookie: cookies=7
Set-Cookie: mixins="{"chips": "dark chocolate"}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I'm wondering, shouldn't we have quotes? I actually didn't check how browsers do but do they escape quotes? (we have an example above that escape the quotes which is why I'm asking).

Copy link
Contributor Author

@nburns nburns Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC: A string like that is technically not included in the RFC, but all the major browsers support strings like that.

There was a big 3rd party (zoom) that was putting JSON in cookies for a while, and all the cookies after that one would just get truncated by the python cookie module. In our case our session cookie came after the 3rd party one, resulting in users being "logged out" in a super hard to debug way.

So as a more direct answer to your question ITT: We shouldn't care about quotes, and if someone cares to escape quotes inside their cookies that's up to them. Ideally the cookies module should put as few restrictions on the value of a cookie as possible (while still being able to parse the cookies), since that's an application level consideration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yt-dlp/yt-dlp#4780 and aio-libs/aiohttp#7993 have both taken it upon themselves to write a whole cookie parsing module to work around this issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, browsers do support strings like these. If that is the case, then it's fine. Historically, the module applies strict parsing so if this is not allowed by the RFC, we shouldn't do it either and that's an issue with the applications themselves. However, we relaxed the rules so this is an acceptable change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, we should demonstrate with an escaped double quote, for json like value in the docs. We wont get a doubt if it is a valid json / valid python string.

>>> import http.cookies
>>> C = http.cookies.SimpleCookie()
>>> C.load('cookies=7; mixins="{\"chips\": \"dark chocolate\"}"; state="gooey"')
>>> C
<SimpleCookie: cookies='7' mixins='{"chips": "dark chocolate"}' state='gooey'>

Previous behavior was wrong for this state too.

Python 3.13.4 (main, Jun  3 2025, 15:34:24) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import http.cookies
>>> C = http.cookies.SimpleCookie()
>>> C.load('cookies=7; mixins="{\"chips\": \"dark chocolate\"}"; state="gooey"')
>>> C
<SimpleCookie: cookies='7'>

>>> C.load('cookies=7; mixins="{"chips": "dark chocolate"}"; state="gooey"')
>>> C
<SimpleCookie: cookies='7'>

Copy link
Contributor Author

@nburns nburns Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the / is getting consumed?

>>> C = cookies.SimpleCookie()
>>> C.load('cookies=7; mixins="{"chips": "dark chocolate"}"; state="gooey"')
>>> print(C)
Set-Cookie: cookies=7
Set-Cookie: mixins="{"chips": "dark chocolate"}"
Set-Cookie: state="gooey"
>>> C = cookies.SimpleCookie()
>>> C.load(r'cookies=7; mixins="{\"chips\": \"dark chocolate\"}"; state="gooey"')
>>> print(C)
Set-Cookie: cookies=7
Set-Cookie: mixins="{\"chips\": \"dark chocolate\"}"
Set-Cookie: state="gooey"

since

>>> '\"' == '"'
True

Copy link
Member

@orsenthil orsenthil Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, In the docs, I was wondering if giving an example that is more valid of ways of representing json or python string would help. Like my snippet above. Otherwise, this double-quotes within double-quotes in docs, I suspect, will invite more questions in the future. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the unescaped-quote-inside-quotes is kinda weird looking, but in the case of the bug, that was exactly the problem that was breaking cookie parsing. Not opposed to changing don't want to also make some docs that are misleading, or otherwise odd.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it acceptable to use other modules in examples? Maybe something like this which shows it's json in a cookie:

>>> C = http.cookies.SimpleCookie()
>>> C.load(f'mixins="{json.dumps({"chips": "dark chocolate"})}";')
>>> print(C)
Set-Cookie: mixins="{"chips": "dark chocolate"}"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like this which shows it's json in a cookie

This is better. It is find to use stdlib module json in this example as demonstrates the json handling with the cookie. Please update the example with this suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir skip news
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants