Opened 10 years ago

Last modified 10 years ago

#735 defect closed fixed (fixed)

SMTP Client upgrade and bug fixes

Reported by: bkirsch Owned by:
Priority: highest Milestone:
Component: Keywords:
Cc: exarkun, itamarst, bkirsch Branch:
Author: Launchpad Bug:

Description


Attachments (5)

smtp.diff (22.9 KB) - added by bkirsch 10 years ago.
smtp.2.diff (22.7 KB) - added by bkirsch 10 years ago.
smtp.3.diff (22.7 KB) - added by bkirsch 10 years ago.
smtp.4.diff (22.4 KB) - added by bkirsch 10 years ago.
smtp.py (59.3 KB) - added by bkirsch 10 years ago.

Download all attachments as: .zip

Change History (13)

Changed 10 years ago by bkirsch

comment:1 Changed 10 years ago by bkirsch

Here is the updated and hopefully final smtp.py patch for the near future. It fixes and adds additional 
error handling and error reporting functionality, better retry support, and other bug fixes (missing 
import).

comment:2 Changed 10 years ago by exarkun

Thanks for supplying the patch in unified diff format.  A few things:

  At least one (on a cursor skim) change made by this patch is a reversion of
unrelated functionality recently added in SVN.  I think this is a mistake. 
Could you supply a patch with only your changes?  Similar to this, part of the
patch does not apply cleanly.  I'm not sure what these two lines represent:

  -        self.authenticators = {}
  +        self.authenticators = []

  Next, the change to make the client only log if __debug__ is True isn't going
to work.  It may be a crummy way to do error reporting, but at least two
projects I know of rely on it.  A parameter to allow it to be disabled would be
alright.

  Finally, the documentation throughout is appreciated.  The preferred format in
Twisted is to use comments instead of string literals, though, unless the string
literal will become the __doc__ attribute of an object.  I can make the switch
in this patch, but keep it in mind for future submissions.

  Thanks

Changed 10 years ago by bkirsch

comment:3 Changed 10 years ago by bkirsch

This diff is a result of diff -u smtp.py smtp.final > smtp.diff

The smtp.py is the latest version pulled from SVN.

Changes:
1. Altered comment lines from """ """ to #
2. Cleaned up code slightly
3. Tested patch (it works fine)
4. Removed __debug__ and added a self.debug flag which defaults to True. The flag 
    indicates whether to log the SMTP client / server communication.
 
Response to JP:
-----------------
I'm not sure what these two lines represent:

  -        self.authenticators = {}
  +        self.authenticators = []


The self.authenticators contains all the registered Authenticators the ESMTPClient supports. I changed it 
from a dict to a list so that the order the Authenticators are register in is the order the Client tries for
Authentication. When it was a dict the order the server returned for Auth dictated the order the client 
tried.

if the server sent AUTH PLAIN LOGIN MD-5, the client would use PLAIN to AUTH. But the 
desired behavior should be to use the most secure (MD-5) Auth available on the server.

Changed 10 years ago by bkirsch

comment:4 Changed 10 years ago by bkirsch

Minor addition to errorback the correct exception instead of a failure instance.

Use the smtp.diff submitted today 10-7-2004 to patch smtp.py. The diff contains all
the  cumulative changes from previous diffs.

Changed 10 years ago by bkirsch

comment:5 Changed 10 years ago by bkirsch

This patch incorporates updates made to smtp.py between r11303 and the current version as well as 
my changes to make the client less buggy and more robust.

Changed 10 years ago by bkirsch

comment:6 Changed 10 years ago by bkirsch

This is the smtp.py that results when the previous patch is applied

comment:7 Changed 10 years ago by exarkun

Committed at revision 11985

comment:8 Changed 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.