Skip to content

Conversation

@ofek
Copy link
Contributor

@ofek ofek commented Mar 4, 2019

get_server_certificate is the function it is referring to; there is no fetch_server_certificate

@csabella
Copy link
Contributor

csabella commented Mar 5, 2019

Hi @ofek, thank you for reporting this and suggesting a fix. I'm afraid the issue here may be deeper than just the function name and would require a bpo issue.

First, some history. In the commit on 8/28/2007, the ssl.py module was first added and it contained the fetch_server_certificate() function. That function was removed with commit r57680 on 8/30/2007. The function get_server_certificate() was added in commit r58164 on 9/18/2007.

One problem with your correction of the docstring is that the original argument list to fetch_server_certificate() was (HOST, PORT) just as the docstring says. However, those are not the arguments to get_server_certificate(), so it is not correct to keep the same signature. And, because of that, even the textual description doesn't match the function's docstring.

Additionally, there are more than just 2 functions now, so it seems to me that the entire module docstring should be reviewed and updated to reflect the current state of the module or else it should be removed since it hasn't been kept in sync.

In any event, that would all need to be discussed on the issue tracker, so I'm closing this PR. Please open the bpo ticket. Thanks!

@csabella csabella closed this Mar 5, 2019
@ofek ofek deleted the patch-1 branch August 28, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants