Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#21271 closed Cleanup/optimization (fixed)

Django's usage of smtplib.SMTP should have a timeout

Reported by: André Cruz Owned by: nobody
Component: Core (Mail) Version: 1.5
Severity: Normal Keywords: smtp timeout
Cc: susan.tan.fleckerl@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

When EmailBackend initializes SMTP objects it does not provide a timeout, and the default timeout is object() (no timeout). It would be sensible to provide some configurable timeout.

I've got bitten by this when using the AdminEmailHandler. My database went down, a lot of exceptions were generated and Django was trying to send emails. The SMTP server started not responding and the requests started blocking, until all my workers were used up. By then Django stopped serving requests. Basically, I was DoSed by my own SMTP server.

Change History (11)

comment:1 by André Cruz, 10 years ago

Easy pickings: set
Has patch: set

comment:2 by Claude Paroz, 10 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:3 by Susan Tan, 10 years ago

This looks like an interesting ticket! The PR is currently missing a test. I've merged @edevil's PR into a new one here + a test: https://github.com/django/django/pull/1758

In the current PR, the timeout is set to 60 seconds; it's hard-coded in. On the email backend side, should any action happen (such as closing the smtp email-backend connection) when that time limit of 60 seconds is reached?

Last edited 10 years ago by Susan Tan (previous) (diff)

comment:4 by Susan Tan, 10 years ago

Cc: susan.tan.fleckerl@… added

comment:5 by André Cruz, 10 years ago

Can't you use the global_setting in assertEqual(), so that the value isn't hardcoded?

As for the backend, currently send_messages() already skips close() when _send() throws an exception. But in the case of timeouts, smtplib automatically closes the underlying connection.

comment:6 by André Cruz, 10 years ago

Any news on this? There is a patch available with tests.

comment:7 by Tim Graham, 10 years ago

Patch needs improvement: set

The patch needs improvement per the comments on the pull request.

comment:8 by Claude Paroz <claude@…>, 10 years ago

In 3afde36d03e2b3b5ff5a265af39d8fb27afa8959:

Undelete the login() call inadvertantly removed in 4e0a2fe59c

Refs #21271.

comment:9 by Tim Graham, 10 years ago

Resolution: fixed
Status: newclosed

In 4e0a2fe59c8b9c32c2f3111474354356474128a8:

Fixed #21271 -- Added timeout parameter to SMTP EmailBackend.

Thanks Tobias McNulty and Tim Graham for discussions and code review.
Thanks Andre Cruz the suggestion and initial patch.

comment:10 by anonymous, 10 years ago

Any chance of this fix landing in 1.6?

comment:11 by Tim Graham, 10 years ago

No, now that 1.6 has been released, it'll only receive critical bug fixes. See https://docs.djangoproject.com/en/dev/internals/release-process/#supported-versions for details.

Note: See TracTickets for help on using tickets.
Back to Top