[Django] #26487: Use EHLO after smtplib.SMTP_SSL too.
Django
--------------------------------------+--------------------
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
In file django/core/mail/backends/smtp.py after smtplib.SMTP_SSL EHLO
should be used.
--
Ticket URL: <https://code.djangoproject.com/ticket/26487>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by kchmiela):
* Attachment "patch.diff" added.
Patch
Django
Reporter: kchmiela | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Mail) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I haven't looked at the history behind this yet, but in reading
[https://docs.python.org/3/library/smtplib.html#smtplib.SMTP.ehlo the
Python docs]:
Unless you wish to use has_extn() before sending mail, it should not be
necessary to call this method explicitly. It will be implicitly called by
sendmail() when necessary.
I don't see a `has_extn()` call in Django, so maybe the call can be
removed?
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:1>
Django
Reporter: kchmiela | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Mail) | Version: master
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by the-kid89):
Well you are correct in all currently supported versions of Python it does
state that its not needed unless using has_extn().
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:2>
Django
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
The ticket is missing the rationale of what problem is being solved. Is it
to detect communication failures in `open()` rather than wait until
sending a message? Maybe `ehlo_or_helo_if_needed()` should be used
instead?
{{{ #!diff
diff --git a/django/core/mail/backends/smtp.py
b/django/core/mail/backends/smtp.py
index 432f3a6..40778eb 100644
--- a/django/core/mail/backends/smtp.py
+++ b/django/core/mail/backends/smtp.py
@@ -56,13 +56,12 @@ class EmailBackend(BaseEmailBackend):
})
try:
self.connection = connection_class(self.host, self.port,
**connection_params)
+ self.connection.ehlo_or_helo_if_needed()
# TLS/SSL are mutually exclusive, so only attempt TLS over
# non-secure connections.
if not self.use_ssl and self.use_tls:
- self.connection.ehlo()
self.connection.starttls(keyfile=self.ssl_keyfile,
certfile=self.ssl_certfile)
- self.connection.ehlo()
if self.username and self.password:
self.connection.login(self.username, self.password)
return True
}}}
Anyway, tests are still needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:3>
Django
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Comment (by claudep):
In my opinion, both `ehlo()` calls could be deleted, and nothing should be
added. `starttls`, `login` and other connection methods are caring for
that themselves. However, they should not be harmful either.
If there is a real problem with `EmailBackend` not calling `ehlo()`,
please provide more details.
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:4>
Django
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Comment (by timgraham):
The docs for
[https://github.com/python/cpython/commit/029b2ce0c309bcf933a16b8bbf9f6140a20b5102
starttls()] say, "You should then call ehlo() again." This might be an
obsolete requirement as the `sendmail()` and `login()` method's in
Python's `Lib/smtplib.py` call `self.ehlo_or_helo_if_needed()`. I'm not
familiar with the SMTP protocol though so I'm not positive what this all
means.
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:5>
Django
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by Tim Graham):
* needs_tests: 1 => 0
Comment:
I'll [https://github.com/django/django/pull/7725 remove the ehlo() calls]
and close this ticket as "needsinfo" unless anyone can explain why the
change proposed in the ticket's description is needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:6>
Django
Reporter: kchmiela | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by GitHub <noreply@…>):
In [changeset:"bcae045de01588debe49b266a07a5bfb95068679" bcae045d]:
{{{
#!CommitTicketReference repository=""
revision="bcae045de01588debe49b266a07a5bfb95068679"
Refs #26487 -- Removed unneeded ehlo() calls in SMTP backend.
starttls(), login(), and other connection methods already call
the method as needed.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:7>
Django
Reporter: kchmiela | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Core (Mail) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/26487#comment:8>