imaplib: must not replace LF or CR by CRLF in literals

[6d455403-7a2f-4b36-bbc4-ae3f94622bf1]

BPO	5430
Nosy	@warsaw, @mcepl, @bitdancer, @rduplain, @JulienPalard, @csabella
PRs	python/cpython#10901
Files	test_imaplib_cr_lf.diff: an attempt at testing IMAP4.append for CR, LF preservation

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['easy', 'type-bug', '3.8', 'expert-email', '3.10', 'library', '3.9'] title = 'imaplib: must not replace LF or CR by CRLF in literals' updated_at = user = 'https://bugs.python.org/memeplex' ``` bugs.python.org fields: ```python activity = actor = 'iritkatriel' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)', 'email'] creation = creator = 'memeplex' dependencies = [] files = ['13789'] hgrepos = [] issue_num = 5430 keywords = ['patch', 'easy'] message_count = 8.0 messages = ['83241', '86572', '86602', '94578', '315575', '315577', '316170', '352292'] nosy_count = 8.0 nosy_names = ['barry', 'mcepl', 'r.david.murray', 'memeplex', 'ron.duplain', 'rajeshsr', 'mdk', 'cheryl.sabella'] pr_nums = ['10901'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue5430' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

[6d455403-7a2f-4b36-bbc4-ae3f94622bf1]

For example, after that "normalization", quoted printable encoded headers (as described at rfc 2047) longer than 76 characters are splitted in two different ill-formed headers because the soft LF line break becomes a "hard" CRLF one. This is clearly wrong.

rfc 2060 specifically allows CR and LF inside literals:

""" A literal is a sequence of zero or more octets (including CR and LF), prefix-quoted with an octet count in the form of an open brace ("{"), the number of octets, close brace ("}"), and CRLF. """

[57e243d5-2c00-4ea4-a015-ea12eb4773e4]

It looks like the IMAP4.append method is responsible for the CRLF substitution (trunk/Lib/imaplib.py).

# defined near top of module:
MapCRLF = re.compile(r'\r\n|\r|\n')

# in append method:
self.literal = MapCRLF.sub(CRLF, message)

I'll work on a test for it this evening.

-Ron

[57e243d5-2c00-4ea4-a015-ea12eb4773e4]

Module imaplib has pretty sparse test code. There is only 1 test case, for imaplib.Time2Internaldate. trunk/Lib/test/test_imaplib.py

The attached patch tests for LF, CR preservation with regard to the IMAP4.append method, but more testing is necessary to make sure LF, CR are preserved in IMAP literals throughout imaplib and that the respective fix doesn't break anything. Mock IMAP interaction may be the way to go for better test cases. Is anyone working on expanding imaplib test coverage?

-Ron

[9192ed0b-5011-49ca-8b07-721ca1d103f5]

Am quite new here; just searching hard to contribute, would like to patch this, if I can go ahead.

Don't we need to patch the original imaplib code also?

just remove the line: self.literal = MapCRLF.sub(CRLF, message) and have: self.literal = message Or am I missing something completely?

[4fce49d7-9c43-4783-b6f9-bd43eb64c326]

Module imaplib has pretty sparse test code.

On that note: would anybody be willing to express in form of (simplified) test case, what's the problem reported here? I am not sure, I follow.

[4fce49d7-9c43-4783-b6f9-bd43eb64c326]

Oh, this is 2.6 bug. This should be closed.

[csabella]

I'm not sure if this is a 2.6/2.7 only issue because the code mentioned in msg86572 still exists, although with a change for bpo-21800 (RFC 6855/UTF-8) applied.

For reference, bpo-25591 expanded the test coverage for the imaplib. However, I don't see any tests related to CR or LF in literals.

[JulienPalard]

It looks like a revert of 47404fffff3e36699786082d0ee6565872d627e1 Which is the fix for https://bugs.python.org/issue723962 which I'm currently reading.