Skip to content
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

SMTP regression and failure during TLS negotiation under Python 3.7. #83

Open
mentat51 opened this issue Oct 16, 2019 · 13 comments
Open
Labels
1.bug An error has been encountered that is preventing utilization. meta:blocked This issue is blocked pending an external factor. transport:smtp

Comments

@mentat51
Copy link

mentat51 commented Oct 16, 2019

Upstream tracking bug: https://bugs.python.org/issue36094


from marrow.mailer import Mailer, Message

mailer = Mailer(dict(
        transport = dict(
                use = 'smtp',
                host = 'smtp.office365.com',
                port = 587,
                tls='required',
                username='[email protected]',
                password='password')))
print('connection...')
mailer.start()
print('connected')

message = Message(author="[email protected]", to="[email protected]")
message.subject = "Testing Marrow Mailer"
message.plain = "This is a test."
print('send mail')
mailer.send(message)

print('disconnect')
mailer.stop()

This works great with python 2.7, but with python 3.7 (tested on 3.7.1 and 3.7.4), I have this error :

Delivery of message <[email protected]> failed.
Traceback (most recent call last):
  File "C:\usr\Python37\lib\site-packages\marrow\mailer\manager\util.py", line 50, in __enter__
    transport = pool.transports.get(False)
  File "C:\usr\Python37\lib\queue.py", line 167, in get
    raise Empty
_queue.Empty

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "minimal_test.py", line 32, in <module>
    mailer.send(message)
  File "C:\usr\Python37\lib\site-packages\marrow\mailer\__init__.py", line 149, in send
    result = self.manager.deliver(message)
  File "C:\usr\Python37\lib\site-packages\marrow\mailer\manager\immediate.py", line 41, in deliver
    with self.transport() as transport:
  File "C:\usr\Python37\lib\site-packages\marrow\mailer\manager\util.py", line 57, in __enter__
    transport.startup()
  File "C:\usr\Python37\lib\site-packages\marrow\mailer\transport\smtp.py", line 50, in startup
    self.connect_to_server()
  File "C:\usr\Python37\lib\site-packages\marrow\mailer\transport\smtp.py", line 86, in connect_to_server
    connection.starttls(self.keyfile, self.certfile)
  File "C:\usr\Python37\lib\smtplib.py", line 771, in starttls
    server_hostname=self._host)
  File "C:\usr\Python37\lib\ssl.py", line 412, in wrap_socket
    session=session
  File "C:\usr\Python37\lib\ssl.py", line 846, in _create
    owner=self, session=self._session,
ValueError: server_hostname cannot be an empty string or start with a leading dot.
@iamraa
Copy link

iamraa commented Feb 7, 2020

You have to monkey-patch this method marrow.mailer.transport.smtp.SMTPTransport.connect_to_server():

# ....
    if self.tls == "ssl":
        connection = SMTP_SSL(
            host=self.host,
            local_hostname=self.local_hostname,
            keyfile=self.keyfile,
            certfile=self.certfile,
            timeout=self.timeout,
        )
    else:
        connection = SMTP(
            host=self.host, local_hostname=self.local_hostname, timeout=self.timeout
        )
# ....

@hyperknot
Copy link

Is there any other way to fix this without monkey patching? Python 3.7 is 1.5 years old, what is the proper way to use this library with it? I hope not a monkey patch.

@amcgregor
Copy link
Member

amcgregor commented Oct 1, 2020

Upstream tracking bug: https://bugs.python.org/issue36094 (my back-referencing comment therein)

Please stop submitting pull requests passing the host name to the constructor. These solutions will be immediately rejected. See also: http://xyproblem.info

@amcgregor amcgregor changed the title Minimal example doesn't work in python 3.7 SMTP regression and failure during TLS negotiation under Python 3.7. Oct 1, 2020
@amcgregor
Copy link
Member

See also: python/cpython#11998 — pull request correcting the issue, pending changes from code review.

@Minstel
Copy link

Minstel commented Jun 6, 2021

This error still exists. No solution except monkey-patching?

@amcgregor
Copy link
Member

@Minstel Please see the official bug report against CPython, linked above your comment.

@amcgregor
Copy link
Member

if I'm understanding properly, this library is rejecting PRs that make it... work? I understand there may be a longstanding upstream issue, but is there any chance we can document "if you're using python 3.7 do exactly this monkey patch?"

feels pedantic?

While I do appreciate the fact this issue comment was subsequently deleted, e-mail notifications make that moot. A point was further ground home; I'm just not sure it's the one that may have been intended. (That was some laborious redaction for politeness, right there.)

To reiterate: the PRs were not accepted because they introduce regressions in expected behavior relating to the ability to enable diagnostics when establishing the initial connection. That's not blowing wind, that's a literal description of the exact problem. The PRs prevented any ability to do so—establishing the connection immediately upon construction of the connection management object. Initiating prior to any ability to invoke the method to set the debug level, which must happen post-initialization. This was documented in at least a few of them.

If there's a correction that can be applied downstream from the actual fault, which does not break existing expectations or behaviors or involve complete reimplementation of SMTP connection handling to preserve them, I'm all ears.

@fdemian
Copy link

fdemian commented Jul 22, 2022

Hi, I'm having this issue with python3.9.

Is this issue supposed to be exclusive to python3.8 and 3.7? Because I switched to 3.9 but it keeps happening.
This is my code (note: there are some irrelevant methods missing).

import sys  # isort:skip
sys.modules["cgi.parse_qsl"] = None
from marrow.mailer import Mailer, Message
from os import path
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart

"""
... 
"""

def construct_email_message(mail_info, settings):
    activation_url = f"{mail_info['url']}/activation/{mail_info['code']}"
    plain_text = f"""Hello, {mail_info['user'].username}. This is your activation url
    {activation_url}
    """
    template_text = get_html_from_template("useractivation")
    html_message = modify_html_template(template_text, mail_info['user'], activation_url)

    # Construct message with plain and html text options.
    message = Message(author=settings["mailAddress"], to=mail_info['user'].email)
    message.subject = 'This is a subject!'
    message.plain = plain_text
    message.rich = html_message

    return message


async def send_email(mail_info):

    # Get mail settings.
    mail_settings = mail_info['options']
    url = mail_settings['url']
    port = mail_settings['port']
    server_url = mail_settings["smtpServer"]
    message = construct_email_message(mail_info, mail_settings)

    if mail_info['type'] == 'local':
        mailer = Mailer({
            'transport.use': 'maildir',
            'transport.directory': 'maildir'
        })
    else:
        user_name = mail_settings['auth']['username']
        user_pass = mail_settings['auth']['password']
        mailer = Mailer(
         dict(
          transport=dict(
            use='smtp',
            host=server_url,
            port='587',
            username=user_name,
            password=user_pass,
            tls='required'
          )
         )
        )

    mailer.start()
    mailer.send(message)
    mailer.stop()
    return

And I get:
ValueError: server_hostname cannot be an empty string or start with a leading dot.

@amcgregor
Copy link
Member

Been doing the rounds updating issues here; so glad to finally have a chance to do this during work hours. #66 (comment) referencing this issue points out that this correction has been conditionally applied within the cegid branch:

marrow/mailer/transport/smtp.py

@seventhridge
Copy link

seventhridge commented Oct 11, 2023

From what I can tell, in the event you want to initiate connection later and not during SMTP_SSL instantiation, you can explicitly pass host=None in Python 3.

Some detailed code inspection indicates that the error happens in the code as it is because the SMTP_SSL connection defaults host to empty string instead of None, and the socket library balks on the SMTP_SSL's self._host internal variable being an empty string. I do not know why SMTP_SSL's constructor defaults host to be an empty string when that clearly does not work with a later call to connect().

I believe according to the docs that providing the host during SMTP_SSL instantiation actually initiates a connection, which might prompt a few other code changes. In my fork, setting host=None works using Python 3.9. I haven't tested it on Python versions before 3.5 as in the cegid branch.

@amcgregor
Copy link
Member

@seventhridge Notably, a diagnostic level / debug flag can't be set prior to connection initiation if a host is explicitly passed; this is the key point I've mentioned across all prior pull requests which "fixed" this by passing the host in. Thank you very much for a pull request I actually need to validate rather than immediately reject! 🙂

Also… so… uh… the Python standard library doesn't follow its own guidelines about is None comparisons? Gasp. Shock. Awe. 🤪 Tell me it ain't so!

@mohllal
Copy link

mohllal commented May 16, 2024

For those looking for a temporary solution to this problem until the underlying Python's issue is addressed.

I've created a custom SMTP transport class based on the standard SMTPTransport class that comes with the mailer package, with the sole adjustment being providing host and port arguments to the constructors of SMTP and SMTP_SSL classes in the connect_to_server method.

https://gist.github.com/mohllal/ae99252a79bda4af30f16630e49f72ea

@amcgregor
Copy link
Member

As I recover from my most recent medical issue, I'll try to incorporate this adaption into the next release. Then, uh… actually perform that release!

Apologies for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.bug An error has been encountered that is preventing utilization. meta:blocked This issue is blocked pending an external factor. transport:smtp
Projects
None yet
8 participants