RFC 2822 strict

[bcardarella]

Nearly all of the issues that have been opened up recently are related to relaxing the RFC 2822 parser, which makes sense because there are so many implementations that go outside the spec. However, I feel strongly there is also a need for a strict parser for certain use-caes. I think we should support a strict: true parser option that will strictly enforce the spec. The default should be permissive as the module has moved toward as this is where more of the use-cases will be.

@andrewtimberlake thoughts?

[andrewtimberlake]

I disagree. Many of the latest fixes have been bug fixes in incorrect handling of certain RFCs.
The only relaxed handling we support is in date parsing, which seems to be the most loose part of email encoding.
Also, while elixir-mail is based on RFC 2822, technically that RFC is a replacement of RFC 822, and is itself an obsolete RFC that has been replaced by RFC 5322
Also there are multiple other RFCs that have to come into play in correctly handling email (like RFC 2047 for encoded words, RFC 2046 for mime)
If I‘ve missed this, can you point to the places where elixir-mail has become relaxed outside of date handling?

From my point of view, and this is similar to HTML, an email parser should be able to parse any email found on the internet and produce a sane working result.
On the other hand, an email encoder should follow strict rules to the letter.
I think we should have a relaxed parser and a strict encoder.

What use cases do you think we need a strict parser for? If we do this, then we need a good way of reporting failures that always point back to the spec—I personally don’t think this is worth it.

[btihen]

I agree we need some kind of solution. We here in Switzerland work with several languages that use Extended ASCII (ü, ß and ç, etc). On top of that, we have many (most clients) that use Outlook and they send Extended ASCII encoded in latin1 -

I see that these are sometimes handled, but we are having problems where mail and mailroom crash (or pass through unencoded subjects and filenames for attachments) and at least with subject and filenames it causes crashes.

2025-06-06T12:44:36Z app[9185953a72e548] fra [info]12:44:36.031 [error] Error processing %{from: ["xxxxx@xxx.xx"], cc: [], to: ["xxx.xxx@xxx.xxx.xx"], headers: %{}, subject: "=?iso-8859-1?Q?WG:_Faktura_0625.28_Friesstrasse_28,_8050_Z=FCrich_?=", reply_to: ["xxxx@xxx.xx"], bcc: [], has_attachment: false, recipients: ["xxx.xxx@xxx.xxx.xx"]} -> :error, %ErlangError{original: {:invalid_byte, 252}, reason: nil},     (logger 1.18.3) lib/logger.ex:963: Logger.__do_log__/4
2025-06-06T12:44:36Z app[9185953a72e548] fra [info]    (avk_service 1.0.9) deps/mailroom/lib/mailroom/inbox.ex:232: anonymous fn/3 in AvkService.Boundary.ImapClient.process_mailbox/2
2025-06-06T12:44:36Z app[9185953a72e548] fra [info]    (elixir 1.18.3) lib/enum.ex:987: Enum."-each/2-lists^foreach/1-0-"/2
2025-06-06T12:44:36Z app[9185953a72e548] fra [info]    (mailroom 0.5.2) lib/mailroom/imap.ex:167: Mailroom.IMAP.search/4
2025-06-06T12:44:36Z app[9185953a72e548] fra [info]    (avk_service 1.0.9) deps/mailroom/lib/mailroom/inbox.ex:204: AvkService.Boundary.ImapClient.process_mailbox/2

and


2025-06-06 13:15:25.058	11:15:25.057 [error] Error processing %{from: ["xxxx@xxx.xx"], cc: [], to: ["xxx.xxx@xxx.xxx.xxx"], headers: %{}, subject: "=?iso-8859-1?Q?Per_E-Mail_senden:_Rechnung_Hauswartung_Juni_-_Wonneberg_Z?= =?iso-8859-1?Q?=FCri-31-32,_Rechnung_Hauswartung_Juni_-_Wonneberg_Z=FCri-?= =?iso-8859-1?Q?33-34,_Rechnung_Hauswartung_Juni_-_Wonneberg_Z=FCri-35-36,?= =?iso-8859-1?Q?_Rechnung_Hauswartung_Juni_-_Wonneberg_Z=FCri-37-38?=", reply_to: ["xxxx@xxx.xx"], bcc: [], has_attachment: false, recipients: ["xxx.xxx@xxx.xxx.xx"]} -> :error, %ErlangError{original: {:invalid_byte, 252}, reason: nil},     (logger 1.18.3) lib/logger.ex:963: Logger.__do_log__/4
2025-06-06 13:14:59.257	11:14:59.256 [error] Error processing %{from: ["xxxx@xxx.xx"], cc: [], to: ["xxx.xxx@xxx.xxx.xxx"], headers: %{}, subject: "=?iso-8859-1?Q?Per_E-Mail_senden:_Rechnung_Hauswartung_Juni_-_Wonneberg_Z?= =?iso-8859-1?Q?=FCri-17-18,_Rechnung_Hauswartung_Juni_-_Wonneberg_Z=FCri-?= =?iso-8859-1?Q?19-20,_Rechnung_Hauswartung_Juni_-_Wonneberg_Z=FCri-21-22,?= =?iso-8859-1?Q?_Rechnung_Hauswartung_Juni_-_Wonneberg_Z=FCri-23-24,_Rechn?= =?iso-8859-1?Q?ung_Hauswartung_Juni_-_Wonneber?=", reply_to: ["xxxx@xxx.xx"], bcc: [], has_attachment: false, recipients: ["xxx.xxx@xxx.xxx.xx"]} -> :error, %ErlangError{original: {:invalid_byte, 252}, reason: nil},     (logger 1.18.3) lib/logger.ex:963: Logger.__do_log__/4

The same thing happens with filenames, but when these don't get handled in mail, then mailroom can also crash with.

I am working on a fix, but I am also confused by the tests allowing / expecting escape character - I am also unsure why it looks like iso-8859-1 is tested, but crashes. I am happy to help, but we here in CH with lots of languages and lots of windows outlook users need a reliable safe fix. I can provide problem emails. I am also glad (would prefer) to help with fixes that help with multilingual solutions like here in CH.

[bcardarella]

@btihen if you could provide some failing test cases it would be a place we can start to look. As far as the test case on your end failing I'm not sure, our CI is passing and I just ran the tests locally with out issue.

[andrewtimberlake]

@btihen happy to help if you can give us some examples. Have you seen that you can pass in a charset decoding function to handle multi language encoding (see #178)
Mailroom uses elixir-mail but is its own project so feel free to add a bug report there if needed.
Feel free to send me copies of emails you don’t want publicly available but show the problems. I’ll work out a generic test case from those and discard them as soon as we’ve found a fix.
Please start new issues to address any of the above so we keep things on topic.

[btihen]

@bcardarella and @andrewtimberlake - thanks, I am trying to test and narrow the problem with the problematic emails. They are include customer information so I can't post them as is. So I am trying to learn enough to make a 'fake' problematic email. It seems like it is happening within: Mail.Parsers.RFC2822

It seems like if I run my emails directly through:

    raw_email = File.read!("test/fixtures/problem_source_email_one_pdf.txt")
    message = Mail.Parsers.RFC2822.parse(raw_email)

it blows up, but if I do what you do in the tests

    raw_email = File.read!("test/fixtures/problem_source_email_one_pdf.txt")
    message = raw_email |> String.replace("\n", "\r\n") |> Mail.Parsers.RFC2822.parse(raw_email)

Perhaps our code fetches email differently than I expect - we use - where we start AvkService.Boundary.ImapClient as a child process within phoenix - this is when we get the email: subject: "=?iso-8859-1?Q?WG:_Faktura_0625.28_Friesstrasse_28,_8050_Z=FCrich_?=", has_attachment: false, recipients: ["xxx.xxx@xxx.xxx.xx"]} -> :error, %ErlangError{original: {:invalid_byte, 252}, reason: nil} - these emails do have attachments, Ideally the subject would resolve to: WG:_Faktura_0625.28_Friesstrasse_28,_8050_Zürich - I am having troubles getting the same error and traceback without pulling from the imap server.

defmodule AvkService.Boundary.ImapClient do
  @moduledoc """
  IMAP Client Genserver
  """

  use Mailroom.Inbox
  require Logger

  alias AvkService.Mail.{Parser, Processor}
  alias AvkService.Util.Parser, as: UtilParser

  match do
    # Previous matching is done on headers and IMAP envelope, if the match succeeds and you need the body,
    # then this will retrieve the message body
    fetch_mail
    process(AvkService.Boundary.ImapClient, :process_email)
  end

  def config(_opts) do
    [
      folder: :inbox,
      ssl: true,
      ssl_opts: [verify: :verify_none]
    ]
    |> Keyword.merge(Application.get_env(:avk_service, :mailroom, []))
  end

  def process_email(%Mailroom.Inbox.MessageContext{message: message}) do
    with {:ok, :no_spam} <- Parser.check_spam(message),
         {:ok, email_address} <- email_address(message),
         {:ok, accounting_reference} <- maybe_accounting_reference_from_subject(message),
         attachments <- Mail.get_attachments(message) do
      Processor.process(email_address, accounting_reference, attachments)
    else
      {:error, reason} -> Logger.error(reason)
    end
  end

  def maybe_accounting_reference_from_subject(message),
    do: Mail.Message.get_header(message, "subject") |> UtilParser.parse_accounting_hint()

  defp email_address(message) do
    case Mail.Message.get_header(message, "from") do
      {_name, email_address} ->
        {:ok, email_address}

      unexpected ->
        {:error, "Mail.Processor: cannot decode email address from email header: #{unexpected}"}
    end
  end
end

PS - I sent @andrewtimberlake directly two problematic emails. Thanks for your help.

[btihen]

@bcardarella - sent the problematic emails to @andrewtimberlake, since I can't post them here. The crash is happening in mailroom, but as I suspected, it is probably because of non-utf-8 subject contents. It's not clear to me if mail or mailroom would be the best place to address this. mail seems to already do some utf-8 cleanup in the parser:

  defp decode(body, message, opts) do
    body = String.trim_trailing(body)
    content_type = message.headers["content-type"]
    charset = Mail.Proplist.get(content_type, "charset")
    transfer_encoding = Mail.Message.get_header(message, "content-transfer-encoding")
    decoded = Mail.Encoder.decode(body, transfer_encoding)

    if charset do
      charset_handler = Keyword.get(opts, :charset_handler, fn charset, string ->
        # Always convert known charsets to UTF-8 consistently
        normalized_charset = String.downcase(charset)

        cond do
          # Latin-based charsets (ISO-8859-1, Latin1)
          normalized_charset in ["iso-8859-1", "latin1"] ->
            :unicode.characters_to_binary(string, :latin1, :utf8)

          # Windows charsets (Windows-1252, Windows-1258, etc)
          # Use Latin1 encoding for all Windows charsets as a best effort
          # This may not be perfect but avoids special casing
          String.contains?(normalized_charset, "windows") ->
            :unicode.characters_to_binary(string, :latin1, :utf8)

          # For all other cases, keep as is
          true -> string
        end
      end)
      charset_handler.(charset, decoded)
    else
      decoded
    end
  end