-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-43921: Cleanup test_ssl.test_wrong_cert_tls13() #26520
Conversation
Don't catch OSError, and check the SSLError message.
I don't see why a SSLSocket would raise an OSError on read()/write(). On ECONNRESET error, read() raises an SSLEOFError exception: see https://bugs.python.org/issue43921#msg394967 cc @tiran |
It's based on old code from Antoine, see |
I see. For test_wrong_cert_tls12(), SSLError or OSError is expected on connect(), not on read/write. There is a comment explaining the OSError:
test_wrong_cert_tls13() is different because TLS 1.3 is different. The test doesn't expect any error on connect(), only on read() or write(). It also explained in a comment:
In short, test_wrong_cert_tls12() looks correct, and I now understand how test_wrong_cert_tls13() inherited |
This change makes the test more strict (check the exact error message). I'm not sure that it works well on all platforms and all supported OpenSSL versions. I prefer to not backport the change to 3.10. At least, not now. |
Either keep 3.9 to 3.11 tests in sync or don't merge changes. |
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Sorry @vstinner, I had trouble checking out the |
Sorry, @vstinner, I could not cleanly backport this to |
@vstinner please backport changes to 3.10 and 3.9. Tests should be kept in sync. |
Don't catch OSError, and check the SSLError message.
https://bugs.python.org/issue43921