Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Added implicit connections for smtp #17471. #792

Closed
wants to merge 1 commit into from

4 participants

@ezequielsz

Credits to dje as this is just an update for his patch.

Ticket: https://code.djangoproject.com/ticket/17471

@ezequielsz ezequielsz Added implicit connections for smtp #17471.
Credits to dje as this is just an update for his patch.
90a9140
@charettes charettes commented on the diff
django/core/mail/backends/smtp.py
@@ -31,6 +31,10 @@ def __init__(self, host=None, port=None, username=None, password=None,
self.use_tls = settings.EMAIL_USE_TLS
else:
self.use_tls = use_tls
+ if use_ssl is None:
+ self.use_ssl = settings.EMAIL_USE_SSL
+ else:
+ self.use_ssl = use_ssl
self.connection = None
@charettes Collaborator

Maybe we could raise a ValueError if self.use_ssl and self.use_tls? The message should also make reference to EMAIL_USE_SSL and EMAIL_USE_TLS since we default to them when use_(ssl|tsl) are None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@charettes charettes commented on the diff
docs/ref/settings.txt
@@ -996,6 +996,21 @@ EMAIL_USE_TLS
Default: ``False``
Whether to use a TLS (secure) connection when talking to the SMTP server.
+This is used for explicit TLS connections, generally on port 587. If you are
+experiencing hanging connections, see the implicit TLS setting
+:setting:`EMAIL_USE_SSL`.
+
+.. setting:: EMAIL_USE_SSL
+
+EMAIL_USE_SSL
@charettes Collaborator

This will need a versionadded clause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@charettes charettes commented on the diff
tests/regressiontests/mail/tests.py
((25 lines not shown))
+ def test_email_ssl_override_settings(self):
+ backend = smtp.EmailBackend(use_ssl=False)
+ self.assertFalse(backend.use_ssl)
+
+ def test_email_ssl_default_disabled(self):
+ backend = smtp.EmailBackend()
+ self.assertFalse(backend.use_ssl)
+
+ @override_settings(EMAIL_USE_TLS=True)
+ def test_email_tls_attempts_starttls(self):
+ backend = smtp.EmailBackend()
+ self.assertTrue(backend.use_tls)
+ try:
+ backend.open()
+ self.fail('SMTPException STARTTLS not raised.')
+ except SMTPException, e:
@charettes Collaborator

This will fail on PY3 because of the comma syntax. Can this be replaced by a assertRaisesMessage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@charettes charettes commented on the diff
tests/regressiontests/mail/tests.py
((36 lines not shown))
+ self.assertTrue(backend.use_tls)
+ try:
+ backend.open()
+ self.fail('SMTPException STARTTLS not raised.')
+ except SMTPException, e:
+ self.assertNotEqual(-1, str(e).find('STARTTLS'), "SMTPException wasn't for STARTTLS")
+
+ @override_settings(EMAIL_USE_SSL=True)
+ def test_email_ssl_attempts_ssl_connection(self):
+ backend = smtp.EmailBackend()
+ self.assertTrue(backend.use_ssl)
+ try:
+ backend.open()
+ self.fail('SSLError not raised.')
+ except SSLError, e:
+ pass
@charettes Collaborator

This whole try/except logic can be replaced by a self.assertRaises(SSLError, backend.open).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@charettes charettes commented on the diff
tests/regressiontests/mail/tests.py
@@ -21,6 +21,10 @@
from django.utils.six import PY3, StringIO
from django.utils.translation import ugettext_lazy
+from smtplib import SMTPException
+from ssl import SSLError
+
@charettes Collaborator

Please move standard library imports with the other ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@areski

during djangocon eu I tried to correct this ticket :
please find a new pull request : #1124

@timgraham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 23, 2013
  1. @ezequielsz

    Added implicit connections for smtp #17471.

    ezequielsz authored
    Credits to dje as this is just an update for his patch.
This page is out of date. Refresh to see the latest.
View
1  django/conf/global_settings.py
@@ -175,6 +175,7 @@
EMAIL_HOST_USER = ''
EMAIL_HOST_PASSWORD = ''
EMAIL_USE_TLS = False
+EMAIL_USE_SSL = False
# List of strings representing installed apps.
INSTALLED_APPS = ()
View
22 django/core/mail/backends/smtp.py
@@ -15,7 +15,7 @@ class EmailBackend(BaseEmailBackend):
A wrapper that manages the SMTP network connection.
"""
def __init__(self, host=None, port=None, username=None, password=None,
- use_tls=None, fail_silently=False, **kwargs):
+ use_tls=None, fail_silently=False, use_ssl=None, **kwargs):
super(EmailBackend, self).__init__(fail_silently=fail_silently)
self.host = host or settings.EMAIL_HOST
self.port = port or settings.EMAIL_PORT
@@ -31,6 +31,10 @@ def __init__(self, host=None, port=None, username=None, password=None,
self.use_tls = settings.EMAIL_USE_TLS
else:
self.use_tls = use_tls
+ if use_ssl is None:
+ self.use_ssl = settings.EMAIL_USE_SSL
+ else:
+ self.use_ssl = use_ssl
self.connection = None
@charettes Collaborator

Maybe we could raise a ValueError if self.use_ssl and self.use_tls? The message should also make reference to EMAIL_USE_SSL and EMAIL_USE_TLS since we default to them when use_(ssl|tsl) are None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
self._lock = threading.RLock()
@@ -45,12 +49,18 @@ def open(self):
try:
# If local_hostname is not specified, socket.getfqdn() gets used.
# For performance, we use the cached FQDN for local_hostname.
- self.connection = smtplib.SMTP(self.host, self.port,
+ if self.use_ssl:
+ self.connection = smtplib.SMTP_SSL(self.host, self.port,
+ local_hostname=DNS_NAME.get_fqdn())
+ else:
+ self.connection = smtplib.SMTP(self.host, self.port,
local_hostname=DNS_NAME.get_fqdn())
- if self.use_tls:
- self.connection.ehlo()
- self.connection.starttls()
- self.connection.ehlo()
+ # TLS/SSL are mutually exclusive, so only attempt TLS over
+ # non-secure connections.
+ if self.use_tls:
+ self.connection.ehlo()
+ self.connection.starttls()
+ self.connection.ehlo()
if self.username and self.password:
self.connection.login(self.username, self.password)
return True
View
15 docs/ref/settings.txt
@@ -996,6 +996,21 @@ EMAIL_USE_TLS
Default: ``False``
Whether to use a TLS (secure) connection when talking to the SMTP server.
+This is used for explicit TLS connections, generally on port 587. If you are
+experiencing hanging connections, see the implicit TLS setting
+:setting:`EMAIL_USE_SSL`.
+
+.. setting:: EMAIL_USE_SSL
+
+EMAIL_USE_SSL
@charettes Collaborator

This will need a versionadded clause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+-------------
+
+Default: ``False``
+
+Whether to use an implicit TLS (secure) connection when talking to the SMTP
+server. In most email documentation this type of TLS connection is referred
+to as SSL. It is generally used on port 465. If you are experiencing problems,
+see the explicit TLS setting :setting:`EMAIL_USE_TLS`.
.. setting:: FILE_CHARSET
View
7 docs/topics/email.txt
@@ -27,7 +27,8 @@ Mail is sent using the SMTP host and port specified in the
:setting:`EMAIL_HOST` and :setting:`EMAIL_PORT` settings. The
:setting:`EMAIL_HOST_USER` and :setting:`EMAIL_HOST_PASSWORD` settings, if
set, are used to authenticate to the SMTP server, and the
-:setting:`EMAIL_USE_TLS` setting controls whether a secure connection is used.
+:setting:`EMAIL_USE_TLS` and :setting:`EMAIL_USE_SSL` settings control whether
+a secure connection is used.
.. note::
@@ -408,8 +409,8 @@ SMTP backend
This is the default backend. Email will be sent through a SMTP server.
The server address and authentication credentials are set in the
:setting:`EMAIL_HOST`, :setting:`EMAIL_PORT`, :setting:`EMAIL_HOST_USER`,
-:setting:`EMAIL_HOST_PASSWORD` and :setting:`EMAIL_USE_TLS` settings in your
-settings file.
+:setting:`EMAIL_HOST_PASSWORD`, :setting:`EMAIL_USE_TLS` and
+:setting:`EMAIL_USE_SSL` settings in your settings file.
The SMTP backend is the default configuration inherited by Django. If you
want to specify it explicitly, put the following in your settings::
View
53 tests/regressiontests/mail/tests.py
@@ -21,6 +21,10 @@
from django.utils.six import PY3, StringIO
from django.utils.translation import ugettext_lazy
+from smtplib import SMTPException
+from ssl import SSLError
+
@charettes Collaborator

Please move standard library imports with the other ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
class MailTests(TestCase):
"""
@@ -738,3 +742,52 @@ def test_server_stopped(self):
backend.close()
except Exception as e:
self.fail("close() unexpectedly raised an exception: %s" % e)
+
+ @override_settings(EMAIL_USE_TLS=True)
+ def test_email_tls_use_settings(self):
+ backend = smtp.EmailBackend()
+ self.assertTrue(backend.use_tls)
+
+ @override_settings(EMAIL_USE_TLS=True)
+ def test_email_tls_override_settings(self):
+ backend = smtp.EmailBackend(use_tls=False)
+ self.assertFalse(backend.use_tls)
+
+ def test_email_tls_default_disabled(self):
+ backend = smtp.EmailBackend()
+ self.assertFalse(backend.use_tls)
+
+ @override_settings(EMAIL_USE_SSL=True)
+ def test_email_ssl_use_settings(self):
+ backend = smtp.EmailBackend()
+ self.assertTrue(backend.use_ssl)
+
+ @override_settings(EMAIL_USE_SSL=True)
+ def test_email_ssl_override_settings(self):
+ backend = smtp.EmailBackend(use_ssl=False)
+ self.assertFalse(backend.use_ssl)
+
+ def test_email_ssl_default_disabled(self):
+ backend = smtp.EmailBackend()
+ self.assertFalse(backend.use_ssl)
+
+ @override_settings(EMAIL_USE_TLS=True)
+ def test_email_tls_attempts_starttls(self):
+ backend = smtp.EmailBackend()
+ self.assertTrue(backend.use_tls)
+ try:
+ backend.open()
+ self.fail('SMTPException STARTTLS not raised.')
+ except SMTPException, e:
@charettes Collaborator

This will fail on PY3 because of the comma syntax. Can this be replaced by a assertRaisesMessage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ self.assertNotEqual(-1, str(e).find('STARTTLS'), "SMTPException wasn't for STARTTLS")
+
+ @override_settings(EMAIL_USE_SSL=True)
+ def test_email_ssl_attempts_ssl_connection(self):
+ backend = smtp.EmailBackend()
+ self.assertTrue(backend.use_ssl)
+ try:
+ backend.open()
+ self.fail('SSLError not raised.')
+ except SSLError, e:
+ pass
@charettes Collaborator

This whole try/except logic can be replaced by a self.assertRaises(SSLError, backend.open).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
Something went wrong with that request. Please try again.