Skip to content
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

Allowing multipart messages to be split when they are separated by LF only instead of CRLF #1287

Closed
wants to merge 1 commit into from
Closed

Allowing multipart messages to be split when they are separated by LF only instead of CRLF #1287

wants to merge 1 commit into from

Conversation

ppadron
Copy link

@ppadron ppadron commented Nov 1, 2018

Some multipart messages use \n only instead of \r\n in their boundaries. Ran into this while parsing feedback loop reports sent by Return Path.

@jeremy
Copy link
Collaborator

jeremy commented Nov 2, 2018

Thanks @ppadron! We convert line endings to \n before parsing which covers this case. Could you share an example of where it fails, including the raw email, and on which version of the Mail library?

@ppadron
Copy link
Author

ppadron commented Nov 9, 2018

Hi @jeremy! I created a repository with a proof of concept for this issue: https://github.com/ppadron/mail-poc-pr-1287. In the Gemfile you can switch the mail version and see that in 2.7.1 it detects only one part, and with the fix it splits the email in 3 parts.

@sebbASF
Copy link
Collaborator

sebbASF commented Feb 13, 2019

See #1319 which has the same fix, but also removes the conversion to CRLF (*).
It also adds a test case that fails without the change.

(*) The conversion does not happen anyway unless the mail string is ascii_only, which is not the case for the failing test cases.

@jeremy
Copy link
Collaborator

jeremy commented May 31, 2019

Going ahead with the additional changes in #1319. Thanks again @ppadron!

@jeremy jeremy closed this May 31, 2019
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.

3 participants