Skip to content

bpo-39616: clarify SSLContext.check_hostname effect #18484

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
Merged
merged 1 commit into from
Mar 27, 2021
Merged

bpo-39616: clarify SSLContext.check_hostname effect #18484

merged 1 commit into from
Mar 27, 2021

Conversation

scop
Copy link
Contributor

@scop scop commented Feb 12, 2020 •
edited by miss-islington
Loading

It doesn't actually affect whether match_hostname() is called (it
never is in this context any longer), but whether hostname
verification occurs in the first place.

https://bugs.python.org/issue39616

Automerge-Triggered-By: GH:tiran

@scop scop requested a review from tiran as a code owner February 12, 2020 12:39
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Feb 12, 2020
@ammaraskar
Copy link
Member

For reference this was removed as part of: 61d478c#diff-c49248c7181161e24048bec5e35ba953

Looks like the proper .. versionchanged directives were added in the docs but this little bit was missed out.

ammaraskar
ammaraskar approved these changes Feb 14, 2020
View reviewed changes
Copy link
Member

@ammaraskar ammaraskar left a comment

Choose a reason for hiding this comment

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

LGTM with suggestion.

@scop scop force-pushed the check_hostname-match_hostname branch from cc4a99a to 917a5c4 Compare February 15, 2020 08:07
@iritkatriel iritkatriel added needs backport to 3.9 only security fixes and removed needs backport to 3.7 labels Mar 26, 2021
tiran
tiran requested changes Mar 27, 2021
View reviewed changes
Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

Could you please just remove "with :func:match_hostname" and leave the rest of the text formatted as it is? This reduces the diff. It's ok when some of the lines don't expand to 80 chars.

@bedevere-bot
Copy link

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.

@scop
Copy link
Contributor Author

scop commented Mar 27, 2021

Could you please just remove "with :func:match_hostname" and leave the rest of the text formatted as it is?

This is exactly what I did initially, but was asked to re-flow it. Will re-unflow again now :P

@scop
bpo-39616: clarify SSLContext.check_hostname effect
77a8118 
It doesn't actually affect whether match_hostname() is called (it
never is in this context any longer), but whether hostname
verification occurs in the first place.
@scop scop force-pushed the check_hostname-match_hostname branch from 917a5c4 to 77a8118 Compare March 27, 2021 13:51
@scop
Copy link
Contributor Author

scop commented Mar 27, 2021

Done.

@tiran
Copy link
Member

tiran commented Mar 27, 2021

This is exactly what I did initially, but was asked to re-flow it. Will re-unflow again now :P

Oh, sorry for the extra work. I prefer smaller diffs over slightly nicer formatted markup. It's a matter of personal taste.

@miss-islington miss-islington merged commit 9798cef into python:master Mar 27, 2021
@miss-islington
Copy link
Contributor

Thanks @scop for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 27, 2021
@scop @miss-islington
bpo-39616: clarify SSLContext.check_hostname effect (pythonGH-18484)
d93aa57 
It doesn't actually affect whether match_hostname() is called (it
never is in this context any longer), but whether hostname
verification occurs in the first place.
(cherry picked from commit 9798cef)

Co-authored-by: Ville Skyttä <[email protected]>
@bedevere-bot
Copy link

GH-25036 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Mar 27, 2021
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 27, 2021
@scop @miss-islington
bpo-39616: clarify SSLContext.check_hostname effect (pythonGH-18484)
644949f 
It doesn't actually affect whether match_hostname() is called (it
never is in this context any longer), but whether hostname
verification occurs in the first place.
(cherry picked from commit 9798cef)

Co-authored-by: Ville Skyttä <[email protected]>
@bedevere-bot
Copy link

GH-25037 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Mar 27, 2021
@miss-islington @scop
bpo-39616: clarify SSLContext.check_hostname effect (GH-18484)
9de6451 
It doesn't actually affect whether match_hostname() is called (it
never is in this context any longer), but whether hostname
verification occurs in the first place.
(cherry picked from commit 9798cef)

Co-authored-by: Ville Skyttä <[email protected]>
miss-islington added a commit that referenced this pull request Mar 27, 2021
@miss-islington @scop
bpo-39616: clarify SSLContext.check_hostname effect (GH-18484)
c84e769 
It doesn't actually affect whether match_hostname() is called (it
never is in this context any longer), but whether hostname
verification occurs in the first place.
(cherry picked from commit 9798cef)

Co-authored-by: Ville Skyttä <[email protected]>
@scop scop deleted the check_hostname-match_hostname branch March 27, 2021 17:36
@scop scop mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiran tiran tiran approved these changes

@ammaraskar ammaraskar ammaraskar approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@scop @ammaraskar @bedevere-bot @tiran @miss-islington @iritkatriel @the-knights-who-say-ni