gh-95816: fix client_context example from ssl protocol versions docs #93798
Conversation
Doc/library/ssl.rst
Outdated
| >>> client_context.maximum_version = ssl.TLSVersion.MAXIMUM_VERSION | ||
|
|
||
|
|
||
| The SSL context created above will only allow TLSv1.2 and later (if |
There was a problem hiding this comment.
this states the context above supports TLSv1.2-max but the example was TLSv1.3 only
There was a problem hiding this comment.
@graingert If you update to current main the wording now states "TLSv1.3 and later" — so I'd recommend to match ssl.* min version with main too, but definitely propose the MAXIMUM_SUPPORTED anyways which makes much more sense compared to the current docs.
(While the docs specify "Deprecated since version 3.10: All TLSVersion members except TLSVersion.TLSv1_2 and TLSVersion.TLSv1_3 are deprecated." I believe that doesn't include the magic constants, as I can't see any proof in the deprecation warning code to raise any warnings for the MIN_/MAX_ constants, so it's just the description that's written too broadly.)
There was a problem hiding this comment.
On the other hand, configuring the max version in this way is superfluous, as that is by the definition the default:
"SSLContext.maximum_version — A TLSVersion enum member representing the highest supported TLS version. The value defaults to TLSVersion.MAXIMUM_SUPPORTED."
— https://docs.python.org/3/library/ssl.html#ssl.SSLContext.minimum_version
so there's no point of showing an example set this way.
I'd personally vouch for showing a real range as in #107273 (review) …
| >>> client_context.minimum_version = ssl.TLSVersion.TLSv1_3 | ||
| >>> client_context.maximum_version = ssl.TLSVersion.TLSv1_3 |
There was a problem hiding this comment.
|
I'm not sure if it's better to update the code example or the description here |
graingert
changed the title
There was a problem hiding this comment.
There have been some minor changes in the past year, so updating few bits would align this more with the currently published docs, as the explainer bumped versions in e5252c6 (see #95816 (comment) for progress)
Co-authored-by: Jan Brasna <[email protected]>
Co-authored-by: Jan Brasna <[email protected]>
| >>> client_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) | ||
| >>> client_context.minimum_version = ssl.TLSVersion.TLSv1_3 | ||
| >>> client_context.maximum_version = ssl.TLSVersion.TLSv1_3 | ||
| >>> client_context.maximum_version = ssl.TLSVersion.MAXIMUM_SUPPORTED |
There was a problem hiding this comment.
I think just removing maximum_version would automatically make it v1.3+ itself.
|
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 |
4 participants
Uh oh!
There was an error while loading. Please reload this page.