Skip to content

add failing test for custom headers in multipart message#198

Closed
romsahel wants to merge 2 commits intoDockYard:masterfrom
romsahel:missing-headers-in-multipart-message
Closed

add failing test for custom headers in multipart message#198
romsahel wants to merge 2 commits intoDockYard:masterfrom
romsahel:missing-headers-in-multipart-message

Conversation

@romsahel
Copy link
Copy Markdown
Contributor

@romsahel romsahel commented Apr 23, 2025

Hi,

Following this commit f738af7, we encounter an issue: when rendering a multi-part message, the headers are missing.

This PR adds 2 tests to showcase the problem:

  1. The first test uses a single-part message and succeeds
  2. The second test uses a multi-part message and fails (no headers except for the mime-version are present)

I've attempted a fix in lib/mail/renderers/rfc_2822.ex although I'm unsure if it's the best way to approach the problem.

Comment on lines +323 to +352
%Mail.Message{
headers: %{
"from" => "\"Sender\" <sender@example.com>",
"subject" => "Test Subject",
"to" => [{"", "recipient@example.com"}],
"x-custom-header-1" => "custom-header-value",
"x-custom-header-2" => "custom-header-value"
},
body: nil,
parts: [
%Mail.Message{
headers: %{
"content-transfer-encoding" => :quoted_printable,
"content-type" => ["text/plain", {"charset", "utf-8"}]
},
body: "Test text body",
parts: [],
multipart: false
},
%Mail.Message{
headers: %{
"content-transfer-encoding" => :quoted_printable,
"content-type" => ["text/html", {"charset", "UTF-8"}]
},
body: "<html>html body</html>",
parts: [],
multipart: false
}
],
multipart: true
Copy link
Copy Markdown
Contributor Author

@romsahel romsahel Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We build the message using a combination of build_multipart |> put_from |> put_to |> put_header and using it in production, we had no issue before.

@andrewtimberlake
Copy link
Copy Markdown
Collaborator

Sorry about the regression. I’ve pushed a fix to master. Please check that works for you.

@romsahel
Copy link
Copy Markdown
Contributor Author

Excellent! Tested and it works for us, thanks a lot 🥇

@andrewtimberlake
Copy link
Copy Markdown
Collaborator

I have released 0.5.1 with the regression fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants