Eliminate attachment corruption caused by line ending conversions #1113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 22, 2017
Closed
* 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.
jeremy
added a commit
to jeremy/mail
that referenced
this pull request
Oct 24, 2017
Binary-safe newline conversion ensures that attachments don't get corrupted. Requires that text is ASCII-only. Fixes mikel#1129 References mikel#1113
muhammadn
pushed a commit
to muhammadn/mail
that referenced
this pull request
Jan 30, 2018
instructure-gerrit
pushed a commit
to instructure/canvas-lms
that referenced
this pull request
Apr 17, 2018
this bump includes a change to show line ending characters are treated when processing mail. standard email line ending is \r\n and prior to 2.7.0, this was automatically converted back to \n when decoding a message part. however, in some cases this could corrupt images, and so that conversion is no longer happening. see: mikel/mail#1113 this means that message replies that become discussion or conversation replies will have the \r\n line ending in them. we could restore the previous behavior be manually calling `::Mail::Utilities.to_lf` in incoming mail processor when we pull the decoded message parts, but everything should work just fine with the different line ending. closes COMMS-620 test plan: - regression test incoming mail processing - smoke test outgoing mail Change-Id: I349c3e70b08ecb84ef92e597762c48708d52e800 Reviewed-on: https://gerrit.instructure.com/136259 Reviewed-by: Rob Orton <rob@instructure.com> Tested-by: Jenkins QA-Review: Aaron Kc Hsu <ahsu@instructure.com> Product-Review: Simon Williams <simon@instructure.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 Fixing #408 by not standarizing whitespace #609. Fixes Corrupted binary parts #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 Correct line breaks in quoted-printable encoding #496. Fixes Broken images when parsing emails #1010.