From b115aef414b2e95f7db0a8ab9fbd977d300fe368 Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Thu, 7 Jul 2022 10:43:00 +0530 Subject: [PATCH] fix: sanitize all line boundaries for email headers (#17408) * test: add test case for subject with LF, CR and line separator --- frappe/email/email_body.py | 21 +++++++++++++++------ frappe/email/test_email_body.py | 21 ++++++++++----------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index 3288ca2148..b493ca2cb5 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -332,12 +332,12 @@ class EMail: def set_header(self, key, value): if key in self.msg_root: + # delete key if found + # this is done because adding the same key doesn't override + # the existing key, rather appends another header with same key. del self.msg_root[key] - try: - self.msg_root[key] = value - except ValueError: - self.msg_root[key] = sanitize_email_header(value) + self.msg_root[key] = sanitize_email_header(value) def as_string(self): """validate, build message and convert to string""" @@ -580,8 +580,17 @@ def get_header(header=None): return email_header -def sanitize_email_header(str): - return str.replace("\r", "").replace("\n", "") +def sanitize_email_header(header: str): + """ + Removes all line boundaries in the headers. + + Email Policy (python's std) has some bugs in it which uses splitlines + and raises ValueError (ref: https://github.com/python/cpython/blob/main/Lib/email/policy.py#L143). + Hence removing all line boundaries while sanitization of headers to prevent such faliures. + The line boundaries which are removed can be found here: https://docs.python.org/3/library/stdtypes.html#str.splitlines + """ + + return "".join(header.splitlines()) def get_brand_logo(email_account): diff --git a/frappe/email/test_email_body.py b/frappe/email/test_email_body.py index 9f76ec6a59..3ff4245924 100644 --- a/frappe/email/test_email_body.py +++ b/frappe/email/test_email_body.py @@ -152,20 +152,19 @@ w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

Hey John Doe!

This is embedded image you asked for

""" - email_string = ( - get_email( - recipients=["test@example.com"], - sender="me@example.com", - subject="Test Subject", - content=email_html, - header=["Email Title", "green"], - ) - .as_string() - .replace("\r\n", "\n") - ) + email_string = get_email( + recipients=["test@example.com"], + sender="me@example.com", + subject="Test Subject\u2028, with line break, \nand Line feed \rand carriage return.", + content=email_html, + header=["Email Title", "green"], + ).as_string() # REDESIGN-TODO: Add style for indicators in email self.assertTrue("""""" in email_string) self.assertTrue("Email Title" in email_string) + self.assertIn( + "Subject: Test Subject, with line break, and Line feed and carriage return.", email_string + ) def test_get_email_header(self): html = get_header(["This is test", "orange"])