Commit 6c8f1921 authored by Michael Gratton's avatar Michael Gratton 🤞 Committed by Michael Gratton

Attempt to de-mangle From names from Mailman, GitLab, etc

Some software like the above will mangle From mailbox names by appending
"via Some Service" to the From mailbox name. This messes up generating
default avatars for the actual people sending these messages, so
attempt to de-mangle the names.

This involves moving primary originator determination from the engine
to the client, since it's now a policy thing. Add unit tests.
parent 133167df
Pipeline #66523 passed with stages
in 38 minutes and 28 seconds
......@@ -289,6 +289,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
/** Determines if the email is a draft message. */
public bool is_draft { get; private set; }
/** The email's primary originator, if any. */
public Geary.RFC822.MailboxAddress? primary_originator { get; private set; }
/** The view displaying the email's primary message headers and body. */
public ConversationMessage primary_message { get; private set; }
......@@ -444,6 +447,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
base_ref();
this.email = email;
this.is_draft = is_draft;
this.primary_originator = Util.Email.get_primary_originator(email);
this.email_store = email_store;
this.contact_store = email_store.account.get_contact_store();
this.avatar_store = avatar_store;
......@@ -507,9 +511,11 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
// Construct the view for the primary message, hook into it
bool load_images = email.load_remote_images().is_certain();
Geary.Contact contact = this.contact_store.get_by_rfc822(
email.get_primary_originator()
);
Geary.Contact? contact = null;
if (this.primary_originator != null) {
contact = this.contact_store.get_by_rfc822(this.primary_originator);
}
if (contact != null) {
load_images |= contact.always_load_remote_images();
}
......@@ -577,7 +583,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
} catch (IOError.CANCELLED err) {
// okay
} catch (Error err) {
Geary.RFC822.MailboxAddress? from = this.email.get_primary_originator();
Geary.RFC822.MailboxAddress? from = this.primary_originator;
debug("Avatar load failed for \"%s\": %s",
from != null ? from.to_string() : "<unknown>", err.message);
}
......@@ -1038,7 +1044,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
private void on_remember_remote_images(ConversationMessage view) {
Geary.RFC822.MailboxAddress? sender = this.email.get_primary_originator();
Geary.RFC822.MailboxAddress? sender = this.primary_originator;
if (sender != null) {
Geary.Contact? contact = this.contact_store.get_by_rfc822(sender);
if (contact != null) {
......
......@@ -289,7 +289,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
bool load_remote_images,
Configuration config) {
this(
email.get_primary_originator(),
Util.Email.get_primary_originator(email),
email.from,
email.reply_to,
email.sender,
......@@ -315,7 +315,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
bool load_remote_images,
Configuration config) {
this(
message.get_primary_originator(),
Util.Email.get_primary_originator(message),
message.from,
message.reply_to,
message.sender,
......
......@@ -95,7 +95,8 @@ public class Libnotify : Geary.BaseObject {
return;
// possible to receive email with no originator
Geary.RFC822.MailboxAddress? primary = email.get_primary_originator();
Geary.RFC822.MailboxAddress? primary =
Util.Email.get_primary_originator(email);
if (primary == null) {
notify_new_mail(folder, 1);
......
......@@ -38,6 +38,65 @@ namespace Util.Email {
return !Geary.String.is_empty(cleaned) ? cleaned : _("(no subject)");
}
/**
* Returns a mailbox for the primary originator of an email.
*
* RFC 822 allows multiple and absent From header values, and
* software such as Mailman and GitLab will mangle the names in
* From mailboxes. This provides a canonical means to obtain a
* mailbox (that is, name and email address) for the first
* originator, and with the mailbox's name having been fixed up
* where possible.
*
* The first From mailbox is used and de-mangled if found, if not
* the Sender mailbox is used if present, else the first Reply-To
* mailbox is used.
*/
public Geary.RFC822.MailboxAddress?
get_primary_originator(Geary.EmailHeaderSet email) {
Geary.RFC822.MailboxAddress? primary = null;
if (email.from != null && email.from.size > 0) {
// We have a From address, so attempt to de-mangle it
Geary.RFC822.MailboxAddresses? from = email.from;
string from_name = "";
if (from != null && from.size > 0) {
primary = from[0];
from_name = primary.name ?? "";
}
Geary.RFC822.MailboxAddresses? reply_to = email.reply_to;
Geary.RFC822.MailboxAddress? primary_reply_to = null;
string reply_to_name = "";
if (reply_to != null && reply_to.size > 0) {
primary_reply_to = reply_to[0];
reply_to_name = primary_reply_to.name ?? "";
}
// Spaces are important
const string VIA = " via ";
if (reply_to_name != "" && from_name.has_prefix(reply_to_name)) {
// Mailman sometimes sends the true originator as the
// Reply-To for the email
primary = primary_reply_to;
} else if (VIA in from_name) {
// Mailman, GitLib, Discourse and others send the
// originator's name prefixing something starting with
// "via".
primary = new Geary.RFC822.MailboxAddress(
from_name.split(VIA, 2)[0], primary.address
);
}
} else if (email.sender != null) {
primary = email.sender;
} else if (email.reply_to != null && email.reply_to.size > 0) {
primary = email.reply_to[0];
}
return primary;
}
/**
* Returns a shortened recipient list suitable for display.
*
......
......@@ -530,25 +530,6 @@ public class Geary.Email : BaseObject, EmailHeaderSet {
return (preview != null) ? preview.buffer.to_string() : "";
}
/**
* Returns the primary originator of an email, which is defined as the first mailbox address
* in From:, Sender:, or Reply-To:, in that order, depending on availability.
*
* Returns null if no originators are present.
*/
public RFC822.MailboxAddress? get_primary_originator() {
if (from != null && from.size > 0)
return from[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());
}
......
......@@ -441,25 +441,6 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
: "";
}
/**
* Returns the primary originator of an email, which is defined as the first mailbox address
* in From:, Sender:, or Reply-To:, in that order, depending on availability.
*
* Returns null if no originators are present.
*/
public RFC822.MailboxAddress? get_primary_originator() {
if (from != null && from.size > 0)
return from[0];
if (sender != null)
return sender;
if (reply_to != null && reply_to.size > 0)
return reply_to[0];
return null;
}
public Gee.List<RFC822.MailboxAddress>? get_recipients() {
Gee.List<RFC822.MailboxAddress> addrs = new Gee.ArrayList<RFC822.MailboxAddress>();
......
/*
* Copyright 2019 Michael Gratton <mike@vee.net>
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
public class Util.Email.Test : TestCase {
public Test() {
base("UtilEmailTest");
add_test("null_originator", null_originator);
add_test("from_originator", from_originator);
add_test("sender_originator", sender_originator);
add_test("reply_to_originator", reply_to_originator);
add_test("reply_to_via_originator", reply_to_via_originator);
add_test("plain_via_originator", plain_via_originator);
}
public void null_originator() throws GLib.Error {
Geary.RFC822.MailboxAddress? originator = get_primary_originator(
new_email(null, null, null)
);
assert_null(originator);
}
public void from_originator() throws GLib.Error {
Geary.RFC822.MailboxAddress? originator = get_primary_originator(
new_email(
new Geary.RFC822.MailboxAddress("from", "from@example.com"),
new Geary.RFC822.MailboxAddress("sender", "sender@example.com"),
new Geary.RFC822.MailboxAddress("reply-to", "reply-to@example.com")
)
);
assert_non_null(originator);
assert_string("from", originator.name);
assert_string("from@example.com", originator.address);
}
public void sender_originator() throws GLib.Error {
Geary.RFC822.MailboxAddress? originator = get_primary_originator(
new_email(
null,
new Geary.RFC822.MailboxAddress("sender", "sender@example.com"),
new Geary.RFC822.MailboxAddress("reply-to", "reply-to@example.com")
)
);
assert_non_null(originator);
assert_string("sender", originator.name);
assert_string("sender@example.com", originator.address);
}
public void reply_to_originator() throws GLib.Error {
Geary.RFC822.MailboxAddress? originator = get_primary_originator(
new_email(
null,
null,
new Geary.RFC822.MailboxAddress("reply-to", "reply-to@example.com")
)
);
assert_non_null(originator);
assert_string("reply-to", originator.name);
assert_string("reply-to@example.com", originator.address);
}
public void reply_to_via_originator() throws GLib.Error {
Geary.RFC822.MailboxAddress? originator = get_primary_originator(
new_email(
new Geary.RFC822.MailboxAddress("test via bot", "bot@example.com"),
null,
new Geary.RFC822.MailboxAddress("test", "test@example.com")
)
);
assert_non_null(originator);
assert_string("test", originator.name);
assert_string("test@example.com", originator.address);
}
public void plain_via_originator() throws GLib.Error {
Geary.RFC822.MailboxAddress? originator = get_primary_originator(
new_email(
new Geary.RFC822.MailboxAddress("test via bot", "bot@example.com"),
null,
null
)
);
assert_non_null(originator);
assert_string("test", originator.name);
assert_string("bot@example.com", originator.address);
}
private Geary.Email new_email(Geary.RFC822.MailboxAddress? from,
Geary.RFC822.MailboxAddress? sender,
Geary.RFC822.MailboxAddress? reply_to)
throws GLib.Error {
Geary.Email email = new Geary.Email(new Geary.MockEmailIdentifer(1));
email.set_originators(
from != null
? new Geary.RFC822.MailboxAddresses(Geary.Collection.single(from))
: null,
sender,
reply_to != null
? new Geary.RFC822.MailboxAddresses(Geary.Collection.single(reply_to))
: null
);
return email;
}
}
......@@ -68,6 +68,7 @@ geary_test_client_sources = [
# and the engine test sute needs to depend
# geary-engine_internal.vapi, which leads to duplicate symbols when
# linking
'engine/api/geary-email-identifier-mock.vala',
'engine/api/geary-credentials-mediator-mock.vala',
'client/accounts/accounts-manager-test.vala',
......@@ -76,6 +77,7 @@ geary_test_client_sources = [
'client/components/client-web-view-test-case.vala',
'client/composer/composer-web-view-test.vala',
'client/util/util-avatar-test.vala',
'client/util/util-email-test.vala',
'js/client-page-state-test.vala',
'js/composer-page-state-test.vala',
......
......@@ -44,6 +44,7 @@ int main(string[] args) {
client.add_suite(new ComposerWebViewTest().get_suite());
client.add_suite(new ConfigurationTest().get_suite());
client.add_suite(new Util.Avatar.Test().get_suite());
client.add_suite(new Util.Email.Test().get_suite());
TestSuite js = new TestSuite("js");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment