RFC822 Sender header cleanup. Bug 768468.

This patch does a few things:

- Removes any notion of a sender from the Composer, since there's no UI
  for it at the moment and it doesn't do anything when multiple
  addresses are available.
- Converts Geary.Email and the DB use the RFC822 model for the Sender and
  Reply-To headers, and translates between that and the IMAP model only
  once: When dealing with IMAP ENVELOPE.
- Fixes an incorrect assumption about the Sender in the SMTP code.

* src/client/composer/composer-widget.vala (ComposerWidget): Remove all
  mentions of a sender address. Make `from` prop private for setting.

* src/client/composer/email-entry.vala (EmailEntry::validate_addresses):
  Fix crash when address list is null.

* src/engine/api/geary-email.vala (Email, Email::set_originators):
  Convert sender prop/param from a list of RFC822 addresses to a single
  address. Fix call call sites.

* src/engine/imap-db/imap-db-message-addresses.vala (MessageAddresses):
  Convert sender prop from a list of RFC822 addresses to a single
  address. Fix call call sites.
  (MessageAddresses::get_address_from_string): New method for parsing
  single addresses from the DB.

* src/engine/imap-db/imap-db-message-row.vala
  (MessageRow::flatten_address, MessageRow::unflatten_address): New
  un/marshalling methods for single addresses.

* src/engine/imap/api/imap-folder.vala (Folder::fetched_data_to_email):
  Normalise sender and reply-to IMAP ENVELOPE responses to conform with
  RFC 822's expectations.

* src/engine/imap/message/imap-envelope.vala (Envelope): Label reply_to
  prop as being non-null, to conform with IMAP spec.

* src/engine/rfc822/rfc822-mailbox-address.vala (MailboxAddress): Added
  support ctor and methods for parsing and comparing single addresses.

* src/engine/rfc822/rfc822-message.vala (Message::from_composed_email):
  Set the from and sender props directly from the ComposedEmail, set the
  GMimeMessage From and Sender headers correctly based on these.
  (Message::stock_from_gmime): Correctly set the from and sender props
  from the GMimeMessage.

* src/engine/smtp/smtp-client-session.vala
  (ClientSession::send_email_async): Don't require the email sender to be
  non-null — it isn't used and isn't required.
This commit is contained in:
Michael James Gratton 2016-07-06 13:34:48 +10:00 committed by Michael Gratton
parent ec60022052
commit bbf5f073e6
10 changed files with 197 additions and 114 deletions

View file

@ -29,15 +29,13 @@ public class ComposerWidget : Gtk.EventBox {
private class FromAddressMap {
public Geary.Account account;
public Geary.RFC822.MailboxAddress? sender;
public Geary.RFC822.MailboxAddresses from;
public FromAddressMap(Geary.Account a, Geary.RFC822.MailboxAddresses f, Geary.RFC822.MailboxAddress? s = null) {
public FromAddressMap(Geary.Account a, Geary.RFC822.MailboxAddresses f) {
account = a;
from = f;
sender = s;
}
}
public const string ACTION_UNDO = "undo";
public const string ACTION_REDO = "redo";
public const string ACTION_CUT = "cut";
@ -140,11 +138,9 @@ public class ComposerWidget : Gtk.EventBox {
private delegate bool CompareStringFunc(string key, string token);
public Geary.Account account { get; private set; }
public Geary.RFC822.MailboxAddress sender { get; set; }
public Geary.RFC822.MailboxAddresses from { get; set; }
public Geary.RFC822.MailboxAddresses from { get; private set; }
public string to {
get { return to_entry.get_text(); }
set { to_entry.set_text(value); }
@ -906,8 +902,7 @@ public class ComposerWidget : Gtk.EventBox {
bool only_html = false) {
Geary.ComposedEmail email = new Geary.ComposedEmail(
date_override ?? new DateTime.now_local(), from);
email.sender = sender;
if (to_entry.addresses != null)
email.to = to_entry.addresses;
@ -2449,13 +2444,12 @@ public class ComposerWidget : Gtk.EventBox {
return false;
assert(from_list.size > index);
Geary.Account new_account = from_list.get(index).account;
from = from_list.get(index).from;
sender = from_list.get(index).sender;
if (new_account == account)
return false;
account = new_account;
set_entry_completions();

View file

@ -55,7 +55,7 @@ public class EmailEntry : Gtk.Entry {
}
private void validate_addresses() {
if (addresses.size == 0) {
if (addresses == null || addresses.size == 0) {
valid_or_empty = true;
empty = true;
return;

View file

@ -111,12 +111,12 @@ public class Geary.Email : BaseObject {
// DATE
public Geary.RFC822.Date? date { get; private set; default = null; }
// ORIGINATORS
public Geary.RFC822.MailboxAddresses? from { get; private set; default = null; }
public Geary.RFC822.MailboxAddresses? sender { get; private set; default = null; }
public Geary.RFC822.MailboxAddress? sender { get; private set; default = null; }
public Geary.RFC822.MailboxAddresses? reply_to { get; private set; default = null; }
// RECEIVERS
public Geary.RFC822.MailboxAddresses? to { get; private set; default = null; }
public Geary.RFC822.MailboxAddresses? cc { get; private set; default = null; }
@ -172,16 +172,27 @@ public class Geary.Email : BaseObject {
fields |= Field.DATE;
}
public void set_originators(Geary.RFC822.MailboxAddresses? from,
Geary.RFC822.MailboxAddresses? sender, Geary.RFC822.MailboxAddresses? reply_to) {
/**
* Sets the RFC822 originators for the message.
*
* RFC 2822 requires at least one From address, that the Sender
* and From not be identical, and that both From and ReplyTo are
* optional.
*/
public void set_originators(Geary.RFC822.MailboxAddresses from,
Geary.RFC822.MailboxAddress? sender,
Geary.RFC822.MailboxAddresses? reply_to)
throws RFC822Error {
// XXX Should be throwing an error here if from is empty or
// sender is same as from
this.from = from;
this.sender = sender;
this.reply_to = reply_to;
fields |= Field.ORIGINATORS;
}
public void set_receivers(Geary.RFC822.MailboxAddresses? to,
Geary.RFC822.MailboxAddresses? cc, Geary.RFC822.MailboxAddresses? bcc) {
this.to = to;
@ -329,16 +340,16 @@ public class Geary.Email : BaseObject {
public RFC822.MailboxAddress? get_primary_originator() {
if (from != null && from.size > 0)
return from[0];
if (sender != null && sender.size > 0)
return sender[0];
if (sender != null)
return sender;
if (reply_to != null && reply_to.size > 0)
return reply_to[0];
return null;
}
public string to_string() {
return "[%s] ".printf(id.to_string());
}

View file

@ -8,7 +8,7 @@ private class Geary.ImapDB.MessageAddresses : BaseObject {
// Read-only view.
public Gee.Collection<Contact> contacts { get; private set; }
private RFC822.MailboxAddresses? sender_addresses;
private RFC822.MailboxAddress? sender_address;
private RFC822.MailboxAddresses? from_addresses;
private RFC822.MailboxAddresses? to_addresses;
private RFC822.MailboxAddresses? cc_addresses;
@ -17,11 +17,11 @@ private class Geary.ImapDB.MessageAddresses : BaseObject {
private int from_importance;
private int to_importance;
private int cc_importance;
private MessageAddresses(string account_owner_email, RFC822.MailboxAddresses? sender_addresses,
private MessageAddresses(string account_owner_email, RFC822.MailboxAddress? sender_address,
RFC822.MailboxAddresses? from_addresses, RFC822.MailboxAddresses? to_addresses,
RFC822.MailboxAddresses? cc_addresses, RFC822.MailboxAddresses? bcc_addresses) {
this.sender_addresses = sender_addresses;
this.sender_address = sender_address;
this.from_addresses = from_addresses;
this.to_addresses = to_addresses;
this.cc_addresses = cc_addresses;
@ -33,7 +33,7 @@ private class Geary.ImapDB.MessageAddresses : BaseObject {
private MessageAddresses.from_strings(string account_owner_email, string? sender_field,
string? from_field, string? to_field, string? cc_field, string? bcc_field) {
this(account_owner_email, get_addresses_from_string(sender_field),
this(account_owner_email, get_address_from_string(sender_field),
get_addresses_from_string(from_field), get_addresses_from_string(to_field),
get_addresses_from_string(cc_field), get_addresses_from_string(bcc_field));
}
@ -60,15 +60,27 @@ private class Geary.ImapDB.MessageAddresses : BaseObject {
return null;
}
}
private static RFC822.MailboxAddress? get_address_from_string(string? field) {
RFC822.MailboxAddress? addr = null;
if (field != null) {
try {
new RFC822.MailboxAddress.from_rfc822_string(field);
} catch (RFC822Error e) {
// oh well
}
}
return addr;
}
private static RFC822.MailboxAddresses? get_addresses_from_string(string? field) {
return field == null ? null : new RFC822.MailboxAddresses.from_rfc822_string(field);
}
private void calculate_importance(string account_owner_email) {
// "Sender" is different than "from", but we give it the same importance.
bool account_owner_in_from =
(sender_addresses != null && sender_addresses.contains_normalized(account_owner_email)) ||
(sender_address != null && sender_address.equal_normalized(account_owner_email)) ||
(from_addresses != null && from_addresses.contains_normalized(account_owner_email));
bool account_owner_in_to = to_addresses != null &&
to_addresses.contains_normalized(account_owner_email);
@ -102,19 +114,21 @@ private class Geary.ImapDB.MessageAddresses : BaseObject {
cc_importance = int.max(cc_importance, ContactImportance.CC_CC);
}
}
private Gee.Collection<Contact> build_contacts() {
Gee.Map<string, Contact> contacts_map = new Gee.HashMap<string, Contact>();
add_contacts(contacts_map, sender_addresses, from_importance);
add_contacts(contacts_map, from_addresses, from_importance);
add_contacts(contacts_map, to_addresses, to_importance);
add_contacts(contacts_map, cc_addresses, cc_importance);
add_contacts(contacts_map, bcc_addresses, cc_importance);
if (this.sender_address != null) {
add_contact(contacts_map, this.sender_address, this.from_importance);
}
add_contacts(contacts_map, this.from_addresses, this.from_importance);
add_contacts(contacts_map, this.to_addresses, this.to_importance);
add_contacts(contacts_map, this.cc_addresses, this.cc_importance);
add_contacts(contacts_map, this.bcc_addresses, this.cc_importance);
return contacts_map.values;
}
private void add_contacts(Gee.Map<string, Contact> contacts_map, RFC822.MailboxAddresses? addresses,
int importance) {
if (addresses == null)

View file

@ -105,17 +105,19 @@ private class Geary.ImapDB.MessageRow {
if (fields.is_all_set(Geary.Email.Field.DATE))
email.set_send_date(!String.is_empty(date) ? new RFC822.Date(date) : null);
if (fields.is_all_set(Geary.Email.Field.ORIGINATORS)) {
email.set_originators(unflatten_addresses(from), unflatten_addresses(sender),
unflatten_addresses(reply_to));
email.set_originators(unflatten_addresses(from),
unflatten_address(sender),
unflatten_addresses(reply_to));
}
if (fields.is_all_set(Geary.Email.Field.RECEIVERS)) {
email.set_receivers(unflatten_addresses(to), unflatten_addresses(cc),
unflatten_addresses(bcc));
email.set_receivers(unflatten_addresses(to),
unflatten_addresses(cc),
unflatten_addresses(bcc));
}
if (fields.is_all_set(Geary.Email.Field.REFERENCES)) {
email.set_full_references(
(message_id != null) ? new RFC822.MessageID(message_id) : null,
@ -188,7 +190,7 @@ private class Geary.ImapDB.MessageRow {
if (email.fields.is_all_set(Geary.Email.Field.ORIGINATORS)) {
from = flatten_addresses(email.from);
sender = flatten_addresses(email.sender);
sender = flatten_address(email.sender);
reply_to = flatten_addresses(email.reply_to);
fields = fields.set(Geary.Email.Field.ORIGINATORS);
@ -250,7 +252,15 @@ private class Geary.ImapDB.MessageRow {
fields = fields.set(Geary.Email.Field.PROPERTIES);
}
}
private static string? flatten_address(RFC822.MailboxAddress? addr) {
string? flat = null;
if (addr != null) {
flat = addr.to_rfc822_string();
}
return flat;
}
private static string? flatten_addresses(RFC822.MailboxAddresses? addrs) {
if (addrs == null)
return null;
@ -274,7 +284,19 @@ private class Geary.ImapDB.MessageRow {
return builder.str;
}
}
private RFC822.MailboxAddress? unflatten_address(string? str) {
RFC822.MailboxAddress? addr = null;
if (str != null) {
try {
addr = new RFC822.MailboxAddress.from_rfc822_string(str);
} catch (RFC822Error e) {
// oh well
}
}
return addr;
}
private RFC822.MailboxAddresses? unflatten_addresses(string? str) {
return String.is_empty(str) ? null : new RFC822.MailboxAddresses.from_rfc822_string(str);
}

View file

@ -848,12 +848,16 @@ private class Geary.Imap.Folder : BaseObject {
switch (data_type) {
case FetchDataSpecifier.ENVELOPE:
Envelope envelope = (Envelope) data;
email.set_send_date(envelope.sent);
email.set_message_subject(envelope.subject);
email.set_originators(envelope.from, envelope.sender, envelope.reply_to);
email.set_originators(
envelope.from,
envelope.sender.equal_to(envelope.from) ? null : envelope.sender[0],
envelope.reply_to.equal_to(envelope.from) ? null : envelope.reply_to
);
email.set_receivers(envelope.to, envelope.cc, envelope.bcc);
// store these to add to References all at once
message_id = envelope.message_id;
in_reply_to = envelope.in_reply_to;
@ -915,20 +919,20 @@ private class Geary.Imap.Folder : BaseObject {
string? value = headers.get_header("From");
if (!String.is_empty(value))
from = new RFC822.MailboxAddresses.from_rfc822_string(value);
RFC822.MailboxAddresses? sender = null;
RFC822.MailboxAddress? sender = null;
value = headers.get_header("Sender");
if (!String.is_empty(value))
sender = new RFC822.MailboxAddresses.from_rfc822_string(value);
sender = new RFC822.MailboxAddress.from_rfc822_string(value);
RFC822.MailboxAddresses? reply_to = null;
value = headers.get_header("Reply-To");
if (!String.is_empty(value))
reply_to = new RFC822.MailboxAddresses.from_rfc822_string(value);
email.set_originators(from, sender, reply_to);
}
// RECEIVERS
if (required_but_not_set(Geary.Email.Field.RECEIVERS, required_fields, email)) {
RFC822.MailboxAddresses? to = null;

View file

@ -15,16 +15,16 @@ public class Geary.Imap.Envelope : Geary.MessageData.AbstractMessageData, Geary.
public Geary.RFC822.Subject subject { get; private set; }
public Geary.RFC822.MailboxAddresses from { get; private set; }
public Geary.RFC822.MailboxAddresses sender { get; private set; }
public Geary.RFC822.MailboxAddresses? reply_to { get; private set; }
public Geary.RFC822.MailboxAddresses reply_to { get; private set; }
public Geary.RFC822.MailboxAddresses? to { get; private set; }
public Geary.RFC822.MailboxAddresses? cc { get; private set; }
public Geary.RFC822.MailboxAddresses? bcc { get; private set; }
public Geary.RFC822.MessageIDList? in_reply_to { get; private set; }
public Geary.RFC822.MessageID? message_id { get; private set; }
public Envelope(Geary.RFC822.Date? sent, Geary.RFC822.Subject subject,
Geary.RFC822.MailboxAddresses from, Geary.RFC822.MailboxAddresses sender,
Geary.RFC822.MailboxAddresses? reply_to, Geary.RFC822.MailboxAddresses? to,
Geary.RFC822.MailboxAddresses reply_to, Geary.RFC822.MailboxAddresses? to,
Geary.RFC822.MailboxAddresses? cc, Geary.RFC822.MailboxAddresses? bcc,
Geary.RFC822.MessageIDList? in_reply_to, Geary.RFC822.MessageID? message_id) {
this.sent = sent;
@ -38,10 +38,9 @@ public class Geary.Imap.Envelope : Geary.MessageData.AbstractMessageData, Geary.
this.in_reply_to = in_reply_to;
this.message_id = message_id;
}
public override string to_string() {
return "[%s] %s: \"%s\"".printf((sent != null) ? sent.to_string() : "(no date)",
from.to_string(), subject.to_string());
}
}

View file

@ -72,6 +72,25 @@ public class Geary.RFC822.MailboxAddress : Geary.MessageData.SearchableMessageDa
address = "%s@%s".printf(mailbox, domain);
}
public MailboxAddress.from_rfc822_string(string rfc822) throws RFC822Error {
InternetAddressList addrlist = InternetAddressList.parse_string(rfc822);
if (addrlist == null)
return;
int length = addrlist.length();
for (int ctr = 0; ctr < length; ctr++) {
InternetAddress? addr = addrlist.get_address(ctr);
// TODO: Handle group lists
InternetAddressMailbox? mbox_addr = addr as InternetAddressMailbox;
if (mbox_addr != null) {
this(mbox_addr.get_name(), mbox_addr.get_addr());
return;
}
}
throw new RFC822Error.INVALID("Could not parse RFC822 address: %s", rfc822);
}
// Borrowed liberally from GMime's internal _internet_address_decode_name() function.
private static string decode_name(string name) {
// see if a broken mailer has sent raw 8-bit information
@ -181,7 +200,11 @@ public class Geary.RFC822.MailboxAddress : Geary.MessageData.SearchableMessageDa
public bool equal_to(MailboxAddress other) {
return this != other ? String.stri_equal(address, other.address) : true;
}
public bool equal_normalized(string address) {
return this.address.normalize().casefold() == address.normalize().casefold();
}
public string to_string() {
return get_full_address();
}

View file

@ -17,7 +17,8 @@ public class Geary.RFC822.Message : BaseObject {
Mime.ContentDisposition? disposition, string? content_id, Geary.Memory.Buffer buffer);
private const string DEFAULT_ENCODING = "UTF8";
private const string HEADER_SENDER = "Sender";
private const string HEADER_IN_REPLY_TO = "In-Reply-To";
private const string HEADER_REFERENCES = "References";
private const string HEADER_MAILER = "X-Mailer";
@ -85,61 +86,70 @@ public class Geary.RFC822.Message : BaseObject {
}
public Message.from_composed_email(Geary.ComposedEmail email, string? message_id) {
message = new GMime.Message(true);
this.message = new GMime.Message(true);
// Required headers
assert(email.from.size > 0);
sender = email.from[0];
date = new RFC822.Date.from_date_time(email.date);
message.set_sender(sender.to_rfc822_string());
message.set_date_as_string(date.serialize());
this.sender = email.sender;
this.from = email.from;
this.date = new RFC822.Date.from_date_time(email.date);
// GMimeMessage.set_sender actually sets the From header - and
// although the API docs make it sound otherwise, it also
// supports a list of addresses
message.set_sender(this.from.to_rfc822_string());
message.set_date_as_string(this.date.serialize());
if (message_id != null)
message.set_message_id(message_id);
// Optional headers
if (email.to != null) {
to = email.to;
this.to = email.to;
foreach (RFC822.MailboxAddress mailbox in email.to)
message.add_recipient(GMime.RecipientType.TO, mailbox.name, mailbox.address);
this.message.add_recipient(GMime.RecipientType.TO, mailbox.name, mailbox.address);
}
if (email.cc != null) {
cc = email.cc;
this.cc = email.cc;
foreach (RFC822.MailboxAddress mailbox in email.cc)
message.add_recipient(GMime.RecipientType.CC, mailbox.name, mailbox.address);
this.message.add_recipient(GMime.RecipientType.CC, mailbox.name, mailbox.address);
}
if (email.bcc != null) {
bcc = email.bcc;
this.bcc = email.bcc;
foreach (RFC822.MailboxAddress mailbox in email.bcc)
message.add_recipient(GMime.RecipientType.BCC, mailbox.name, mailbox.address);
this.message.add_recipient(GMime.RecipientType.BCC, mailbox.name, mailbox.address);
}
if (email.sender != null) {
this.sender = email.sender;
this.message.set_header(HEADER_SENDER, email.sender.to_rfc822_string());
}
if (email.reply_to != null) {
reply_to = email.reply_to;
message.set_reply_to(email.reply_to.to_rfc822_string());
this.reply_to = email.reply_to;
this.message.set_reply_to(email.reply_to.to_rfc822_string());
}
if (email.in_reply_to != null) {
in_reply_to = new Geary.RFC822.MessageIDList.from_rfc822_string(email.in_reply_to);
message.set_header(HEADER_IN_REPLY_TO, email.in_reply_to);
this.in_reply_to = new Geary.RFC822.MessageIDList.from_rfc822_string(email.in_reply_to);
this.message.set_header(HEADER_IN_REPLY_TO, email.in_reply_to);
}
if (email.references != null) {
references = new Geary.RFC822.MessageIDList.from_rfc822_string(email.references);
message.set_header(HEADER_REFERENCES, email.references);
this.references = new Geary.RFC822.MessageIDList.from_rfc822_string(email.references);
this.message.set_header(HEADER_REFERENCES, email.references);
}
if (email.subject != null) {
subject = new Geary.RFC822.Subject(email.subject);
message.set_subject(email.subject);
this.subject = new Geary.RFC822.Subject(email.subject);
this.message.set_subject(email.subject);
}
// User-Agent
if (!Geary.String.is_empty(email.mailer)) {
mailer = email.mailer;
message.set_header(HEADER_MAILER, email.mailer);
this.mailer = email.mailer;
this.message.set_header(HEADER_MAILER, email.mailer);
}
// Body: text format (optional)
@ -201,11 +211,11 @@ public class Geary.RFC822.Message : BaseObject {
GMime.Object? attachment_part = coalesce_parts(attachment_parts, "mixed");
if (attachment_part != null)
main_parts.add(attachment_part);
GMime.Object? main_part = coalesce_parts(main_parts, "mixed");
message.set_mime_part(main_part);
this.message.set_mime_part(main_part);
}
// Makes a copy of the given message without the BCC fields. This is used for sending the email
// without sending the BCC headers to all recipients.
public Message.without_bcc(Message email) {
@ -296,7 +306,7 @@ public class Geary.RFC822.Message : BaseObject {
email.set_message_header(new Geary.RFC822.Header(new Geary.Memory.StringBuffer(
message.get_headers())));
email.set_send_date(date);
email.set_originators(from, new Geary.RFC822.MailboxAddresses.single(sender), reply_to);
email.set_originators(from, sender, reply_to);
email.set_receivers(to, cc, bcc);
email.set_full_references(null, in_reply_to, references);
email.set_message_subject(subject);
@ -326,13 +336,25 @@ public class Geary.RFC822.Message : BaseObject {
}
private void stock_from_gmime() {
// GMime calls the From address the "sender"
string? message_sender = message.get_sender();
if (message_sender != null) {
from = new RFC822.MailboxAddresses.from_rfc822_string(message_sender);
// sender is defined as first From address, from better or worse
sender = (from.size != 0) ? from[0] : null;
this.from = new RFC822.MailboxAddresses.from_rfc822_string(message_sender);
}
// And it doesn't provide a convenience method for Sender header
if (!String.is_empty(message.get_header(HEADER_SENDER))) {
string sender = GMime.utils_header_decode_text(message.get_header(HEADER_SENDER));
try {
this.sender = new RFC822.MailboxAddress.from_rfc822_string(sender);
} catch (RFC822Error e) {
debug("Invalid RDC822 Sender address: %s", sender);
}
}
if (!String.is_empty(message.get_reply_to()))
this.reply_to = new RFC822.MailboxAddresses.from_rfc822_string(message.get_reply_to());
Gee.List<RFC822.MailboxAddress>? converted = convert_gmime_address_list(
message.get_recipients(GMime.RecipientType.TO));
if (converted != null && converted.size > 0)
@ -346,9 +368,6 @@ public class Geary.RFC822.Message : BaseObject {
if (converted != null && converted.size > 0)
bcc = new RFC822.MailboxAddresses(converted);
if (!String.is_empty(message.get_reply_to()))
reply_to = new RFC822.MailboxAddresses.from_rfc822_string(message.get_reply_to());
if (!String.is_empty(message.get_header(HEADER_IN_REPLY_TO)))
in_reply_to = new RFC822.MessageIDList.from_rfc822_string(message.get_header(HEADER_IN_REPLY_TO));

View file

@ -146,11 +146,8 @@ public class Geary.Smtp.ClientSession {
rset_required = false;
}
// MAIL
if (email.sender == null)
throw new SmtpError.REQUIRED_FIELD("No sender in message");
MailRequest mail_request = new MailRequest(from);
Response response = yield cx.transaction_async(mail_request, cancellable);
if (!response.code.is_success_completed())