Skip to content

gh-135768: fix allowed/blocked IPv6 domains in http.cookiejar#135771

Open
LamentXU123 wants to merge 62 commits intopython:mainfrom
LamentXU123:support-IPv6-in-cookiejar
Open

gh-135768: fix allowed/blocked IPv6 domains in http.cookiejar#135771
LamentXU123 wants to merge 62 commits intopython:mainfrom
LamentXU123:support-IPv6-in-cookiejar

Conversation

@LamentXU123
Copy link
Copy Markdown
Contributor

@LamentXU123 LamentXU123 commented Jun 20, 2025

Copy link
Copy Markdown
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.

Please add tests.

Comment thread Lib/http/cookiejar.py
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Jun 20, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

LamentXU123 and others added 5 commits June 21, 2025 01:19
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py
Comment thread Misc/NEWS.d/next/Library/2025-06-20-17-03-51.gh-issue-135768.DhUJWf.rst Outdated
…hUJWf.rst

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Comment thread Lib/http/cookiejar.py
Comment thread Lib/http/cookiejar.py Outdated
@picnixz picnixz changed the title gh-135768: Support IPv6 in http.cookiejar gh-135768: fix allowed/blocked IPv6 domains in http.cookiejar Jun 20, 2025
@LamentXU123
Copy link
Copy Markdown
Contributor Author

Please add tests.

Done.

@LamentXU123 LamentXU123 requested a review from picnixz June 20, 2025 18:18
Comment thread Lib/http/cookiejar.py
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/test/test_http_cookiejar.py
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py
Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
LamentXU123 and others added 3 commits June 22, 2025 22:48
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@LamentXU123
Copy link
Copy Markdown
Contributor Author

The helper functions will help a lot in the future PRs I think.

Copy link
Copy Markdown
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.

last nits and we're done

Comment thread Lib/http/cookiejar.py Outdated
Comment thread Lib/http/cookiejar.py Outdated
LamentXU123 and others added 2 commits June 22, 2025 23:05
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@LamentXU123
Copy link
Copy Markdown
Contributor Author

last nits and we're done

Thanks! This is my first time ever to create a PR for Cpython with nearly a hundred lines of code. Thank you for your patience and guidence!

Copy link
Copy Markdown
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.

I'm mostly good but I'll ask another expert just in case @vadmium.

Comment thread Lib/http/cookiejar.py Outdated
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@LamentXU123
Copy link
Copy Markdown
Contributor Author

I'm mostly good but I'll ask another expert just in case @vadmium.

May I close the issue? I think its mostly solved.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Jun 22, 2025

No, we should never close an issue until the corresponding PR has been merged or we abandoned the proposal. In this case, closing the issue means that both the PR and its backports have been merged.

@LamentXU123
Copy link
Copy Markdown
Contributor Author

No, we should never close an issue until the corresponding PR has been merged or we abandoned the proposal. In this case, closing the issue means that both the PR and its backports have been merged.

Got it!

@LamentXU123
Copy link
Copy Markdown
Contributor Author

LamentXU123 commented Jun 23, 2025

I will also add warnings in docs indicating ::1 in not valid when they are blocking or allowing hostname after this is merged, also, remove the duplicated tests in another PR. IPv6 appears to be an edge case in many functions of http.cookiejar, I'll try to fix them one by one in the future.

@LamentXU123
Copy link
Copy Markdown
Contributor Author

Hi @picnixz, friendly pinging here. Shell we get this PR merged? caz I can't contribute to further following PRs and issues if this is not merged. TiA ;) (I think it is mostly solved, and tiny fixes can be coped later, if any.)

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Jun 24, 2025

No. I want to hear from Martin first. PRs don't need to be rushed and it's common to have PRs landing after a few weeks.

You can still contribute to other parts of the library (which I would advise you to do so).

@picnixz picnixz requested a review from vadmium June 24, 2025 12:07
@github-actions
Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants