Ticket #6524 regression closed fixed

Opened 11 months ago

Last modified 10 months ago

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

Reported by: exarkun Owned by: exarkun
Priority: highest Milestone:
Component: mail Keywords:
Cc: ashfall@… Branch: branches/judicious-smtp-starttls-6524
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

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

mailish.py Download (1.2 KB) - added by exarkun 11 months ago.
It does not work.

Change History

1

Changed 11 months ago by exarkun

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 11 months ago by exarkun

It does not work.

2

Changed 10 months ago by ashfall

  • cc ashfall@… added

3

Changed 10 months ago by exarkun

  • owner set to exarkun
  • status changed from new to assigned

4

Changed 10 months ago by exarkun

  • branch set to branches/judicious-smtp-starttls-6524
  • branch_author set to exarkun

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

5

Changed 10 months ago by exarkun

  • owner exarkun deleted
  • status changed from assigned to new
  • keywords review added

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).

6

Changed 10 months ago by radix

  • owner set to radix
  • status changed from new to assigned

7

Changed 10 months ago by radix

  • keywords review removed
  • owner changed from radix to exarkun
  • status changed from assigned to new

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.

8

Changed 10 months ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(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.