Repository navigation
fix(email): partition an attached email (message/rfc822) instead of raising AssertionError - #4597
Conversation
…aising AssertionError partition_email() processes attachments by default. For a message/rfc822 part (an email forwarded as an attachment, the returned message in a bounce, each part of a multipart/digest) get_content() returns the enclosed message object rather than str or bytes, so _AttachmentPartitioner._file_bytes failed its assert and the whole call raised AssertionError, losing the body elements too. Serialize the enclosed message with as_bytes() and pass content_type="message/rfc822" for such parts, so the attachment is partitioned as an email even when its first header is one libmagic does not recognize (e.g. MIME-Version), instead of emitting its raw headers as text.
cragwolfe
left a comment
cragwolfe
left a comment
There was a problem hiding this comment.
GPT Pro reviewed c9632d5b2981aa909194dde53574bebceaa36704 against c74e6c4c50e58c49778a458745d06c248add2db3.
P2 — Preserve raw binary descendants when serializing the enclosed email (unstructured/partition/email.py:493–494).
content.as_bytes() serializes the entire enclosed MIME tree with the default LF line separator. In Python 3.12, BytesGenerator sends image/application payloads through _writeBody / _handle_text and _write_lines, which normalizes CRLF and bare CR. A forwarded email with an image attachment using Content-Transfer-Encoding: binary therefore loses the original bytes: a PNG signature beginning 89 50 4e 47 0d 0a 1a 0a becomes 89 50 4e 47 0a 1a 0a before the child attachment is partitioned. Base64 attachments do not have this problem.
This is a gap in the newly supported path; the same input previously raised AssertionError. The existing multipart serialization helper has a similar limitation, but that preexisting path is separate from this finding.
Please preserve the decoded binary bytes across the attached-message boundary, for example by processing the parsed child message or safely transfer-encoding binary leaves on a copy before serialization. Switching to CRLF alone cannot preserve arbitrary binary bytes. Add a raw MIME fixture with CRLF, bare CR, bare LF and non-ASCII payload bytes, and assert exact descendant bytes across one and two attached-message levels, with a base64 control. Construct the fixture from raw bytes so the fixture builder does not apply the same lossy serialization.
The MIME hint and its isolation from sibling attachment kwargs look correct. No intentional breaking API change was identified. The two new tests cover the reported assertion and routing issue. No local tests were run for this review. At the CI observation, this head had 47 successful checks and three in progress; this is not a merge-readiness claim. Neither release-managed file is changed.
(authored by codex)
Serializing the enclosed email of a message/rfc822 part with as_bytes() rewrites each CR in a part's body, alone or in a CRLF, as LF. That is harmless for Base64, which ignores line breaks, but it corrupts binary data in any other part, for example a PNG image sent with Content-Transfer-Encoding binary, 8bit or none (7bit). On a copy of the enclosed email, each non-multipart part that is not in Base64 and whose decoded bytes contain a CR is now re-encoded in Base64 from those bytes before serializing. Each part then reaches its partitioner with the same bytes as when the enclosed email is partitioned on its own. The parsed email is not changed. A test builds raw MIME bytes with an image attached as binary, without a transfer-encoding header and in Base64 (the control), one and two attached-message levels deep, and checks the exact bytes each attachment's partitioner receives.
cragwolfe
left a comment
cragwolfe
left a comment
There was a problem hiding this comment.
GPT Pro reviewed the complete final diff at 01c9d4b094dbb36565b5d1c60e42c2e454395715 against c74e6c4c50e58c49778a458745d06c248add2db3, with surrounding source, tests, and the previous review. No concrete introduced correctness defect or breaking compatibility change was found.
The previous P2 is resolved for the attached message/rfc822 path. _serialize_message() walks a copy of the complete enclosed MIME tree and Base64-encodes non-Base64 leaves whose decoded bytes contain carriage returns before serialization. The raw-byte tests exercise binary and missing transfer-encoding headers, Base64 controls, CRLF, bare CR, bare LF, and non-ASCII bytes across one and two attached-message levels. The MIME hint remains isolated from sibling kwargs. The separate, unchanged multipart/* serializer is outside this fix's byte-preservation guarantee.
A useful follow-up test would attach a quoted-printable non-UTF-8 text body containing =0D and soft wrapping, with a no-CR control, and assert the resulting Unicode text. Source inspection preserves the decoded bytes and charset; this suggestion does not identify a current failure.
No local tests were run. At this head, 50 checks succeeded; CI, CodeQL, and Partition Benchmark workflows passed, while Security is action_required. This source approval does not establish integration readiness. Neither release-managed file changed; uv.lock and CHANGELOG.md were excluded from the Pro bundle.
(authored by codex)
Summary
partition_email()processes attachments by default. When a message has amessage/rfc822part, the whole call fails with a bareAssertionErrorandthe body elements are lost too. Common inputs have such a part:
multipart/reportwith the returned message),multipart/digest, where every part ismessage/rfc822.The repository's own fixture
example-docs/eml/mime-multipart-digest.emlfails this way, through
partition()too.Cause: for a
message/rfc822part,get_content()returns the enclosedEmailMessage, notstrorbytes, so_AttachmentPartitioner._file_bytesfails its
assert isinstance(content, bytes). The assert sits outside thetry/exceptthat skips attachments it cannot partition.Fix, in
unstructured/partition/email.py:_file_bytesserializes an enclosed message withas_bytes().message/rfc822part,content_type="message/rfc822"is added to thekwargs of the nested
partition()call. Without it the bytes go throughlibmagic, which reports a message as
text/plainwhen its first header ise.g.
MIME-Version,Message-IDorToand notFrom/Received/Date.The attachment's raw headers then come out as
Titleelements. Python'sEmailMessagewrites such messages: the forwarded message in the new teststarts with
Content-Type, which libmagic does not recognize either. Thehint is merged into the kwargs only for these parts, so a
content_type=kwarg passed to
partition_email()directly still reaches the otherattachments as before instead of colliding with an explicit keyword.
Related:
_file_bytesfunction (theKeyErroron a
multipart/*attachment). Its comment already listsmessage/rfc822among the types
get_content()handles, but that path still hit the assert.process_attachments=Truefailing on a nested.eml, with aTypeErrorinthe implementation of that time. A maintainer asked there for an email that
reproduces it so a test could guard the behavior. The tests here use two
such emails: the existing digest fixture and a forwarded message built in
the test.
Validation
Environment:
mainat c74e6c4, Python 3.12,uv sync --locked --group test(no extras; libmagic installed).New tests in
test_unstructured/partition/test_email.py(attachments section):test_partition_email_partitions_the_messages_of_a_multipart_digest: usesthe existing
mime-multipart-digest.emlfixture. It checks the threemessages' text, their
subjectmetadata andattached_to_filename.test_partition_email_partitions_a_message_forwarded_as_an_attachment: anEmailMessageforwarded withadd_attachment()whose first header isContent-Type. It checks the body, the forwarded text,subjectandsent_from.Results:
pytest test_unstructured/partition/test_email.py -k "multipart_digest or forwarded_as_an_attachment"gives 2 failed, both
AssertionErroratunstructured/partition/email.py:485(
assert isinstance(content, bytes)). The whole file without the fix gives2 failed, 77 passed, 1 skipped.
as_bytes()change and nocontent_typehint, the digesttest passes and the forwarded-message test fails with 6 extra
Titleelements (the forwarded message's raw headers). That is why the hint is
part of the fix.
pytest test_unstructured/partition/test_email.py: 79 passed, 1 skipped.The skip is the WEBP test that needs
unstructured_inference.pytest test_unstructured/file_utils/test_filetype.py test_unstructured/partition/test_msg.py:362 passed, 1 failed. The failure is
test_partition_msg_can_process_attachments, which fails the same way onmainbecause thepptxextra is not installed.msg.pyhas its ownattachment partitioner and is not changed.
multipart/reportwithtext/plain,message/delivery-statusand amessage/rfc822part whose first header isMIME-Version): returns the bounce text, the delivery-status lines and thereturned message's body with
subject == "Lunch". Before the fix it raisedAssertionError. A forward of a forward is also partitioned at every level.make check(ruff 0.15.10 from thelintgroup):All checks passed!,355 files already formatted.bash scripts/test-check-new-release-version.sh:Passed 27 version guard tests.VERSION_BASE_REF=upstream/main bash scripts/check-new-release-version.sh:PASS: unchanged development version 0.27.35-dev (no tag lookup).CHANGELOG.mdorunstructured/__version__.pychanges.Release note
partition_email(), andpartition()on an.emlfile, now partition anattached email (a message forwarded as an attachment, the returned message in
a bounce, each message of a
multipart/digest) like any other attachment.Before, they raised
AssertionError. Each element from the attached messagecarries that message's own
subject,sent_fromand other email metadata.Checklist
enhancement,bug, or an appropriate existing label), or ask a maintainer to apply it. A maintainer needs to applybug.!in the title and explain impact and migration above. (Not a breaking change.)