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
cadata param can (and PEM-encoded cadata must) be unicode under PY2 #3150
Conversation
The documentation for that parameter states: "The cadata object, if present, is either an ASCII string of one or more PEM-encoded certificates or a bytes-like object of DER-encoded certificates." Similar wording is used for PY2 and PY3. But that's a bit of a backporting bug in the documentation, since "ASCII string" and "bytes-like object" sound like they could both be str under PY2. In fact, if the type is str it's assumed to be DER-encoded and PEM-encoded cadata must use the unicode type under PY2 (https://bugs.python.org/issue37079): https://github.com/python/cpython/blob/2149a9ad/Modules/_ssl.c#L3011 Thus, these type declarations should use Union[Text, bytes, None] instead of Union[str, bytes, None]. Also, _create_unverified_context exists in Python 2: https://github.com/python/cpython/blob/2149a9ad/Lib/ssl.py#L448
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.
Thanks for the analysis. I left one comment below.
There is a merge conflict now. |
Resolved, I think. |
CI is failing now though (haven't checked why). |
Whitespace typo in the merge, missed when looking at the diff. Should be fixed. |
This seems to be fixed, can someone merge this, please? |
The documentation for that parameter states: "The cadata object, if
present, is either an ASCII string of one or more PEM-encoded
certificates or a bytes-like object of DER-encoded certificates."
Similar wording is used for PY2 and PY3. But that's a bit of a
backporting bug in the documentation, since "ASCII string" and
"bytes-like object" sound like they could both be str under PY2. In
fact, if the type is str it's assumed to be DER-encoded and
PEM-encoded cadata must use the unicode type under PY2
(https://bugs.python.org/issue37079):
https://github.com/python/cpython/blob/2149a9ad/Modules/_ssl.c#L3011
Thus, these type declarations should use Union[Text, bytes, None]
instead of Union[str, bytes, None].
Also, _create_unverified_context exists in Python 2:
https://github.com/python/cpython/blob/2149a9ad/Lib/ssl.py#L448