Commit 89ca0167 authored by Jim Nelson's avatar Jim Nelson

Fix displaying images referenced by Content-ID: Bug #743676

The logic in our image replacement code needed to take account if the
MIME part was inside a multipart/mixed container or otherwise.  This
patch does that as well as clean up the code path and better document
the API as well as what's going on inside ConversationViewer when the
message is assembled.
parent 13665f77
......@@ -243,6 +243,7 @@ engine/mime/mime-content-type.vala
engine/mime/mime-data-format.vala
engine/mime/mime-disposition-type.vala
engine/mime/mime-error.vala
engine/mime/mime-multipart-subtype.vala
engine/nonblocking/nonblocking-abstract-semaphore.vala
engine/nonblocking/nonblocking-batch.vala
......
......@@ -453,7 +453,7 @@ public class ComposerWidget : Gtk.EventBox {
if (referred.subject != null)
subject = referred.subject.value;
try {
body_html = referred.get_message().get_body(true);
body_html = referred.get_message().get_body(Geary.RFC822.TextFormat.HTML, null);
} catch (Error error) {
debug("Error getting message body: %s", error.message);
}
......@@ -468,7 +468,8 @@ public class ComposerWidget : Gtk.EventBox {
case ComposeType.REPLY_ALL:
subject = reply_subject;
references = Geary.RFC822.Utils.reply_references(referred);
body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_reply(referred, quote, true);
body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_reply(referred, quote,
Geary.RFC822.TextFormat.HTML);
pending_attachments = referred.attachments;
if (quote != null)
top_posting = false;
......@@ -478,7 +479,8 @@ public class ComposerWidget : Gtk.EventBox {
case ComposeType.FORWARD:
subject = forward_subject;
body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_forward(referred, quote, true);
body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_forward(referred, quote,
Geary.RFC822.TextFormat.HTML);
add_attachments(referred.attachments);
pending_attachments = referred.attachments;
break;
......@@ -898,7 +900,7 @@ public class ComposerWidget : Gtk.EventBox {
WebKit.DOM.Document document = editor.get_dom_document();
// Always use reply styling, since forward styling doesn't work for inline quotes
document.exec_command("insertHTML", false,
Geary.RFC822.Utils.quote_email_for_reply(referred, quote, true));
Geary.RFC822.Utils.quote_email_for_reply(referred, quote, Geary.RFC822.TextFormat.HTML));
if (!referred_ids.contains(referred.id)) {
add_recipients_and_ids(new_type, referred);
......
......@@ -848,11 +848,31 @@ public class ConversationViewer : Gtk.Box {
} catch (Error error) {
debug("Failed to add preview text: %s", error.message);
}
//
// Build an HTML document from the email with two passes:
//
// * Geary.RFC822.Message.get_body() recursively walks the message's MIME structure looking
// for text MIME parts and assembles them sequentially. If non-text MIME parts are
// discovered within a multipart/mixed container, it calls inline_image_replacer(), which
// converts them to an IMG tag with a data: URI if they are a supported image type.
// Otherwise, the MIME part is dropped.
//
// * insert_html_markup() then strips everything outside the BODY, turning the BODY tag
// itself into a DIV, and performs other massaging of the HTML. It also looks for IMG
// tags that refer to other MIME parts via their Content-ID, converts them to data: URIs,
// and inserts them into the document.
//
// Attachments are generated and added in add_message(), which calls this method before
// building the HTML for them. The above two steps take steps to avoid inlining images
// that are actually attachments (in particular, get_body() considers their
// Content-Disposition)
//
string body_text = "";
remote_images = false;
try {
body_text = message.get_body(true, inline_image_replacer) ?? "";
body_text = message.get_body(Geary.RFC822.TextFormat.HTML, inline_image_replacer) ?? "";
body_text = insert_html_markup(body_text, message, out remote_images);
} catch (Error err) {
debug("Could not get message text. %s", err.message);
......@@ -915,9 +935,20 @@ public class ConversationViewer : Gtk.Box {
return false;
}
// This delegate is called from within Geary.RFC822.Message.get_body while assembling the plain
// or HTML document when a non-text MIME part is encountered within a multipart/mixed container.
// If this returns null, the MIME part is dropped from the final returned document; otherwise,
// this returns HTML that is placed into the document in the position where the MIME part was
// found
private string? inline_image_replacer(string filename, Geary.Mime.ContentType? content_type,
Geary.Mime.ContentDisposition? disposition, string? content_id, Geary.Memory.Buffer buffer) {
if (content_type == null || !is_content_type_supported_inline(content_type)) {
if (content_type == null) {
debug("Not displaying inline: no Content-Type");
return null;
}
if (!is_content_type_supported_inline(content_type)) {
debug("Not displaying %s inline: unsupported Content-Type", content_type.to_string());
return null;
......@@ -1958,7 +1989,7 @@ public class ConversationViewer : Gtk.Box {
if (Geary.String.is_empty(src))
continue;
// if no Content-ID, then leave as-is, but note if a data: URI is being used for
// if no Content-ID, then leave as-is, but note if a non-data: URI is being used for
// purposes of detecting remote images
string? content_id = src.has_prefix("cid:") ? src.substring(4) : null;
if (Geary.String.is_empty(content_id)) {
......@@ -1981,10 +2012,10 @@ public class ConversationViewer : Gtk.Box {
guess = ContentType.guess(null, unowned_buffer.to_unowned_uint8_array(), null);
else
guess = ContentType.guess(null, image_content.get_uint8_array(), null);
string mimetype = ContentType.get_mime_type(guess);
// Replace the SRC to a data URIm the class to a known label for the popup menu,
// Replace the SRC to a data URI, the class to a known label for the popup menu,
// and the ALT to its filename, if supplied
img.set_attribute("src", assemble_data_uri(mimetype, image_content));
img.set_attribute("class", DATA_IMAGE_CLASS);
......@@ -1995,6 +2026,7 @@ public class ConversationViewer : Gtk.Box {
// Content-Disposition)
inlined_content_ids.add(content_id);
} else {
// replaced by data: URI, remove this tag and let the inserted one shine through
img.parent_element.remove_child(img);
}
}
......@@ -2010,7 +2042,7 @@ public class ConversationViewer : Gtk.Box {
debug("Error removing inlined image: %s", error.message);
}
}
// Now return the whole message.
return container.get_inner_html();
} catch (Error e) {
......
......@@ -90,7 +90,7 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
* @see is_type
*/
public bool has_media_type(string media_type) {
return (media_type != WILDCARD) ? String.stri_equal(this.media_type, media_type) : true;
return (media_type != WILDCARD) ? Ascii.stri_equal(this.media_type, media_type) : true;
}
/**
......@@ -101,7 +101,7 @@ public class Geary.Mime.ContentType : Geary.BaseObject {
* @see is_type
*/
public bool has_media_subtype(string media_subtype) {
return (media_subtype != WILDCARD) ? String.stri_equal(this.media_subtype, media_subtype) : true;
return (media_subtype != WILDCARD) ? Ascii.stri_equal(this.media_subtype, media_subtype) : true;
}
/**
......
/* Copyright 2015 Yorba Foundation
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
/**
* A represenation of a MIME multipart Content-Type subtype.
*
* See [[https://tools.ietf.org/html/rfc2046#section-5.1]]
*/
public enum Geary.Mime.MultipartSubtype {
/**
* Used as a placeholder for no or unknown multipart subtype.
*
* Technically an unknown or unspecified subtype should be treated as {@link MIXED}, but there
* are situations in code where this is useful.
*/
UNSPECIFIED,
/**
* A multipart structure of mixed media.
*
* "Any 'multipart' subtypes that an implementation does not recognize must be treated as
* being of subtype 'mixed'."
*
* See [[https://tools.ietf.org/html/rfc2046#section-5.1.3]]
*/
MIXED,
/**
* A multipart structure of alternative media.
*
* See [[https://tools.ietf.org/html/rfc2046#section-5.1.4]]
*/
ALTERNATIVE,
/**
* A multipart structure of related media.
*
* See [[http://tools.ietf.org/html/rfc2387]]
*/
RELATED;
/**
* Converts a {@link ContentType} into a {@link MultipartSubtype}.
*
* If unknown, {@link MIXED} is returned but is_unknown will be true.
*/
public static MultipartSubtype from_content_type(ContentType? content_type, out bool is_unknown) {
if (content_type == null || !content_type.has_media_type("multipart")) {
is_unknown = true;
return MIXED;
}
is_unknown = false;
switch (Ascii.strdown(content_type.media_subtype)) {
case "mixed":
return MIXED;
case "alternative":
return ALTERNATIVE;
case "related":
return RELATED;
default:
is_unknown = true;
return MIXED;
}
}
}
......@@ -8,6 +8,10 @@ public class Geary.RFC822.Message : BaseObject {
/**
* This delegate is an optional parameter to the body constructers that allows callers
* to process arbitrary non-text, inline MIME parts.
*
* This is only called for non-text MIME parts in mixed multipart sections. Inline parts
* referred to by rich text in alternative or related documents must be located by the caller
* and appropriately presented.
*/
public delegate string? InlinePartReplacer(string filename, Mime.ContentType? content_type,
Mime.ContentDisposition? disposition, string? content_id, Geary.Memory.Buffer buffer);
......@@ -306,7 +310,7 @@ public class Geary.RFC822.Message : BaseObject {
public string get_preview() {
string? preview = null;
try {
preview = get_text_body(false, null);
preview = get_plain_body(false, null);
} catch (Error e) {
try {
preview = Geary.HTML.remove_html_tags(get_html_body(null));
......@@ -442,40 +446,50 @@ public class Geary.RFC822.Message : BaseObject {
}
/**
* This method is the main utility method used by the other body constructors. It calls itself
* recursively via the last argument ("node").
*
* The constructed body is stored in ref string? body. If the constructed body is null,
* ref string? body remains unmodified.
* This method is the main utility method used by the other body-generating constructors.
*
* Only text/* MIME parts of the specified subtype are added to body. If a non-text part is
* within a multipart/mixed container, the {@link InlinePartReplacer} is invoked.
*
* If to_html is true, the text is run through a filter to HTML-ize it. (Obviously, this
* should be false if text/html is being searched for.).
*
* The final constructed body is stored in the body string.
*
* ref string? body is only modified if the constructed body is non-empty
*
* Returns: a bool indicating whether a text part with the desired text_subtype was found
* The initial call should pass the root of this message and UNSPECIFIED as its container
* subtype.
*
* @returns Whether a text part with the desired text_subtype was found
*/
private bool construct_body_from_mime_parts(ref string? body, InlinePartReplacer? replacer,
string text_subtype, bool allow_only_replaced, bool to_html, GMime.Object? node)
throws RFC822Error {
if (node == null) {
node = message.get_mime_part();
}
private bool construct_body_from_mime_parts(GMime.Object node, Mime.MultipartSubtype container_subtype,
string text_subtype, bool to_html, InlinePartReplacer? replacer, ref string? body) throws RFC822Error {
Mime.ContentType? this_content_type = null;
if (node.get_content_type() != null)
this_content_type = new Mime.ContentType.from_gmime(node.get_content_type());
// If this is a multipart, call ourselves recursively on the children
GMime.Multipart? multipart = node as GMime.Multipart;
if (multipart != null) {
Mime.MultipartSubtype this_subtype = Mime.MultipartSubtype.from_content_type(this_content_type,
null);
bool found_text_subtype = false;
StringBuilder builder = new StringBuilder();
int count = multipart.get_count();
for (int i = 0; i < count; ++i) {
GMime.Object child = multipart.get_part(i);
string? child_body = null;
found_text_subtype |= construct_body_from_mime_parts(ref child_body, replacer,
text_subtype, allow_only_replaced, to_html, child);
found_text_subtype |= construct_body_from_mime_parts(child, this_subtype, text_subtype,
to_html, replacer, ref child_body);
if (child_body != null)
builder.append(child_body);
}
if (!Geary.String.is_empty_or_whitespace(builder.str))
if (!String.is_empty(builder.str))
body = builder.str;
return found_text_subtype;
}
......@@ -492,53 +506,45 @@ public class Geary.RFC822.Message : BaseObject {
if (disposition != null && disposition.disposition_type == Mime.DispositionType.ATTACHMENT)
return false;
/* Handle text parts that are not attachments
* They may have inline disposition, or they may have no disposition specified
*/
Mime.ContentType? content_type = null;
if (part.get_content_type() != null) {
content_type = new Mime.ContentType.from_gmime(part.get_content_type());
if (content_type.has_media_type("text")) {
if (content_type.has_media_subtype(text_subtype)) {
body = mime_part_to_memory_buffer(part, true, to_html).to_string();
return true;
}
// Assemble body from text parts that are not attachments
if (this_content_type != null && this_content_type.has_media_type("text")) {
if (this_content_type.has_media_subtype(text_subtype)) {
body = mime_part_to_memory_buffer(part, true, to_html).to_string();
// We were the wrong kind of text part
return false;
return true;
}
// We were the wrong kind of text part
return false;
}
// If images have no disposition, they are handled elsewhere; see #7299
if (disposition == null || disposition.disposition_type == Mime.DispositionType.UNSPECIFIED)
return false;
if (replacer == null)
// Use inline part replacer *only* if in a mixed multipart where each element is to be
// presented to the user as structure dictates; for alternative and related, the inline
// part is referred to elsewhere in the document and it's the callers responsibility to
// locate them
if (replacer == null || container_subtype != Mime.MultipartSubtype.MIXED)
return false;
// Hand off to the replacer for processing
string? replaced_part = replacer(RFC822.Utils.get_clean_attachment_filename(part), content_type,
disposition, part.get_content_id(), mime_part_to_memory_buffer(part));
if (replaced_part != null)
body = replaced_part;
body = replacer(RFC822.Utils.get_clean_attachment_filename(part),
this_content_type, disposition, part.get_content_id(), mime_part_to_memory_buffer(part));
return allow_only_replaced && (replaced_part != null);
return body != null;
}
/**
* A front-end to construct_body_from_mime_parts() that converts its output parameters into
* something that front-facing methods want to return.
*
* The allow_only_replaced flag indicates if it's allowable for the method to return only the
* InlinePartReplacer's returned text. In other words, if only an inline MIME section is found
* but no portion of text_subtype, allow_only_replaced indicates if the InlinePartReplacer's
* returned text constitutes a "body".
*/
private string? internal_get_body(bool allow_only_replaced, string text_subtype, bool to_html,
InlinePartReplacer? replacer) throws RFC822Error {
private string? internal_get_body(string text_subtype, bool to_html, InlinePartReplacer? replacer)
throws RFC822Error {
string? body = null;
if (!construct_body_from_mime_parts(ref body, replacer, text_subtype, allow_only_replaced,
to_html, null)) {
if (!construct_body_from_mime_parts(message.get_mime_part(), Mime.MultipartSubtype.UNSPECIFIED,
text_subtype, to_html, replacer, ref body)) {
throw new RFC822Error.NOT_FOUND("Could not find any \"text/%s\" parts", text_subtype);
}
......@@ -548,48 +554,73 @@ public class Geary.RFC822.Message : BaseObject {
/**
* Returns the HTML portion of the message body, if present.
*
* Throws {@link RFC822Error.NOT_FOUND} if an HTML body is not present.
* See {@link get_body} for more details on how the body is assembled from the message's MIME
* structure and the role of the {@link InlinePartReplacer}.
*
* @throws {@link RFC822Error.NOT_FOUND} if an HTML body is not present.
*/
private string? get_html_body(InlinePartReplacer? replacer) throws RFC822Error {
return internal_get_body(true, "html", false, replacer);
public string? get_html_body(InlinePartReplacer? replacer) throws RFC822Error {
return internal_get_body("html", false, replacer);
}
/**
* Returns the plaintext portion of the message body, if present.
*
* See {@link get_body} for more details on how the body is assembled from the message's MIME
* structure and the role of the {@link InlinePartReplacer}.
*
* The convert_to_html flag indicates if the plaintext body should be converted into HTML.
* Note that the InlinePartReplacer's output is not converted; it's up to the caller to know
* what format to return when invoked.
*
* Throws {@link RFC822Error.NOT_FOUND} if a plaintext body is not present.
* @throws RFC822Error.NOT_FOUND if a plaintext body is not present.
*/
private string? get_text_body(bool convert_to_html, InlinePartReplacer? replacer) throws RFC822Error {
return internal_get_body(true, "plain", convert_to_html, replacer);
public string? get_plain_body(bool convert_to_html, InlinePartReplacer? replacer) throws RFC822Error {
return internal_get_body("plain", convert_to_html, replacer);
}
/**
* Returns a body of the email as HTML.
* Returns the complete email body as HTML.
*
* The html_format flag indicates whether to use the HTML portion of the message body or to
* get_body() recursively walks the MIME structure (depth-first) serializing all text MIME
* parts of the specified type into a single UTF-8 string. Non-text MIME parts inside of
* multipart/mixed containers are offered to the {@link InlinePartReplacer}, which can either
* return null or return a string that is inserted in lieu of the MIME part into the final
* document. All other MIME parts are ignored.
*
* The format flag indicates whether to assemble the HTML portion of the message body or to
* convert the plaintext portion into HTML. If the requested portion is not present, the
* method will fallback and attempt to return the other (converted to HTML, if necessary).
* It is possible for html_format to be false and this method to return HTML (if plaintext
* is unavailable). Consider using {@link get_html_body} or {@link get_text_body} if finer
* control is desired.
* It is possible for format to be PLAIN and this method to return an HTML portion of the
* message (if plaintext is unavailable).
*
* Note that the InlinePartReplacer's output is never converted and should return HTML.
*
* Throws {@link RFC822Error.NOT_FOUND if neither format is available.
* @throws RFC822Error.NOT_FOUND if neither format is available.
*/
public string? get_body(bool html_format, InlinePartReplacer? replacer = null) throws RFC822Error {
public string? get_body(TextFormat format, InlinePartReplacer? replacer) throws RFC822Error {
try {
return html_format
? internal_get_body(false, "html", false, replacer)
: internal_get_body(false, "plain", true, replacer);
switch (format) {
case TextFormat.HTML:
return get_html_body(replacer);
case TextFormat.PLAIN:
return get_plain_body(true, replacer);
default:
return null;
}
} catch (Error err) {
return html_format
? internal_get_body(true, "plain", true, replacer)
: internal_get_body(true, "html", false, replacer);
switch (format) {
case TextFormat.HTML:
return get_plain_body(true, replacer);
case TextFormat.PLAIN:
return get_html_body(replacer);
default:
assert_not_reached();
}
}
}
......@@ -608,7 +639,7 @@ public class Geary.RFC822.Message : BaseObject {
html = true;
} catch (Error e) {
try {
body = get_text_body(false, null);
body = get_plain_body(false, null);
} catch (Error e) {
// Ignore.
}
......
......@@ -184,13 +184,20 @@ public string reply_references(Geary.Email source) {
return (list.size > 0) ? string.joinv(" ", strings) : "";
}
public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses,
bool html_format) {
public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses, TextFormat format) {
if (addresses == null)
return "";
return html_format ? HTML.escape_markup(addresses.to_string()) : addresses.to_string();
switch (format) {
case TextFormat.HTML:
return HTML.escape_markup(addresses.to_string());
case TextFormat.PLAIN:
return addresses.to_string();
default:
assert_not_reached();
}
}
......@@ -203,7 +210,7 @@ public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses
* If html_format is true, the message will be quoted in HTML format.
* Otherwise it will be in plain text.
*/
public string quote_email_for_reply(Geary.Email email, string? quote, bool html_format) {
public string quote_email_for_reply(Geary.Email email, string? quote, TextFormat format) {
if (email.body == null && quote == null)
return "";
......@@ -219,13 +226,13 @@ public string quote_email_for_reply(Geary.Email email, string? quote, bool html_
/// the original sender.
string QUOTED_LABEL = _("On %1$s, %2$s wrote:");
quoted += QUOTED_LABEL.printf(email.date.value.format(DATE_FORMAT),
email_addresses_for_reply(email.from, html_format));
email_addresses_for_reply(email.from, format));
} else if (email.from != null) {
/// The quoted header for a message being replied to (in case the date is not known).
/// %s will be replaced by the original sender.
string QUOTED_LABEL = _("%s wrote:");
quoted += QUOTED_LABEL.printf(email_addresses_for_reply(email.from, html_format));
quoted += QUOTED_LABEL.printf(email_addresses_for_reply(email.from, format));
} else if (email.date != null) {
/// The quoted header for a message being replied to (in case the sender is not known).
......@@ -236,7 +243,7 @@ public string quote_email_for_reply(Geary.Email email, string? quote, bool html_
quoted += "<br />";
quoted += "\n" + quote_body(email, quote, true, html_format);
quoted += "\n" + quote_body(email, quote, true, format);
if (quote != null)
quoted += "<br /><br />\n";
......@@ -253,7 +260,7 @@ public string quote_email_for_reply(Geary.Email email, string? quote, bool html_
* If html_format is true, the message will be quoted in HTML format.
* Otherwise it will be in plain text.
*/
public string quote_email_for_forward(Geary.Email email, string? quote, bool html_format) {
public string quote_email_for_forward(Geary.Email email, string? quote, TextFormat format) {
if (email.body == null && quote == null)
return "";
......@@ -261,31 +268,31 @@ public string quote_email_for_forward(Geary.Email email, string? quote, bool htm
quoted += _("---------- Forwarded message ----------");
quoted += "\n\n";
string from_line = email_addresses_for_reply(email.from, html_format);
string from_line = email_addresses_for_reply(email.from, format);
if (!String.is_empty_or_whitespace(from_line))
quoted += _("From: %s\n").printf(from_line);
quoted += _("Subject: %s\n").printf(email.subject != null ? email.subject.to_string() : "");
quoted += _("Date: %s\n").printf(email.date != null ? email.date.to_string() : "");
string to_line = email_addresses_for_reply(email.to, html_format);
string to_line = email_addresses_for_reply(email.to, format);
if (!String.is_empty_or_whitespace(to_line))
quoted += _("To: %s\n").printf(to_line);
string cc_line = email_addresses_for_reply(email.cc, html_format);
string cc_line = email_addresses_for_reply(email.cc, format);
if (!String.is_empty_or_whitespace(cc_line))
quoted += _("Cc: %s\n").printf(cc_line);
quoted += "\n"; // A blank line between headers and body
quoted = quoted.replace("\n", "<br />");
quoted += quote_body(email, quote, false, html_format);
quoted += quote_body(email, quote, false, format);
return quoted;
}
private string quote_body(Geary.Email email, string? quote, bool use_quotes, bool html_format) {
private string quote_body(Geary.Email email, string? quote, bool use_quotes, TextFormat format) {
string? body_text = "";
try {
body_text = quote ?? email.get_message().get_body(html_format);
body_text = quote ?? email.get_message().get_body(format, null);
} catch (Error error) {
debug("Could not get message text. %s", error.message);
}
......
......@@ -6,6 +6,14 @@
namespace Geary.RFC822 {
/**
* Common text formats supported by {@link Geary.RFC822}.
*/
public enum TextFormat {
PLAIN,
HTML
}
private int init_count = 0;
internal Regex? invalid_filename_character_re = null;
......
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