Skip to content

Eliminate attachment corruption caused by line ending conversions - #1113

Closed
jeremy wants to merge 1 commit into
mikel:masterfrom
jeremy:crlf-conversion
Closed

jeremy wants to merge 1 commit into
mikel:masterfrom
jeremy:crlf-conversion

Conversation

@jeremy

@jeremy jeremy commented May 22, 2017

Copy link
Copy Markdown
Collaborator

* 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
jeremy force-pushed the crlf-conversion branch from c0e6fd5 to cc9a65e Compare May 22, 2017 18:43
@jeremy jeremy closed this in 21222e1 May 22, 2017
@jeremy
jeremy deleted the crlf-conversion branch May 22, 2017 20:13
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
jeremy added a commit 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 #1129
References #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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Broken images when parsing emails Corrupted binary parts

1 participant