[geary/mjog/rfc822-cleanup: 5/21] Geary.RFC822: Ensure various data constructors throw errors
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/mjog/rfc822-cleanup: 5/21] Geary.RFC822: Ensure various data constructors throw errors
- Date: Wed, 6 May 2020 04:52:23 +0000 (UTC)
commit c6bb391f4bc08268a77a15525eb53b1e0958af12
Author: Michael Gratton <mike vee net>
Date: Tue May 5 21:48:11 2020 +1000
Geary.RFC822: Ensure various data constructors throw errors
Rather than just silently ignoring error conditions, throw RFC822
errors instead.
src/client/accounts/accounts-manager.vala | 13 +-
src/client/composer/composer-email-entry.vala | 11 +-
src/engine/imap-db/imap-db-message-row.vala | 28 +++--
src/engine/imap/message/imap-message-data.vala | 10 +-
src/engine/rfc822/rfc822-mailbox-address.vala | 34 +++---
src/engine/rfc822/rfc822-mailbox-addresses.vala | 13 +-
src/engine/rfc822/rfc822-message-data.vala | 60 +++++-----
src/engine/rfc822/rfc822-message.vala | 153 ++++++++++++------------
test/engine/rfc822-message-test.vala | 3 +-
test/integration/smtp/client-session.vala | 3 +-
10 files changed, 172 insertions(+), 156 deletions(-)
---
diff --git a/src/client/accounts/accounts-manager.vala b/src/client/accounts/accounts-manager.vala
index f6d5749f5..7b7c101ca 100644
--- a/src/client/accounts/accounts-manager.vala
+++ b/src/client/accounts/accounts-manager.vala
@@ -1274,10 +1274,15 @@ public class Accounts.AccountConfigLegacy : AccountConfig, GLib.Object {
ALTERNATE_EMAILS_KEY
);
foreach (string alt_email in alt_email_list) {
- Geary.RFC822.MailboxAddresses mailboxes =
- new Geary.RFC822.MailboxAddresses.from_rfc822_string(alt_email);
- foreach (Geary.RFC822.MailboxAddress mailbox in mailboxes.get_all()) {
- info.append_sender(mailbox);
+ try {
+ var mailboxes = new Geary.RFC822.MailboxAddresses.from_rfc822_string(alt_email);
+ foreach (Geary.RFC822.MailboxAddress mailbox in mailboxes.get_all()) {
+ info.append_sender(mailbox);
+ }
+ } catch (Geary.RFC822Error error) {
+ throw new ConfigError.SYNTAX(
+ "Invalid alternate email: %s", error.message
+ );
}
}
diff --git a/src/client/composer/composer-email-entry.vala b/src/client/composer/composer-email-entry.vala
index b5e87314c..cd4202b0b 100644
--- a/src/client/composer/composer-email-entry.vala
+++ b/src/client/composer/composer-email-entry.vala
@@ -81,9 +81,14 @@ public class Composer.EmailEntry : Gtk.Entry {
this._addresses = new Geary.RFC822.MailboxAddresses();
this.is_valid = false;
} else {
- this._addresses =
- new Geary.RFC822.MailboxAddresses.from_rfc822_string(text);
- this.is_valid = true;
+ try {
+ this._addresses =
+ new Geary.RFC822.MailboxAddresses.from_rfc822_string(text);
+ this.is_valid = true;
+ } catch (Geary.RFC822Error err) {
+ this._addresses = new Geary.RFC822.MailboxAddresses();
+ this.is_valid = false;
+ }
}
}
diff --git a/src/engine/imap-db/imap-db-message-row.vala b/src/engine/imap-db/imap-db-message-row.vala
index 36a4699b8..4744a9572 100644
--- a/src/engine/imap-db/imap-db-message-row.vala
+++ b/src/engine/imap-db/imap-db-message-row.vala
@@ -272,20 +272,22 @@ private class Geary.ImapDB.MessageRow {
return (addrs == null || addrs.size == 0) ? null : addrs.to_rfc822_string();
}
- 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.MailboxAddress? unflatten_address(string? str)
+ throws RFC822Error {
+ return (
+ String.is_empty_or_whitespace(str)
+ ? null
+ : new RFC822.MailboxAddress.from_rfc822_string(str)
+ );
}
- private RFC822.MailboxAddresses? unflatten_addresses(string? str) {
- return String.is_empty(str) ? null : new RFC822.MailboxAddresses.from_rfc822_string(str);
+ private RFC822.MailboxAddresses? unflatten_addresses(string? str)
+ throws RFC822Error {
+ return (
+ String.is_empty_or_whitespace(str)
+ ? null
+ : new RFC822.MailboxAddresses.from_rfc822_string(str)
+ );
}
-}
+}
diff --git a/src/engine/imap/message/imap-message-data.vala b/src/engine/imap/message/imap-message-data.vala
index e4d423ff0..07c10f3cd 100644
--- a/src/engine/imap/message/imap-message-data.vala
+++ b/src/engine/imap/message/imap-message-data.vala
@@ -26,8 +26,13 @@ public class Geary.Imap.RFC822Size : Geary.RFC822.Size, Geary.Imap.MessageData {
}
public class Geary.Imap.RFC822Header : Geary.RFC822.Header, Geary.Imap.MessageData {
- public RFC822Header(Memory.Buffer buffer) {
- base (buffer);
+ public RFC822Header(Memory.Buffer buffer)
+ throws ImapError {
+ try {
+ base(buffer);
+ } catch (RFC822Error error) {
+ throw new ImapError.INVALID(error.message);
+ }
}
}
@@ -42,4 +47,3 @@ public class Geary.Imap.RFC822Full : Geary.RFC822.Full, Geary.Imap.MessageData {
base (buffer);
}
}
-
diff --git a/src/engine/rfc822/rfc822-mailbox-address.vala b/src/engine/rfc822/rfc822-mailbox-address.vala
index 6d994136c..5f5ad87e2 100644
--- a/src/engine/rfc822/rfc822-mailbox-address.vala
+++ b/src/engine/rfc822/rfc822-mailbox-address.vala
@@ -233,21 +233,27 @@ public class Geary.RFC822.MailboxAddress :
Geary.RFC822.get_parser_options(),
rfc822
);
- if (addrlist == null)
- return;
-
- int length = addrlist.length();
- for (int ctr = 0; ctr < length; ctr++) {
- GMime.InternetAddress? addr = addrlist.get_address(ctr);
-
- // TODO: Handle group lists
- GMime.InternetAddressMailbox? mbox_addr = addr as GMime.InternetAddressMailbox;
- if (mbox_addr != null) {
- this.gmime(mbox_addr);
- return;
- }
+ if (addrlist == null) {
+ throw new RFC822Error.INVALID(
+ "Not a RFC822 mailbox address: %s", rfc822
+ );
+ }
+ if (addrlist.length() != 1) {
+ throw new RFC822Error.INVALID(
+ "Not a single RFC822 mailbox address: %s", rfc822
+ );
}
- throw new RFC822Error.INVALID("Could not parse RFC822 address: %s", rfc822);
+
+ GMime.InternetAddress? addr = addrlist.get_address(0);
+ // TODO: Handle group lists
+ var mbox_addr = addr as GMime.InternetAddressMailbox;
+ if (mbox_addr == null) {
+ throw new RFC822Error.INVALID(
+ "Group lists not currently supported: %s", rfc822
+ );
+ }
+
+ this.gmime(mbox_addr);
}
public MailboxAddress.gmime(GMime.InternetAddressMailbox mailbox) {
diff --git a/src/engine/rfc822/rfc822-mailbox-addresses.vala b/src/engine/rfc822/rfc822-mailbox-addresses.vala
index 4afd096de..41886acba 100644
--- a/src/engine/rfc822/rfc822-mailbox-addresses.vala
+++ b/src/engine/rfc822/rfc822-mailbox-addresses.vala
@@ -78,13 +78,12 @@ public class Geary.RFC822.MailboxAddresses :
this.addrs.add(addr);
}
- public MailboxAddresses.from_rfc822_string(string rfc822) {
- GMime.InternetAddressList addrlist = GMime.InternetAddressList.parse(
- Geary.RFC822.get_parser_options(),
- rfc822
- );
- if (addrlist == null)
- return;
+ public MailboxAddresses.from_rfc822_string(string rfc822)
+ throws RFC822Error {
+ var addrlist = GMime.InternetAddressList.parse(null, rfc822);
+ if (addrlist == null) {
+ throw new RFC822Error.INVALID("Not a RFC822 mailbox address list");
+ }
int length = addrlist.length();
for (int ctr = 0; ctr < length; ctr++) {
diff --git a/src/engine/rfc822/rfc822-message-data.vala b/src/engine/rfc822/rfc822-message-data.vala
index d4a3aa0aa..e7f5df124 100644
--- a/src/engine/rfc822/rfc822-message-data.vala
+++ b/src/engine/rfc822/rfc822-message-data.vala
@@ -169,17 +169,15 @@ public class Geary.RFC822.MessageIDList : Geary.MessageData.AbstractMessageData,
public class Geary.RFC822.Date : Geary.RFC822.MessageData, Geary.MessageData.AbstractMessageData,
Gee.Hashable<Geary.RFC822.Date> {
- public string? original { get; private set; }
+ public string original { get; private set; }
public DateTime value { get; private set; }
- public Date(string rfc822) throws ImapError {
- DateTime? value = GMime.utils_header_decode_date(rfc822);
- if (value == null) {
- throw new ImapError.PARSE_ERROR(
- "Unable to parse \"%s\": Outside supported range", rfc822
- );
+ public Date(string rfc822) throws RFC822Error {
+ var date = GMime.utils_header_decode_date(rfc822);
+ if (date == null) {
+ throw new RFC822Error.INVALID("Not ISO-8601 date: %s", rfc822);
}
- this.value = value;
+ this.value = date;
this.original = rfc822;
}
@@ -302,42 +300,38 @@ public class Geary.RFC822.Header : Geary.MessageData.BlockMessageData, Geary.RFC
private GMime.Message? message = null;
private string[]? names = null;
- public Header(Memory.Buffer buffer) {
+ public Header(Memory.Buffer buffer) throws RFC822Error {
base("RFC822.Header", buffer);
- }
- private unowned GMime.HeaderList get_headers() throws RFC822Error {
- if (message != null)
- return message.get_header_list();
-
- GMime.Parser parser = new GMime.Parser.with_stream(Utils.create_stream_mem(buffer));
+ var parser = new GMime.Parser.with_stream(
+ Utils.create_stream_mem(buffer)
+ );
parser.set_respect_content_length(false);
+ parser.set_format(MESSAGE);
- message = parser.construct_message(Geary.RFC822.get_parser_options());
- if (message == null)
+ this.message = parser.construct_message(null);
+ if (this.message == null) {
throw new RFC822Error.INVALID("Unable to parse RFC 822 headers");
-
- return message.get_header_list();
+ }
}
- public string? get_header(string name) throws RFC822Error {
- GMime.Header header = get_headers().get_header(name);
- if (header != null)
- // We should not parse the raw-value here, but use GMime's parsing
- // functionality instead.
- // See: https://gitlab.gnome.org/GNOME/geary/merge_requests/382#note_669699
- return GMime.utils_header_unfold(header.get_raw_value());
- else
- return null;
+ public string? get_header(string name) {
+ string? value = null;
+ var header = this.message.get_header_list().get_header(name);
+ if (header != null) {
+ value = header.get_value();
+ }
+ return value;
}
- public string[] get_header_names() throws RFC822Error {
+ public string[] get_header_names() {
if (this.names == null) {
- this.names = new string[0];
- GMime.HeaderList headers = get_headers();
- for (int i = 0; i < headers.get_count(); i++) {
- names += headers.get_header_at(i).get_name();
+ GMime.HeaderList headers = this.message.get_header_list();
+ var names = new string[headers.get_count()];
+ for (int i = 0; i < names.length; i++) {
+ names[i] = headers.get_header_at(0).get_name();
}
+ this.names = names;
}
return this.names;
}
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index 4ef341c8e..ff63f58eb 100644
--- a/src/engine/rfc822/rfc822-message.vala
+++ b/src/engine/rfc822/rfc822-message.vala
@@ -6,6 +6,7 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
+
/**
* An RFC-822 style email message.
*
@@ -99,16 +100,19 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
stock_from_gmime();
}
- public Message.from_gmime_message(GMime.Message message) {
+ public Message.from_gmime_message(GMime.Message message)
+ throws RFC822Error {
this.message = message;
stock_from_gmime();
}
- public Message.from_buffer(Memory.Buffer full_email) throws RFC822Error {
+ public Message.from_buffer(Memory.Buffer full_email)
+ throws RFC822Error {
this(new Geary.RFC822.Full(full_email));
}
- public Message.from_parts(Header header, Text body) throws RFC822Error {
+ public Message.from_parts(Header header, Text body)
+ throws RFC822Error {
GMime.StreamCat stream_cat = new GMime.StreamCat();
stream_cat.add_source(new GMime.StreamMem.with_buffer(header.buffer.get_bytes().get_data()));
stream_cat.add_source(new GMime.StreamMem.with_buffer(body.buffer.get_bytes().get_data()));
@@ -126,9 +130,11 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
public async Message.from_composed_email(Geary.ComposedEmail email,
string? message_id,
- GLib.Cancellable? cancellable) {
+ GLib.Cancellable? cancellable)
+ throws RFC822Error {
this.message = new GMime.Message(true);
+ //
// Required headers
this.from = email.from;
@@ -802,7 +808,8 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
* disabled by passing false in include_sub_messages). Note that values
* that come out of this function are persisted.
*/
- public string? get_searchable_body(bool include_sub_messages = true) {
+ public string? get_searchable_body(bool include_sub_messages = true)
+ throws RFC822Error {
string? body = null;
bool html = false;
try {
@@ -880,81 +887,70 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
return attachments;
}
- private void stock_from_gmime() {
- GMime.HeaderList headers = this.message.get_header_list();
+ private void stock_from_gmime() throws RFC822Error {
+ GMime.HeaderList headers = this.message.headers;
for (int i = 0; i < headers.get_count(); i++) {
GMime.Header header = headers.get_header_at(i);
- string name = header.get_name();
- // We should not parse the raw-value here, but use GMime's parsing
- // functionality instead.
- // See: https://gitlab.gnome.org/GNOME/geary/merge_requests/382#note_669699
- string value = GMime.utils_header_unfold(header.get_raw_value());
- switch (name.down()) {
- case "from":
- this.from = append_address(this.from, value);
- break;
-
- case "sender":
- try {
- this.sender = new RFC822.MailboxAddress.from_rfc822_string(value);
- } catch (Error err) {
- debug("Could parse subject: %s", err.message);
- }
- break;
-
- case "reply-to":
- this.reply_to = append_address(this.reply_to, value);
- break;
-
- case "to":
- this.to = append_address(this.to, value);
- break;
-
- case "cc":
- this.cc = append_address(this.cc, value);
- break;
-
- case "bcc":
- this.bcc = append_address(this.bcc, value);
- break;
-
- case "subject":
- this.subject = new RFC822.Subject.decode(value);
- break;
-
- case "date":
- try {
- this.date = new Geary.RFC822.Date(value);
- } catch (Error err) {
- debug("Could not parse date: %s", err.message);
- }
- break;
-
- case "message-id":
- this.message_id = new MessageID(value);
- break;
-
- case "in-reply-to":
- this.in_reply_to = append_message_id(this.in_reply_to, value);
- break;
-
- case "references":
- this.references = append_message_id(this.references, value);
- break;
-
- case "x-mailer":
- this.mailer = GMime.utils_header_decode_text(Geary.RFC822.get_parser_options(), value);
- break;
-
- default:
- // do nothing
- break;
+ string value = header.get_value();
+ switch (header.get_name().down()) {
+ case "from":
+ this.from = append_address(this.from, value);
+ break;
+
+ case "sender":
+ this.sender = new MailboxAddress.from_rfc822_string(value);
+ break;
+
+ case "reply-to":
+ this.reply_to = append_address(this.reply_to, value);
+ break;
+
+ case "to":
+ this.to = append_address(this.to, value);
+ break;
+
+ case "cc":
+ this.cc = append_address(this.cc, value);
+ break;
+
+ case "bcc":
+ this.bcc = append_address(this.bcc, value);
+ break;
+
+ case "subject":
+ this.subject = new Subject.decode(value);
+ break;
+
+ case "date":
+ this.date = new Date(value);
+ break;
+
+ case "message-id":
+ this.message_id = new MessageID(value);
+ break;
+
+ case "in-reply-to":
+ this.in_reply_to = append_message_id(this.in_reply_to, value);
+ break;
+
+ case "references":
+ this.references = append_message_id(this.references, value);
+ break;
+
+ case "x-mailer":
+ this.mailer = GMime.utils_header_decode_text(null, value);
+ break;
+
+ default:
+ // Ignore anything else not
+ break;
}
- };
+ }
}
private MailboxAddresses append_address(MailboxAddresses? existing,
- string header_value) {
+ string header_value)
+ throws RFC822Error {
MailboxAddresses addresses = new MailboxAddresses.from_rfc822_string(header_value);
if (existing != null) {
addresses = existing.append(addresses);
@@ -1063,13 +1059,16 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
}
#endif
- public Gee.List<Geary.RFC822.Message> get_sub_messages() {
+ public Gee.List<Geary.RFC822.Message> get_sub_messages()
+ throws RFC822Error {
Gee.List<Geary.RFC822.Message> messages = new Gee.ArrayList<Geary.RFC822.Message>();
find_sub_messages(messages, message.get_mime_part());
return messages;
}
- private void find_sub_messages(Gee.List<Geary.RFC822.Message> messages, GMime.Object root) {
+ private void find_sub_messages(Gee.List<Message> messages,
+ GMime.Object root)
+ throws RFC822Error {
// If this is a multipart container, check each of its children.
GMime.Multipart? multipart = root as GMime.Multipart;
if (multipart != null) {
@@ -1084,7 +1083,7 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
if (messagepart != null) {
GMime.Message sub_message = messagepart.get_message();
if (sub_message != null) {
- messages.add(new Geary.RFC822.Message.from_gmime_message(sub_message));
+ messages.add(new Message.from_gmime_message(sub_message));
} else {
warning("Corrupt message, possibly bug 769697");
}
diff --git a/test/engine/rfc822-message-test.vala b/test/engine/rfc822-message-test.vala
index cc3f0a405..979fcccb1 100644
--- a/test/engine/rfc822-message-test.vala
+++ b/test/engine/rfc822-message-test.vala
@@ -371,7 +371,8 @@ This is the second line.
assert_true(out_buffer.size > (buffer.size+buffer2.size), "Expected sizeable message");
}
- private async Geary.RFC822.Message message_from_composed_email(Geary.ComposedEmail composed) {
+ private async Message message_from_composed_email(ComposedEmail composed)
+ throws RFC822Error {
return yield new Geary.RFC822.Message.from_composed_email(
composed,
GMime.utils_generate_message_id(composed.from.get(0).domain),
diff --git a/test/integration/smtp/client-session.vala b/test/integration/smtp/client-session.vala
index b6c0c8017..33ee89d02 100644
--- a/test/integration/smtp/client-session.vala
+++ b/test/integration/smtp/client-session.vala
@@ -118,7 +118,8 @@ class Integration.Smtp.ClientSession : TestCase {
}
private async Geary.RFC822.Message new_message(Geary.RFC822.MailboxAddress from,
- Geary.RFC822.MailboxAddress to) {
+ Geary.RFC822.MailboxAddress to)
+ throws Geary.RFC822Error {
Geary.ComposedEmail composed = new Geary.ComposedEmail(
new GLib.DateTime.now_local(),
new Geary.RFC822.MailboxAddresses.single(from)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
