-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-135528: Support more Second-Level Domain Names in lib http.cookiejar #135529
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
Conversation
Misc/NEWS.d/next/Library/2025-06-15-03-38-13.gh-issue-135528.XssMeM.rst
Outdated
Show resolved
Hide resolved
…ssMeM.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
"gov", "mil", "int", "aero", "biz", "cat", "coop", | ||
"info", "jobs", "mobi", "museum", "name", "pro", | ||
"travel", "eu") and len(tld) == 2: | ||
known_slds = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure hard code here would be a good idea.
- I think known_slds should be a global variable
- Maybe we can allow people to customize the slds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already been there for a while, and it doesn't cost to have a local variable. I prefer treating customization as a separate FR (and it should rather be a class variable rather than a global one)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will work on this recently for a feature request. Good idea tho. I'm pretty interested. I will launch another issue with PR when I'm done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I think we could just use this before the FR, since its been there for a while. And I'm not sure changing it to global var(or class var) and add customizing won't cause some strange bugs elsewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and the updated list of second-domain will likely become the default one in customization. For now I think we could just update it as a hard code and change it after the FR is done
I will deal with the customize FR later, but since we dont know if the FR will pass, I think we can merge this list update before. |
I will open a new PR for the customizations and directly add the new second-domains in the default list. |
PR for: #135528
Thank for your time!