-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/inline attachments #137
base: master
Are you sure you want to change the base?
Changes from all commits
ad00407
25be71e
4604564
f770cbe
030b0b9
52964cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -251,30 +251,83 @@ defmodule Mail.Renderers.RFC2822 do | |
|> String.pad_leading(2, "0") | ||
end | ||
|
||
defp reorganize(%Mail.Message{multipart: true} = message) do | ||
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 | ||
|
||
@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 | ||
<text and html parts> | ||
end multipart/alternative | ||
<inline parts> | ||
end multipart/related | ||
<attachment parts> | ||
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) | ||
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might it not be more explicit to use |
||
has_mixed_parts = Enum.any?(mixed) | ||
has_text_parts = Enum.any?(text_parts) | ||
|
||
if has_inline || has_mixed_parts do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need multipart/mixed if there are only inline attachments? Shouldn’t it then just be wrapped in related? |
||
# 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)) | ||
|
||
mixed_part = | ||
if has_text_parts do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if it’s only text/html with inline attachments and no text/plain? Then there’s no alternative, right? |
||
# If any text with attachments, wrap in new part | ||
body_part = | ||
Mail.build_multipart() | ||
|> 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 | ||
|
||
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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -339,4 +339,143 @@ 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("[email protected]") | ||
|> Mail.put_from({"User2", "[email protected]"}) | ||
|> Mail.put_subject("Test email") | ||
|> Mail.put_text("Some text") | ||
|> Mail.put_html("<h1>Some HTML</h1>") | ||
|> 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: "<h1>Some HTML</h1>", | ||
parts: [], | ||
multipart: false | ||
} | ||
] | ||
} = message | ||
end | ||
|
||
test "will have correct part order with only a regular attachment" do | ||
message = | ||
Mail.build_multipart() | ||
|> Mail.put_to("[email protected]") | ||
|> Mail.put_from({"User2", "[email protected]"}) | ||
|> Mail.put_subject("Test email") | ||
|> Mail.put_attachment({"tiny_jpeg.jpg", @tiny_jpeg_binary}) | ||
|> Mail.put_text("Some text") | ||
|> Mail.put_html("<h1>Some HTML</h1>") | ||
|> 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: "<h1>Some HTML</h1>", | ||
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("[email protected]") | ||
|> Mail.put_from({"User2", "[email protected]"}) | ||
|> 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("<h1>Some HTML</h1>") | ||
|> Mail.Renderers.RFC2822.reorganize() | ||
|
||
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: "<h1>Some HTML</h1>", | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need tests for
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be better to work with a tuple of lists
{[], [], []}
to be more explicit about the number of lists being generated?