search

DockYard / elixir-mail

Build composable mail messages
388 stars 61 forks source link

Incorrect Test Assertion #117

Closed TylerPachal closed 2 years ago

TylerPachal commented 4 years ago

While I was working on https://github.com/DockYard/elixir-mail/pull/116, I noticed the following test:

https://github.com/DockYard/elixir-mail/blob/31189ca96d6f99ff9fe2eb7f2d31ff2acb23ab5d/test/mail/parsers/rfc_2822_test.exs#L612-L619

If I understand this test correctly, it is meant to test that if you omit the charset= parameter prefix that we will still parse the charset. Thus, shouldn't the assertion statement look like this?

assert message.headers["content-type"] == ["text/html", {"charset", "us-ascii"}]
TylerPachal commented 4 years ago

If this is indeed broken, I have a commit I can open as a PR to fix this.

andrewtimberlake commented 2 years ago

[RFC 2045 §5.2] states that when no content type is specified, us-ascii should be considered as the default. If I understand the spec correctly, that content-type header is actually invalid.

Currently, that test has been updated with an explicit charset: https://github.com/DockYard/elixir-mail/blob/fa9ddf50ddc94cbb19e581873d4d7991ea5fdac0/test/mail/parsers/rfc_2822_test.exs#L614-L621

I have added pull request #144 which could address this. @bcardarella Let me know what your thoughts are on handling this.

bcardarella commented 2 years ago

Closed by #144