From ad004070009b153e0007d76a372cbf9a2e44d35d Mon Sep 17 00:00:00 2001 From: Jarrod Moldrich Date: Fri, 22 Oct 2021 16:28:44 +1100 Subject: [PATCH 1/4] Add support for inline attachments... - provide functions for detecting inline attachments - implement a fast concatenation of a list of parts to a message - add helper method to separate parts into text/inline/attachment - wrap body and inline attachments in `multipart/related` --- lib/mail.ex | 34 ++++++++++++++++------ lib/mail/message.ex | 52 ++++++++++++++++++++++++++++++---- lib/mail/renderers/rfc_2822.ex | 45 ++++++++++++++++++++++------- 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/lib/mail.ex b/lib/mail.ex index 2f0c912..4976e78 100644 --- a/lib/mail.ex +++ b/lib/mail.ex @@ -174,18 +174,30 @@ defmodule Mail do do: Mail.Message.put_attachment(message, {filename, data}, opts) @doc """ - Determines the message has any attachment parts + Determines the message has any non-inline attachment parts Returns a `Boolean` """ def has_attachments?(%Mail.Message{} = message) do - walk_parts([message], {:cont, false}, fn message, _acc -> - case Mail.Message.is_attachment?(message) do - true -> {:halt, true} - false -> {:cont, false} - end - end) - |> elem(1) + has_message?(message, &Mail.Message.is_attachment?/1) + end + + @doc """ + Determines the message has any inline attachment parts + + Returns a `Boolean` + """ + def has_inline_attachments?(%Mail.Message{} = message) do + has_message?(message, &Mail.Message.is_inline_attachment?/1) + end + + @doc """ + Determines the message has any inline attachment parts + + Returns a `Boolean` + """ + def has_any_attachments?(%Mail.Message{} = message) do + has_message?(message, &Mail.Message.is_any_attachment?/1) end @doc """ @@ -194,8 +206,12 @@ defmodule Mail do Returns a `Boolean` """ def has_text_parts?(%Mail.Message{} = message) do + has_message?(message, &Mail.Message.is_text_part?/1) + end + + defp has_message?(%Mail.Message{} = message, condition) do walk_parts([message], {:cont, false}, fn message, _acc -> - case Mail.Message.is_text_part?(message) do + case condition.(message) do true -> {:halt, true} false -> {:cont, false} end diff --git a/lib/mail/message.ex b/lib/mail/message.ex index 16a37b0..83fff18 100644 --- a/lib/mail/message.ex +++ b/lib/mail/message.ex @@ -11,9 +11,16 @@ defmodule Mail.Message do Mail.Message.put_part(%Mail.Message{}, %Mail.Message{}) """ - def put_part(message, %Mail.Message{} = part) do - put_in(message.parts, message.parts ++ [part]) - end + def put_part(message, %Mail.Message{} = part), + do: put_in(message.parts, message.parts ++ [part]) + + @doc """ + Add an arbitrary amount of parts + + Mail.Message.put_parts(%Mail.Message{}, [%Mail.Message{}, %Mail.Message{}]) + """ + def put_parts(message, parts) when is_list(parts), + do: put_in(message.parts, message.parts ++ parts) @doc """ Delete a matching part @@ -24,6 +31,14 @@ defmodule Mail.Message do def delete_part(message, part), do: put_in(message.parts, List.delete(message.parts, part)) + @doc """ + Delete a matching part + + Will remove all parts from message. + """ + def delete_all_parts(message), + do: put_in(message.parts, []) + @doc """ Will match on a full or partial content type @@ -330,7 +345,7 @@ defmodule Mail.Message do end @doc """ - Is the part an attachment or not + Is the part a non-inline attachment or not Returns `Boolean` """ @@ -338,7 +353,7 @@ defmodule Mail.Message do do: Enum.member?(List.wrap(get_header(message, :content_disposition)), "attachment") @doc """ - Determines the message has any attachment parts + Determines the message has any non-inline attachment parts Returns a `Boolean` """ @@ -348,6 +363,33 @@ defmodule Mail.Message do def has_attachment?(message), do: has_attachment?(message.parts) + @doc """ + Is the part an inline attachment or not + + Returns `Boolean` + """ + def is_inline_attachment?(message), + do: Enum.member?(List.wrap(get_header(message, :content_disposition)), "inline") + + @doc """ + Determines the message has any inline attachment parts + + Returns a `Boolean` + """ + def has_inline_attachment?(parts) when is_list(parts), + do: has_part?(parts, &is_inline_attachment?/1) + + def has_inline_attachment?(message), + do: has_inline_attachment?(message.parts) + + @doc """ + Is the part any kind of attachment or not + + Returns `Boolean` + """ + def is_any_attachment?(message), + do: is_inline_attachment?(message) || is_attachment?(message) + @doc """ Is the message text based or not diff --git a/lib/mail/renderers/rfc_2822.ex b/lib/mail/renderers/rfc_2822.ex index 0b400d6..4c58bef 100644 --- a/lib/mail/renderers/rfc_2822.ex +++ b/lib/mail/renderers/rfc_2822.ex @@ -215,30 +215,55 @@ defmodule Mail.Renderers.RFC2822 do |> Integer.to_string() |> String.pad_leading(2, "0") + defp split_attachment_parts(message) do + Enum.reduce(message.parts, [[], [], []], fn part, [texts, mixed, inlines] -> + cond do + match_content_type?(part, ~r/text\/(plain|html)/) -> + [[part | texts], mixed, inlines] + Mail.Message.is_inline_attachment?(part) -> + [texts, mixed, [part | inlines]] + true -> # a mixed part - most likely an attachment + [texts, [part | mixed], inlines] + end + end) + |> Enum.map(&Enum.reverse/1) # retain ordering + end + defp reorganize(%Mail.Message{multipart: true} = message) do content_type = Mail.Message.get_content_type(message) - if Mail.Message.has_attachment?(message) do - text_parts = - Enum.filter(message.parts, &match_content_type?(&1, ~r/text\/(plain|html)/)) - |> Enum.sort(&(&1 > &2)) + [text_parts, mixed, inlines] = split_attachment_parts(message) + has_inline = Enum.any?(inlines) + has_mixed_parts = Enum.any?(mixed) + has_text_parts = Enum.any?(text_parts) + if has_inline || has_mixed_parts do + # If any attaching, change content type to mixed content_type = List.replace_at(content_type, 0, "multipart/mixed") message = Mail.Message.put_content_type(message, content_type) - if Enum.any?(text_parts) do - message = Enum.reduce(text_parts, message, &Mail.Message.delete_part(&2, &1)) + if has_text_parts do + # If any text with attachments, wrap in new part + # If any inline attachments, wrap together with text + # in a "multipart/related" part + inner_content_type = has_inline && "multipart/related" || "multipart/alternative" - mixed_part = + body_part = Mail.build_multipart() - |> Mail.Message.put_content_type("multipart/alternative") + |> Mail.Message.put_content_type(inner_content_type) + |> Mail.Message.put_parts(text_parts) + |> Mail.Message.put_parts(inlines) - mixed_part = Enum.reduce(text_parts, mixed_part, &Mail.Message.put_part(&2, &1)) - put_in(message.parts, List.insert_at(message.parts, 0, mixed_part)) + message + |> Mail.Message.delete_all_parts() + |> Mail.Message.put_part(body_part) + |> Mail.Message.put_parts(mixed) else + # If not text sections, leave all parts as is message end else + # If only text, change content type to alternative content_type = List.replace_at(content_type, 0, "multipart/alternative") Mail.Message.put_content_type(message, content_type) end From 25be71e0d3ab61f0e5ba357e10fb7803c48f6234 Mon Sep 17 00:00:00 2001 From: Jarrod Moldrich Date: Sat, 23 Oct 2021 00:14:43 +1100 Subject: [PATCH 2/4] Ensure text parts wrapped in multipart/alternative --- lib/mail/renderers/rfc_2822.ex | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/mail/renderers/rfc_2822.ex b/lib/mail/renderers/rfc_2822.ex index 4c58bef..157588b 100644 --- a/lib/mail/renderers/rfc_2822.ex +++ b/lib/mail/renderers/rfc_2822.ex @@ -244,15 +244,21 @@ defmodule Mail.Renderers.RFC2822 do if has_text_parts do # If any text with attachments, wrap in new part - # If any inline attachments, wrap together with text - # in a "multipart/related" part - inner_content_type = has_inline && "multipart/related" || "multipart/alternative" - body_part = Mail.build_multipart() - |> Mail.Message.put_content_type(inner_content_type) + |> Mail.Message.put_content_type("multipart/alternative") |> Mail.Message.put_parts(text_parts) + + # If any inline attachments, wrap together with text + # in a "multipart/related" part + body_part = if has_inline do + Mail.build_multipart() + |> Mail.Message.put_content_type("multipart/related") + |> Mail.Message.put_part(body_part) |> Mail.Message.put_parts(inlines) + else + body_part + end message |> Mail.Message.delete_all_parts() From f770cbe3d472f6a38de3e8881dc618256ebaf6c7 Mon Sep 17 00:00:00 2001 From: Jarrod Moldrich Date: Sat, 16 Nov 2024 16:23:07 +1100 Subject: [PATCH 3/4] Add tests and clarify docs for MIME part ordering --- lib/mail/message.ex | 2 - lib/mail/renderers/rfc_2822.ex | 24 ++++- test/mail/renderers/rfc_2822_test.exs | 147 ++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 3 deletions(-) diff --git a/lib/mail/message.ex b/lib/mail/message.ex index ebe4600..e3e571e 100644 --- a/lib/mail/message.ex +++ b/lib/mail/message.ex @@ -35,8 +35,6 @@ defmodule Mail.Message do do: put_in(message.parts, List.delete(message.parts, part)) @doc """ - Delete a matching part - Will remove all parts from message. """ def delete_all_parts(message), diff --git a/lib/mail/renderers/rfc_2822.ex b/lib/mail/renderers/rfc_2822.ex index a9118e6..66d244d 100644 --- a/lib/mail/renderers/rfc_2822.ex +++ b/lib/mail/renderers/rfc_2822.ex @@ -265,7 +265,29 @@ defmodule Mail.Renderers.RFC2822 do |> Enum.map(&Enum.reverse/1) # retain ordering end - defp reorganize(%Mail.Message{multipart: true} = message) do + @doc """ + Will organize message parts to conform to expectations on MIME-part order, + specifically in the following format: + + start multipart/mixed + start multipart/related + start multipart/alternative + + end multipart/alternative + + end multipart/related + + end multipart/mixed + + Such that: + + - text and html parts will be grouped in a `multipart/alternative`; + - inline attachments will be postpended and grouped with text and html parts + in a `multipart/related` (RFC 2387); and + - regular attachments will be postpended and grouped with all content in a + `multipart/mixed` + """ + def reorganize(%Mail.Message{multipart: true} = message) do content_type = Mail.Message.get_content_type(message) [text_parts, mixed, inlines] = split_attachment_parts(message) diff --git a/test/mail/renderers/rfc_2822_test.exs b/test/mail/renderers/rfc_2822_test.exs index 3c928c0..123673a 100644 --- a/test/mail/renderers/rfc_2822_test.exs +++ b/test/mail/renderers/rfc_2822_test.exs @@ -339,4 +339,151 @@ defmodule Mail.Renderers.RFC2822Test do |> Mail.Renderers.RFC2822.render() end end + + test "will have correct part order for regular message" do + message = + Mail.build_multipart() + |> Mail.put_to("user1@example.com") + |> Mail.put_from({"User2", "user2@example.com"}) + |> Mail.put_subject("Test email") + |> Mail.put_text("Some text") + |> Mail.put_html("

Some HTML

") + |> Mail.Renderers.RFC2822.reorganize() + + assert %Mail.Message{ + headers: %{"content-type" => ["multipart/alternative"]}, + parts: [ + %Mail.Message{ + headers: %{"content-type" => ["text/plain", {"charset", "UTF-8"}]}, + body: "Some text", + parts: [], + multipart: false + }, + %Mail.Message{ + headers: %{"content-type" => ["text/html", {"charset", "UTF-8"}]}, + body: "

Some HTML

", + parts: [], + multipart: false + } + ] + } = message + end + + test "will have correct part order with only a regular attachment" do + message = + Mail.build_multipart() + |> Mail.put_to("user1@example.com") + |> Mail.put_from({"User2", "user2@example.com"}) + |> Mail.put_subject("Test email") + |> Mail.put_attachment({"tiny_jpeg.jpg", @tiny_jpeg_binary}) + |> Mail.put_text("Some text") + |> Mail.put_html("

Some HTML

") + |> Mail.Renderers.RFC2822.reorganize() + + assert %Mail.Message{ + headers: %{"content-type" => ["multipart/mixed"]}, + parts: [ + %Mail.Message{ + headers: %{"content-type" => ["multipart/alternative"]}, + parts: [ + %Mail.Message{ + headers: %{"content-type" => ["text/plain", {"charset", "UTF-8"}]}, + body: "Some text", + parts: [], + multipart: false + }, + %Mail.Message{ + headers: %{"content-type" => ["text/html", {"charset", "UTF-8"}]}, + body: "

Some HTML

", + parts: [], + multipart: false + } + ] + }, + %Mail.Message{ + headers: %{ + "content-transfer-encoding" => :base64, + "content-type" => ["image/jpeg"] + }, + parts: [], + multipart: false + } + ] + } = message + end + + test "will have correct part order with inline attachment" do + message = + Mail.build_multipart() + |> Mail.put_to("user1@example.com") + |> Mail.put_from({"User2", "user2@example.com"}) + |> Mail.put_subject("Test email") + |> Mail.put_attachment({"tiny_jpeg.jpg", @tiny_jpeg_binary}) + |> Mail.put_attachment({"inline_jpeg.jpg", @tiny_jpeg_binary}, + headers: %{ + content_id: "c_id", + content_type: "image/jpeg", + x_attachment_id: "a_id", + content_disposition: ["inline", filename: "filename"] + } + ) + |> Mail.put_text("Some text") + |> Mail.put_html("

Some HTML

") + |> Mail.Renderers.RFC2822.reorganize() + + # Bamboo.Attachment.new( + # Application.app_dir(:notifications, path), + # filename: filename, + # content_id: "<#{id}>", + # headers: [x_attachment_id: id, content_disposition: ["inline", filename: filename]], + # content_type: "image/#{type}" + # ) + + assert %Mail.Message{ + headers: %{"content-type" => ["multipart/mixed"]}, + parts: [ + %Mail.Message{ + headers: %{"content-type" => ["multipart/related"]}, + parts: [ + %Mail.Message{ + headers: %{"content-type" => ["multipart/alternative"]}, + parts: [ + %Mail.Message{ + headers: %{"content-type" => ["text/plain", {"charset", "UTF-8"}]}, + body: "Some text", + parts: [], + multipart: false + }, + %Mail.Message{ + headers: %{"content-type" => ["text/html", {"charset", "UTF-8"}]}, + body: "

Some HTML

", + parts: [], + multipart: false + } + ] + }, + %Mail.Message{ + headers: %{ + "content-transfer-encoding" => :base64, + "content-type" => "image/jpeg", + "content-id" => "c_id", + "content-disposition" => ["inline", {:filename, "filename"}], + "x-attachment-id" => "a_id", + }, + parts: [], + multipart: false + } + ] + }, + %Mail.Message{ + headers: %{ + "content-transfer-encoding" => :base64, + "content-type" => ["image/jpeg"] + }, + parts: [], + multipart: false + } + ] + } = message + end end From 52964cc8b1b8efa3af3c82bdbfb60312ff9c5562 Mon Sep 17 00:00:00 2001 From: Jarrod Moldrich Date: Sun, 17 Nov 2024 08:48:21 +1100 Subject: [PATCH 4/4] Remove comment --- test/mail/renderers/rfc_2822_test.exs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/mail/renderers/rfc_2822_test.exs b/test/mail/renderers/rfc_2822_test.exs index 123673a..ef5a846 100644 --- a/test/mail/renderers/rfc_2822_test.exs +++ b/test/mail/renderers/rfc_2822_test.exs @@ -431,14 +431,6 @@ defmodule Mail.Renderers.RFC2822Test do |> Mail.put_html("

Some HTML

") |> Mail.Renderers.RFC2822.reorganize() - # Bamboo.Attachment.new( - # Application.app_dir(:notifications, path), - # filename: filename, - # content_id: "<#{id}>", - # headers: [x_attachment_id: id, content_disposition: ["inline", filename: filename]], - # content_type: "image/#{type}" - # ) - assert %Mail.Message{ headers: %{"content-type" => ["multipart/mixed"]}, parts: [