Skip to content

fix: partition_email raises KeyError on a multipart/* attachment - #4424

Merged
cragwolfe merged 5 commits into
Unstructured-IO:mainfrom
mittalpk:fix/partition-email-multipart-attachment-keyerror
Oct 15, 2026
Merged

cragwolfe merged 5 commits into
Unstructured-IO:mainfrom
mittalpk:fix/partition-email-multipart-attachment-keyerror

Conversation

@mittalpk

@mittalpk mittalpk commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3922

What's wrong

partition_email(..., process_attachments=True) crashes with an uncaught KeyError on emails containing a PGP-signed (or any other multipart/*-typed) attachment — matching the traceback in #3922 exactly.

Root cause: Python's email.contentmanager has no get_content() handler registered for any multipart/* content-type (handlers exist for text/*, application/*, image/*, message/rfc822, etc., but multipart sub-parts are normally consumed via .iter_parts(), not .get_content()). A multipart sub-part can still surface as an "attachment" via EmailMessage.iter_attachments() though — e.g. a PGP/MIME-signed forwarded message (multipart/signed) nested inside a multipart/mixed envelope (the linked testcase.txt scenario), or a multipart/mixed part itself (the exact KeyError: 'multipart/mixed' in the original traceback).

_AttachmentPartitioner._file_bytes (unstructured/partition/email.py) unconditionally calls self._attachment.get_content(), which raises KeyError(content_type) for these cases — and since that call happens before the try/except in _iter_elements() that's meant to gracefully skip unpartitionable attachments, the exception propagates out of the whole partition_email() call instead of being caught.

Fix

_file_bytes now checks self._attachment.get_content_type().startswith("multipart/") and falls back to the part's raw serialized bytes (.as_bytes()) in that case, instead of calling get_content(). This is a root-cause fix at the actual failure point.

I deliberately did not widen EXPECTED_ATTACHMENT_ERRORS (in unstructured/partition/common/__init__.py) to include KeyError as an alternative fix — that tuple's own comment says it's "Intentionally narrow" specifically because a bare KeyError catch there would also silently swallow unrelated bugs elsewhere in partition(). Fixing _file_bytes directly avoids that trade-off entirely.

How was this tested?

  • Reproduced the crash locally with a synthetic .eml (a multipart/mixed email with a nested PGP/MIME-signed forwarded message as an attachment, mirroring the issue's real-world scenario) before writing any fix — confirmed the exact KeyError('multipart/signed').
  • Added test_partition_email_does_not_raise_on_multipart_attachment, using a new fixture example-docs/eml/mime-attach-multipart-signed.eml, following the existing pattern of test_partition_email_silently_skips_attachments_it_cannot_partition (the MP3-attachment test). Confirmed the new test fails against unpatched email.py (git stash) with the exact reported KeyError, and passes after the fix — the email body is still partitioned, and the unpartitionable attachment produces no elements without crashing.
  • Full test_unstructured/partition/test_email.py suite: 72 passed, no regressions.
  • ruff check / ruff format --check — clean.
  • scripts/version-sync.sh -c — clean (no version bump needed; CHANGELOG.md updated under the existing unreleased 0.25.2-dev0 section per the contributing checklist).

Context on prior attempt

A previous PR (#4277) attempted a fix for this same issue but was self-closed by its author after 3 days with no maintainer engagement, and its fixtures never landed — confirmed the bug is still fully live on current main before starting this PR. This PR takes a narrower approach than that one (a single-point fix in _file_bytes, no changes to the shared EXPECTED_ATTACHMENT_ERRORS tuple or to _iter_elements's control flow).

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Shadow auto-approve: would auto-approve. Fixes a KeyError crash when partition_email encounters a multipart/* attachment by falling back to raw serialized bytes; includes a regression test.

Re-trigger cubic

@cragwolfe cragwolfe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Draft review: Unstructured-IO/unstructured PR #4424

Verdict: Request changes

[High] Serialize multipart attachments as standalone documents

unstructured/partition/email.py:443

self._attachment.as_bytes() preserves the part's top-level Content-Disposition: attachment header. When an attachment is named *.eml, auto.partition() routes those serialized bytes back through partition_email(). The serialized part is now the recursive root message, and EmailMessage.get_body() excludes roots whose disposition is attachment, so a valid multipart attachment's body is silently dropped.

I reproduced this with a boundary-bearing multipart/signed attachment: the outer body was returned, but the signed message body was absent. This is introduced by the new serialization path and affects the PGP/MIME forwarded-message case described by the PR.

The added fixture does not exercise that behavior. Its multipart/signed part has no boundary and applies base64 transfer encoding to the multipart entity, so Python parses it as a scalar payload (is_multipart() == False, zero child parts). It proves that the original KeyError no longer escapes, but not that a well-formed multipart attachment is processed correctly.

Please serialize a copy of the multipart part as a standalone document by removing only the copy's top-level Content-Disposition before calling as_bytes(). Preserve the original part for filename metadata and do not mutate the parsed message tree. Add a boundary-bearing multipart/signed regression case that asserts:

  • the outer email body is emitted;
  • the signed attachment body is emitted;
  • the signed body retains filename == "signed-message.eml" and the enclosing attached_to_filename metadata.

The existing malformed fixture can remain as a separate no-crash regression.

(authored by codex)

@eeshsaxena eeshsaxena left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable fix. Since email.contentmanager genuinely has no get_content() handler registered for any multipart/* type, catching that specific case in _AttachmentPartitioner._file_bytes and falling back to the part's raw serialized bytes is the right call, and it lets partition_email() recover the main body instead of the whole call dying on one signed/forwarded sub-part. The regression .eml with a multipart/signed attachment reproduces the original KeyError path, so the test genuinely guards it. Reads correct.

@mittalpk

mittalpk commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Good catch. Fixed in 1d1ca7e — the fix now strips Content-Disposition on a copy before serializing, and I rewrote the fixture too since the original had an invalid Content-Transfer-Encoding: base64 on the multipart entity that made Python parse it as a scalar payload with zero child parts, so it could never have exercised this path.

@mittalpk
mittalpk force-pushed the fix/partition-email-multipart-attachment-keyerror branch from 1d1ca7e to ba4935a Compare August 7, 2026 18:51
email.contentmanager has no get_content() handler registered for any
multipart/* content-type (handlers exist for text/*, application/*,
image/*, message/rfc822, etc., but multipart sub-parts are normally
consumed via .iter_parts(), not .get_content()). A multipart sub-part
can still surface as an "attachment" via iter_attachments() though --
e.g. a PGP/MIME-signed forwarded message (multipart/signed) nested
inside a multipart/mixed envelope, or a multipart/mixed forwarded
message itself. Resolving such an attachment's bytes in
_AttachmentPartitioner._file_bytes raised KeyError('multipart/signed')
(or 'multipart/mixed'), crashing the entire partition_email() call
instead of processing or skipping just that one attachment.

Fixed by falling back to the MIME part's raw serialized bytes
(as_bytes()) when its content-type is multipart/*, rather than calling
get_content(). This is a root-cause fix at the actual failure point,
not a broadening of the shared, deliberately narrow
EXPECTED_ATTACHMENT_ERRORS tuple (its own comment: "Intentionally
narrow... we do not catch RuntimeError... would otherwise be silently
skipped") -- a bare KeyError there would mask unrelated bugs elsewhere
in partition().

Fixes Unstructured-IO#3922.
Serializing a multipart/* attachment with as_bytes() preserved its
own Content-Disposition: attachment header (that's how it surfaced
as an attachment via iter_attachments() in the first place). Since
EmailMessage.get_body() skips any candidate part -- including a
multipart/* root message -- whose own disposition is "attachment",
re-parsing that serialized part as the root of a new message found
no body at all: the attachment partitioned to zero elements instead
of surfacing its own content (e.g. a PGP-signed forwarded message's
body).

_serialized_multipart_bytes now serializes a deep copy of the part
with that header stripped, leaving the original attachment object
(still needed for its filename) untouched.

Also fixed the existing regression fixture
(mime-attach-multipart-signed.eml): its multipart/signed part had
Content-Transfer-Encoding: base64, which is invalid for a multipart
entity per RFC 2045 -- Python's parser treated it as an opaque
scalar payload with zero child parts, so the fixture could never
have exercised this bug regardless of the fix. Rewrote it as a
genuine boundary-bearing multipart/signed part and extended the
test to assert the attachment's own body and metadata now come
through, not just that no exception is raised.
@mittalpk
mittalpk force-pushed the fix/partition-email-multipart-attachment-keyerror branch from ba4935a to 39df846 Compare August 15, 2026 18:38
@mittalpk

Copy link
Copy Markdown
Contributor Author

@cragwolfe following up here -- the fix landed in 1d1ca7e and eeshsaxena confirmed the approach reads correct on 08-06. Would appreciate a re-review when you get a chance.

@cragwolfe

Copy link
Copy Markdown
Contributor

@mittalpk , lgtm. please bump the version and update CHANGELOG.md accordingly (most recent changes at top of the file). thanks!

0.27.2 (and several releases before it) shipped while this PR sat open, so
the CHANGELOG entry had drifted into an already-released section instead
of a new one. Moved it to a fresh 0.27.3 section at the top and bumped
__version__.py to match; make check-version confirms no further sync is
needed.

Requested by @cragwolfe.
@mittalpk

Copy link
Copy Markdown
Contributor Author

Thanks! Bumped to 0.27.3 and moved the CHANGELOG entry -- 0.27.2 (and a few releases before it) shipped while this was open, so it had drifted into an already-released section. make check-version confirms no further sync needed.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Shadow auto-approve: would not auto-approve. Auto-approval skipped because this PR is from an external contributor.

Re-trigger cubic

@cragwolfe
cragwolfe merged commit 911083b into Unstructured-IO:main Oct 15, 2026
53 checks passed
huangyingting pushed a commit to repomesh/unstructured that referenced this pull request Oct 27, 2026
…aising AssertionError (Unstructured-IO#4597)

## Summary

`partition_email()` processes attachments by default. When a message has
a
`message/rfc822` part, the whole call fails with a bare `AssertionError`
and
the body elements are lost too. Common inputs have such a part:

- an email forwarded "as attachment" (Outlook, Thunderbird),
- a delivery-failure bounce (`multipart/report` with the returned
message),
- a `multipart/digest`, where every part is `message/rfc822`.

The repository's own fixture
`example-docs/eml/mime-multipart-digest.eml`
fails this way, through `partition()` too.

Cause: for a `message/rfc822` part, `get_content()` returns the enclosed
`EmailMessage`, not `str` or `bytes`, so
`_AttachmentPartitioner._file_bytes`
fails its `assert isinstance(content, bytes)`. The assert sits outside
the
`try`/`except` that skips attachments it cannot partition.

Fix, in `unstructured/partition/email.py`:

- `_file_bytes` serializes an enclosed message with `as_bytes()`.
- For a `message/rfc822` part, `content_type="message/rfc822"` is added
to the
kwargs of the nested `partition()` call. Without it the bytes go through
libmagic, which reports a message as `text/plain` when its first header
is
e.g. `MIME-Version`, `Message-ID` or `To` and not
`From`/`Received`/`Date`.
The attachment's raw headers then come out as `Title` elements. Python's
`EmailMessage` writes such messages: the forwarded message in the new
test
starts with `Content-Type`, which libmagic does not recognize either.
The
hint is merged into the kwargs only for these parts, so a
`content_type=`
  kwarg passed to `partition_email()` directly still reaches the other
  attachments as before instead of colliding with an explicit keyword.

Related:

- Unstructured-IO#4424 is the sibling fix in the same `_file_bytes` function (the
`KeyError`
on a `multipart/*` attachment). Its comment already lists
`message/rfc822`
among the types `get_content()` handles, but that path still hit the
assert.
- Unstructured-IO#3604 (closed in October 2024 with no fix linked) reported
`process_attachments=True` failing on a nested `.eml`, with a
`TypeError` in
the 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: `main` at 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`:
uses
  the existing `mime-multipart-digest.eml` fixture. It checks the three
  messages' text, their `subject` metadata and `attached_to_filename`.
-
`test_partition_email_partitions_a_message_forwarded_as_an_attachment`:
an
  `EmailMessage` forwarded with `add_attachment()` whose first header is
  `Content-Type`. It checks the body, the forwarded text, `subject` and
  `sent_from`.

Results:

- New tests without the fix:
`pytest test_unstructured/partition/test_email.py -k "multipart_digest
or forwarded_as_an_attachment"`
gives 2 failed, both `AssertionError` at
`unstructured/partition/email.py:485`
(`assert isinstance(content, bytes)`). The whole file without the fix
gives
  2 failed, 77 passed, 1 skipped.
- New tests with the fix: 2 passed.
- With only the `as_bytes()` change and no `content_type` hint, the
digest
  test passes and the forwarded-message test fails with 6 extra `Title`
elements (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
on
  `main` because the `pptx` extra is not installed. `msg.py` has its own
  attachment partitioner and is not changed.
- Manual check of a bounce (`multipart/report` with `text/plain`,
`message/delivery-status` and a `message/rfc822` part whose first header
is
`MIME-Version`): returns the bounce text, the delivery-status lines and
the
returned message's body with `subject == "Lunch"`. Before the fix it
raised
`AssertionError`. A forward of a forward is also partitioned at every
level.
- `make check` (ruff 0.15.10 from the `lint` group): `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).`
- No `CHANGELOG.md` or `unstructured/__version__.py` changes.

## Release note

`partition_email()`, and `partition()` on an `.eml` file, now partition
an
attached 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
message
carries that message's own `subject`, `sent_from` and other email
metadata.

## Checklist

- [x] The PR title is a user-facing release-note entry with a
conventional commit prefix.
- [ ] Apply a category label (`enhancement`, `bug`, or an appropriate
existing label), or ask a maintainer to apply it. A maintainer needs to
apply `bug`.
- [ ] For breaking changes, use `!` in the title and explain impact and
migration above. (Not a breaking change.)

---------

Co-authored-by: sinnercode228 <[email protected]>
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.

bug/partition_signed_emails

3 participants