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.

Sign up for GitHub

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

Jump to bottom

mail: fixed STARTTLS module working with python 3.7.0 #47412

Merged
merged 1 commit into from
Nov 6, 2018
Merged

mail: fixed STARTTLS module working with python 3.7.0 #47412

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/ansible/modules/notification/mail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ def main():
try:
if secure != 'never':
try:
smtp = smtplib.SMTP_SSL(host=host, timeout=timeout)
code, smtpmessage = smtp.connect(host, port=port)
smtp = smtplib.SMTP_SSL(host=host, port=port, timeout=timeout)
code, smtpmessage = smtp.connect(host, port)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange to me: the documentation of smtplib says that if you pass host/port to the constructor, it'll call connect for you:

If the optional host and port parameters are given, the SMTP connect() method is called with those parameters during initialization.

So you're connecting to the SMTP server twice -- why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First connect is a secure layer.

Copy link

@tpeyton tpeyton Nov 21, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the mail module on Ansible v2.7.1 on Python 3.7.1 and if you specify secure: never the mail module works fine. Changes are only necessary to be applied to the attempt to use ssl (which you did on lines 267 & 268). By changing the regular smtp section, it breaks functionality for ansible users that run on python 2.7.x. Is there a reason you changed this section as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tpeyton, because smtp_ssl and smtp_over_tls is different things.

secure_state = True
except ssl.SSLError as e:
if secure == 'always':
Expand All @@ -275,8 +275,8 @@ def main():
pass

if not secure_state:
smtp = smtplib.SMTP(timeout=timeout)
code, smtpmessage = smtp.connect(host, port=port)
smtp = smtplib.SMTP(host=host, port=port, timeout=timeout)
code, smtpmessage = smtp.connect(host, port)

except smtplib.SMTPException as e:
module.fail_json(rc=1, msg='Unable to Connect %s:%s: %s' % (host, port, to_native(e)), exception=traceback.format_exc())
Expand Down