#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 , 11 years ago
| Easy pickings: | set |
|---|---|
| Has patch: | set |
comment:2 by , 11 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Bug → Cleanup/optimization |
comment:3 by , 11 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?
comment:4 by , 11 years ago
| Cc: | added |
|---|
comment:5 by , 11 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:7 by , 11 years ago
| Patch needs improvement: | set |
|---|
The patch needs improvement per the comments on the pull request.
comment:9 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
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:11 by , 11 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.
Sent pull request https://github.com/django/django/pull/1752.