Skip to content

Commit fdf13ca

Browse files
committed
gh-157254: Preserve email content on failure and reject unsupported bytes CTEs
Failed content setup, such as an unknown charset, can discard an existing message's content or leave its parent converted to multipart. Restore the original headers and payload when a content handler raises, and prepare new parts before converting their parent while preserving conversion error precedence. Reject unsupported bytes content-transfer encodings with ValueError instead of emitting an unrecognized Content-Transfer-Encoding header.
1 parent c700121 commit fdf13ca

5 files changed

Lines changed: 129 additions & 9 deletions

File tree

Lib/email/contentmanager.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,15 @@ def set_content(self, msg, obj, *args, **kw):
3434
# but we can't add it later, so do it for now.
3535
raise TypeError("set_content not valid on multipart")
3636
handler = self._find_set_handler(msg, obj)
37-
msg.clear_content()
38-
handler(msg, obj, *args, **kw)
37+
headers = msg._headers
38+
payload = msg._payload
39+
try:
40+
msg.clear_content()
41+
handler(msg, obj, *args, **kw)
42+
except BaseException:
43+
msg._headers = headers
44+
msg._payload = payload
45+
raise
3946

4047
def _find_set_handler(self, msg, obj):
4148
full_path_for_error = None
@@ -234,6 +241,8 @@ def set_bytes_content(msg, data, maintype, subtype, cte='base64',
234241
data = data.decode('ascii')
235242
elif cte in ('8bit', 'binary'):
236243
data = data.decode('ascii', 'surrogateescape')
244+
else:
245+
raise ValueError("Unknown content transfer encoding {}".format(cte))
237246
msg.set_payload(data)
238247
msg['Content-Transfer-Encoding'] = cte
239248
_finalize_set(msg, disposition, filename, cid, params)

Lib/email/message.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,13 +1144,22 @@ def set_content(self, *args, content_manager=None, **kw):
11441144
content_manager = self.policy.content_manager
11451145
content_manager.set_content(self, *args, **kw)
11461146

1147-
def _make_multipart(self, subtype, disallowed_subtypes, boundary):
1147+
_multipart_disallowed_subtypes = {
1148+
'related': ('alternative', 'mixed'),
1149+
'alternative': ('mixed',),
1150+
'mixed': (),
1151+
}
1152+
1153+
def _check_multipart_conversion(self, subtype, disallowed_subtypes):
11481154
if self.get_content_maintype() == 'multipart':
11491155
existing_subtype = self.get_content_subtype()
11501156
disallowed_subtypes = disallowed_subtypes + (subtype,)
11511157
if existing_subtype in disallowed_subtypes:
11521158
raise ValueError("Cannot convert {} to {}".format(
11531159
existing_subtype, subtype))
1160+
1161+
def _make_multipart(self, subtype, disallowed_subtypes, boundary):
1162+
self._check_multipart_conversion(subtype, disallowed_subtypes)
11541163
keep_headers = []
11551164
part_headers = []
11561165
for name, value in self._headers:
@@ -1172,22 +1181,30 @@ def _make_multipart(self, subtype, disallowed_subtypes, boundary):
11721181
self.set_param('boundary', boundary)
11731182

11741183
def make_related(self, boundary=None):
1175-
self._make_multipart('related', ('alternative', 'mixed'), boundary)
1184+
self._make_multipart(
1185+
'related', self._multipart_disallowed_subtypes['related'], boundary)
11761186

11771187
def make_alternative(self, boundary=None):
1178-
self._make_multipart('alternative', ('mixed',), boundary)
1188+
self._make_multipart(
1189+
'alternative', self._multipart_disallowed_subtypes['alternative'],
1190+
boundary)
11791191

11801192
def make_mixed(self, boundary=None):
1181-
self._make_multipart('mixed', (), boundary)
1193+
self._make_multipart(
1194+
'mixed', self._multipart_disallowed_subtypes['mixed'], boundary)
11821195

11831196
def _add_multipart(self, _subtype, *args, _disp=None, **kw):
1184-
if (self.get_content_maintype() != 'multipart' or
1185-
self.get_content_subtype() != _subtype):
1186-
getattr(self, 'make_' + _subtype)()
1197+
needs_conversion = (self.get_content_maintype() != 'multipart' or
1198+
self.get_content_subtype() != _subtype)
1199+
if needs_conversion:
1200+
self._check_multipart_conversion(
1201+
_subtype, self._multipart_disallowed_subtypes[_subtype])
11871202
part = type(self)(policy=self.policy)
11881203
part.set_content(*args, **kw)
11891204
if _disp and 'content-disposition' not in part:
11901205
part['Content-Disposition'] = _disp
1206+
if needs_conversion:
1207+
getattr(self, 'make_' + _subtype)()
11911208
self.attach(part)
11921209

11931210
def add_related(self, *args, **kw):

Lib/test/test_email/test_contentmanager.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,20 @@ def test_set_content_calls_clear_content(self):
131131
self.assertEqual(m['To'], 'test')
132132
self.assertIsNone(m.get_payload())
133133

134+
def test_set_content_base_exception_preserves_content(self):
135+
def handler(msg, obj):
136+
msg['X-New'] = 'new'
137+
msg.set_payload(obj)
138+
raise KeyboardInterrupt
139+
cm = ContentManager()
140+
cm.add_set_handler(str, handler)
141+
m = self._make_message()
142+
m.set_content('original')
143+
original = m.as_bytes()
144+
with self.assertRaises(KeyboardInterrupt):
145+
m.set_content('replacement', content_manager=cm)
146+
self.assertEqual(m.as_bytes(), original)
147+
134148

135149
@parameterize
136150
class TestRawDataManager(TestEmailBase):
@@ -145,6 +159,31 @@ class TestRawDataManager(TestEmailBase):
145159
content_manager=raw_data_manager)
146160
message = EmailMessage
147161

162+
content_failure_params = {
163+
'unknown_charset': ('replacement',
164+
{'charset': 'does-not-exist'}, LookupError),
165+
'ascii_charset': ('\N{LATIN SMALL LETTER E WITH ACUTE}',
166+
{'charset': 'ascii'}, UnicodeEncodeError),
167+
'text_cte': ('replacement', {'cte': 'unknown'}, ValueError),
168+
'bytes_7bit': (b'\xff', {'maintype': 'application',
169+
'subtype': 'octet-stream', 'cte': '7bit'},
170+
UnicodeDecodeError),
171+
'header': ('replacement', {'headers': ['Subject: duplicate']},
172+
ValueError),
173+
'cid': ('replacement', {'cid': 'bad\nvalue'}, ValueError),
174+
}
175+
176+
def content_failure_as_set_content_preserves_content(self, content, kw,
177+
error):
178+
m = self._make_message()
179+
m['Subject'] = 'original subject'
180+
m.set_content('original')
181+
original = m.as_bytes()
182+
with self.assertRaises(error):
183+
m.set_content(content, **kw)
184+
self.assertEqual(m.as_bytes(), original)
185+
self.assertFalse(m.is_multipart())
186+
148187
def test_get_text_plain(self):
149188
m = self._str_msg(textwrap.dedent("""\
150189
Content-Type: text/plain
@@ -725,6 +764,17 @@ def test_set_application_octet_stream_with_8bit_cte(self):
725764
self.assertEqual(m.get_payload(decode=True), content)
726765
self.assertEqual(m.get_content(), content)
727766

767+
def test_set_bytes_unknown_cte_raises(self):
768+
m = self._make_message()
769+
m.set_content('original')
770+
original = bytes(m)
771+
with self.assertRaisesRegex(
772+
ValueError,
773+
'Unknown content transfer encoding not-a-transfer-encoding'):
774+
m.set_content(b'abc', 'application', 'octet-stream',
775+
cte='not-a-transfer-encoding')
776+
self.assertEqual(bytes(m), original)
777+
728778
def test_set_headers_from_header_objects(self):
729779
m = self._make_message()
730780
content = "Simple message.\n"

Lib/test/test_email/test_message.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,46 @@ def test_default_content_manager_for_add_comes_from_policy(self):
709709
getattr(m, method)('')
710710
self.assertEqual(str(ar.exception), 'test')
711711

712+
def test_add_multipart_failure_preserves_content(self):
713+
for method, existing_subtypes in (
714+
('add_related', (None, 'related')),
715+
('add_alternative', (None, 'related', 'alternative')),
716+
('add_attachment', (None, 'related', 'mixed'))):
717+
for existing in existing_subtypes:
718+
for content, kw, error in (
719+
('replacement', {'charset': 'does-not-exist'},
720+
LookupError),
721+
(b'abc', {'maintype': 'application',
722+
'subtype': 'octet-stream', 'cte': 'unknown'},
723+
ValueError)):
724+
with self.subTest(method=method, existing=existing, kw=kw):
725+
m = self._make_message()
726+
m.set_content('original')
727+
if existing is not None:
728+
getattr(m, 'make_' + existing)()
729+
original = m.as_bytes()
730+
parts = list(m.iter_parts())
731+
with self.assertRaises(error):
732+
getattr(m, method)(content, **kw)
733+
self.assertEqual(m.as_bytes(), original)
734+
self.assertEqual(m.is_multipart(), existing is not None)
735+
self.assertEqual(list(m.iter_parts()), parts)
736+
737+
def test_add_multipart_checks_conversion_before_content(self):
738+
for existing, target in (('mixed', 'related'),
739+
('mixed', 'alternative'),
740+
('alternative', 'related')):
741+
with self.subTest(existing=existing, target=target):
742+
m = self._make_message()
743+
m.set_content('original')
744+
getattr(m, 'make_' + existing)()
745+
original = m.as_bytes()
746+
with self.assertRaisesRegex(
747+
ValueError, f'Cannot convert {existing} to {target}'):
748+
getattr(m, 'add_' + target)(
749+
'replacement', charset='does-not-exist')
750+
self.assertEqual(m.as_bytes(), original)
751+
712752
def message_as_clear(self, body_parts, attachments, parts, msg):
713753
m = self._str_msg(msg)
714754
m.clear()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Preserve existing message content when built-in :mod:`email` content
2+
handlers fail, and avoid changing the parent MIME structure when preparing a
3+
new part fails. Reject unsupported content transfer encodings for bytes
4+
content.

0 commit comments

Comments
 (0)