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

Fixing #408 by not standarizing whitespace #609

Closed
wants to merge 3 commits into from

Conversation

malmckay
Copy link

There may be a better fix, that can standardize line breaks that are outside of the binary blob, but I feel this is a good step forward

Parsing binary encoded attachments still fails for most jpgs in 1.8.7 (1.9+ is fine). Without this patch most binary encoded attachments fail in all versions, so, again, I feel this is good step forward.

PS This is a repeat pull request, my previous one was from HEAD, which included other commits

There may be a better fix, that can standardize whitespace outside of the binary blob, but I feel this is a good step forward
@ConradIrwin
Copy link
Collaborator

@malmckay What does this do to nominally plain-text emails that have been sent with a Binary content-type but with "\n" as a line-break? Do we actually want to look at the content type as well?

@malmckay
Copy link
Author

@ConradIrwin Good point. I looked at that in a slightly different way: If content-type is text and transfer-encoding is binary should we standardize line-breaks in the email body to CRLF?

I think not. Mail.gem's parsing (by this point) is not affected, and I don't believe Mail.gem users should assume CRLF for the body.

@jeremy
Copy link
Collaborator

jeremy commented May 22, 2017

Folded in to #1113 😊

@jeremy jeremy closed this May 22, 2017
@jeremy jeremy added this to the 2.7.0 milestone May 22, 2017
jeremy added a commit that referenced this pull request May 22, 2017
* Omit initial CRLF linefeed conversion since CRLF are required newline
  separators. We shouldn't need to convert bare CR or LF. Update our
  fixture emails to use CRLF throughout. Closes #609. Fixes #408.

* Drop quoted-printable CRLF conversion. This was introduced to
  harmonize with Ruby's \n-based line endings. But this breaks Q-P
  encoding with binary data. It's not *meant* for binary data, but we
  don't yet take adequate measures to use base64 for these cases.
  Reverts #496. Fixes #1010.

Closes #1113
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