Opened 4 years ago

Closed 4 years ago

#6524 release blocker: regression closed fixed (fixed)

twisted.mail.smtp.ESMTPClient tries to STARTTLS multiple times

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: highest Milestone:
Component: mail Keywords:
Cc: ashfall Branch: branches/judicious-smtp-starttls-6524
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

If ESMTPClient is used to connect to an SMTP server that offers the STARTTLS feature, it will try to use it. If the SMTP server still offers the STARTTLS after the command has been used once to start TLS, ESMTPClient will try to use it again. This is useless at best, and in practice probably always triggers an error from the server and prevents mail from being sent.

This is a regression introduced by #3989.

Attachments (1)

mailish.py (1.2 KB) - added by Jean-Paul Calderone 4 years ago.
It does not work.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by Jean-Paul Calderone

Here's a transcript demonstrating the consequences of this bug:

<<< 220 email-smtp.amazonaws.com ESMTP SimpleEmailService-376766033
>>> EHLO top
<<< 250-email-smtp.amazonaws.com
<<< 250-8BITMIME
<<< 250-SIZE 10485760
<<< 250-STARTTLS
<<< 250-AUTH PLAIN LOGIN
<<< 250 Ok
>>> STARTTLS
<<< 220 Ready to start TLS
>>> EHLO top
<<< 250-email-smtp.amazonaws.com
<<< 250-8BITMIME
<<< 250-SIZE 10485760
<<< 250-STARTTLS
<<< 250-AUTH PLAIN LOGIN
<<< 250 Ok
>>> STARTTLS
<<< 454 TLS not available due to temporary reason: TLS already active
>>> QUIT

And attached is a script that reproduces it.

Changed 4 years ago by Jean-Paul Calderone

Attachment: mailish.py added

It does not work.

comment:2 Changed 4 years ago by ashfall

Cc: ashfall added

comment:3 Changed 4 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

comment:4 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/judicious-smtp-starttls-6524

(In [38818]) Branching to 'judicious-smtp-starttls-6524'

comment:5 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted
Status: assignednew

Relatively straightforward; only wished I had schematic tables once during the entire development process (arguably the minimum number of times it is possible to wish for schematic tables when writing a piece of software).

comment:6 Changed 4 years ago by radix

Owner: set to radix
Status: newassigned

comment:7 Changed 4 years ago by radix

Keywords: review removed
Owner: changed from radix to Jean-Paul Calderone
Status: assignednew

Schematic tables would be nice. Maybe we should sprint on them some time.

[1] You have a "verify that" in a docstring in a test module, but I guess that's ok because it's not a test_ method itself, just a utility assertion method?

No flakes, tests pass, +1, please merge.

comment:8 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [38833]) Merge judicious-smtp-starttls-6524

Author: exarkun Reviewer: radix Fixes: #6524

Avoid having twisted.mail.smtp.ESMTPClient try to use the STARTTLS command if the server offers it but the connection is already using TLS.

Note: See TracTickets for help on using tickets.