search

aio-libs / aiosmtpd

A reimplementation of the Python stdlib smtpd.py based on asyncio.
https://aiosmtpd.aio-libs.org
Apache License 2.0
320 stars 96 forks source link

aiosmtpd test suite fails on Python 3.10 #277

Closed Conan-Kudo closed 1 year ago

Conan-Kudo commented 3 years ago

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

hroncok commented 3 years ago

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

maxking commented 3 years ago

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
P-EB commented 3 years ago

With 3.9 I also have the DID NOT RAISE failure.

P-EB commented 3 years ago

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.

waynew commented 3 years ago

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

P-EB commented 3 years ago

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

pepoluan commented 3 years ago

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

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

dvzrv commented 2 years ago

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.

waynew commented 2 years ago

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.

maxking commented 1 year ago

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.