Opened 6 years ago

Closed 6 years ago

#11370 closed defect (fixed)

Compose messages using email package instead of MimeWriter

Reported by: sysrqb Owned by: isis
Priority: Medium Milestone:
Component: Circumvention/BridgeDB Version:
Severity: Keywords: bridgedb-email, bridgedb-0.1.7
Cc: isis, sysrqb Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Deprecated since version 2.3: The email package should be used in preference to the MimeWriter module. This module is present only to maintain backward compatibility.

We don't need to continue using a deprecated package. We might as well move on.

Child Tickets

Change History (4)

comment:1 Changed 6 years ago by sysrqb

Status: newneeds_review

comment:2 Changed 6 years ago by sysrqb

I should note that this commit seems simple, but I don't know if it's actually correct. I thought that there would be a single method that would give us the message headers and body but I couldn't find a way to do it, which is why the headers are joined together and then the body is written.

comment:3 Changed 6 years ago by isis

Cc: sysrqb added
Keywords: bridgedb-email bridgedb-0.1.7 added
Owner: set to isis
Status: needs_reviewassigned

I reviewed and tested these. For the record, there is a twisted.mail.smtp.rfc822.Message class which works similarly, though I did not see any real benefits to switching to it, and the amount of refactoring required to do so is greater.

Because we don't have the best of tests for the email servers, here's what I manually tested:

  • A correctly formed email requesting bridges gets bridge lines in response.
  • A correctly formed email requesting obfs3 bridges gets obfs bridge lines.
  • Too many emails in the allotted time period generates a warning email.
  • The warning email is still sent even if there was a SIGHUP between the original request and the one thereafter which occurred too soon.
  • Any further requests after the warning email are dropped on the floor.
  • Emails which are not correctly formatted (i.e. missing certain required headers) are dropped on the floor.

Seems good to me. I'll merge this and #5232 for 0.1.7.

Thanks, sysrqb!

comment:4 Changed 6 years ago by isis

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.