aiosmtpd icon indicating copy to clipboard operation
aiosmtpd copied to clipboard

aiosmtpd test suite fails on Python 3.10

Open Conan-Kudo opened this issue 2 years ago • 9 comments

While trying to build aiosmtpd 1.4.2 on Fedora for Python 3.10 (as part of upgrading to Python 3.10 for Fedora Linux 35), aiosmtpd failed to build because the test suite fails:

=========================== short test summary info ============================
FAILED aiosmtpd/tests/test_server.py::TestUnixSocketController::test_server_creation_ssl
FAILED aiosmtpd/tests/test_smtp.py::TestAuthMechanisms::test_byclient[login-False]
FAILED aiosmtpd/tests/test_smtp.py::TestAuthArgs::test_warn_authreqnotls - as...
ERROR aiosmtpd/tests/test_smtps.py::TestSMTPS::test_smtps - ssl.SSLError: Can...
ERROR aiosmtpd/tests/test_starttls.py::TestNoTLS::test_disabled_tls - OSError...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_help_starttls - OSE...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_starttls_arg - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_starttls - OSError:...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_starttls_quit - OSE...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_failed_handshake - ...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_tls_handshake_stopcontroller
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_tls_bad_syntax - OS...
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_help_after_starttls
ERROR aiosmtpd/tests/test_starttls.py::TestStartTLS::test_helo_starttls - OSE...
ERROR aiosmtpd/tests/test_starttls.py::TestTLSEnding::test_eof_received - OSE...
ERROR aiosmtpd/tests/test_starttls.py::TestTLSEnding::test_tls_handshake_failing
ERROR aiosmtpd/tests/test_starttls.py::TestTLSForgetsSessionData::test_forget_ehlo
ERROR aiosmtpd/tests/test_starttls.py::TestTLSForgetsSessionData::test_forget_mail
ERROR aiosmtpd/tests/test_starttls.py::TestTLSForgetsSessionData::test_forget_rcpt
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_helo_fails - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_help_fails - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_ehlo - OSError: [...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_mail_fails - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_rcpt_fails - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_vrfy_fails - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_data_fails - OSEr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_noop_okay - OSErr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLS::test_quit_okay - OSErr...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLSAUTH::test_auth_notls - ...
ERROR aiosmtpd/tests/test_starttls.py::TestRequireTLSAUTH::test_auth_tls - OS...
= 3 failed, 521 passed, 1 skipped, 113 warnings, 27 errors in 186.09s (0:03:06) =

Most of it is the test suite returning OSError: [Errno 98] error while attempting to bind on address ('127.0.0.1', 8025): address already in use, but there are other errors too...

Full build log: python-aiosmtpd-1.4.2-3.fc35-build.log.txt

Reference Koji build task: https://koji.fedoraproject.org/koji/taskinfo?taskID=72467725

cc: @abompard, @hroncok

Conan-Kudo avatar Jul 27 '21 12:07 Conan-Kudo

I can reproduce this from git with tox -e py310-nocov.

hroncok avatar Aug 23 '21 15:08 hroncok

FWIW, on a 3.10.0rc2 install, I am seeing only a single test failure:

FAILED aiosmtpd/tests/test_smtp.py::TestAuthMechanisms::test_byclient[login-False] - Failed: DID NOT RAISE <class 'smtplib.SMTPAuthenticationError'>

Results (51.74s):
     561 passed
       1 failed
         - aiosmtpd/tests/test_smtp.py:1053 TestAuthMechanisms.test_byclient[login-False]
       1 skipped

maxking avatar Sep 13 '21 23:09 maxking

With 3.9 I also have the DID NOT RAISE failure.

P-EB avatar Oct 10 '21 11:10 P-EB

Could someone help me backtrack this test failure? For Debian I'm considering disabling that test as it seems it's buggy, but I could have missed something and actually disable something useful.

P-EB avatar Oct 15 '21 12:10 P-EB

Ahhh I see what's happening, at least I believe so. Looks like @pepoluan got https://bugs.python.org/issue27820 resolved, which actually stops this test from failing.

It's a useful test, but it also looks to need some massaging to work across versions

waynew avatar Oct 15 '21 15:10 waynew

@waynew thanks, as usual you're quite better than me at tackling down what changed and what makes things squeak!

P-EB avatar Oct 15 '21 17:10 P-EB

I'll try to replicate the errors in the coming days.

I need to refresh my understanding of the code, it's been awhile... 😅

pepoluan avatar Oct 19 '21 06:10 pepoluan

Hi! We're currently doing rebuilds on Arch Linux for python 3.10. Naturally I would also be interested in a fix for this :)

The current open PRs (e.g. https://github.com/aio-libs/aiosmtpd/pull/284) do not seem to address all issues and it's unclear to me whether they should be used at all.

dvzrv avatar Dec 04 '21 11:12 dvzrv

Based on the discussion on #294 and #303, the problem is not writing the code. That's probably going to be a very trivial change.

The biggest challenge is going to be doing the research to grok the changes in 3.10 in such a way that we can clearly explain why the code change is the correct one.

If someone has the time and desire to tackle that, we should be able to get a fix in/merged, and probably cut a new release.

My own knowledge and experience says that it would probably take me about ~1 day (8+hrs) to refresh my memory, hunt down all of the related bits, and be confident that I knew what was up and the fix that needed to happen. I currently don't have that kind of time in a contiguous block. I might be able to sneak a few hours here and there.

If you're someone who hasn't ever dealt with SSL contexts and server vs client auth, I'd expect a couple of days of effort. If nobody volunteers to jump in and tackle this by July 23 (the Saturday after next), I'll start refreshing my mental context on it.

waynew avatar Jul 04 '22 17:07 waynew

So, I think I've figured out the issue here and it is mostly about using a server side SSL context in client side here https://github.com/aio-libs/aiosmtpd/blob/master/aiosmtpd/controller.py#L428 and here https://github.com/aio-libs/aiosmtpd/blob/master/aiosmtpd/controller.py#L470.

The code used to work on old versions of Python before 3.10 because ssl.PROTOCOL_TLS was previously the default SSL protocol that could be used for both client and server side, so you could pass the same context for both. With 3.10, now ssl.PROTOCOL_TLS is deprecated in the favor of ssl.PROTOCOL_TLS_SERVER and ssl.PROTOCOL_TLS_CLIENT. Even though this is just a deprecation, ssl.create_default_context() used in tests is passed through to the Controller, which will then use the same context to try to initiate a lazy load using the _trigger_server implementations at multiple places. So, even though PROTOCOL_TLS still exists, the fact that create_default_context() has changed the behavior, we can't create a context using it that is applicable to both server and client. I haven't tried it, but it might be possible to do it by instantiating manually, though I don't see a good reason to do that.

I have a branch with the fixes that is passing the CI (all except qa and commenting out the housekeeping stuff that seems to be failing on my machine in all tox environments), but I think I need to think a bit about the solution. I'll be happy to get some comments and if it is okay, I can open a draft PR just for the purpose of discussions.

My branch does need to be broken down into several different PRs to segregate the fixes with other changes I had to make to get the CI running, like bumping actions version, removing python 3.6 and will need some time to get the QA stuff working, there are tons of new flake8 errors showing up.

maxking avatar Oct 26 '22 16:10 maxking