Merge pull request #120 from christianlouis/copilot/debug-t-online-mail-issue
Fix all IMAP emails appearing empty (T-Online, GMX, and any IMAP account)
This commit is contained in:
@@ -27,6 +27,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- Added `.safety-policy.yml` to document and suppress the two unfixable `ecdsa` side-channel CVEs (64396, 64459) that the upstream maintainers have acknowledged cannot be resolved in pure Python.
|
||||
### Fixed
|
||||
|
||||
- **IMAP: fix all IMAP emails appearing empty** — `aioimaplib` stores RFC822
|
||||
literal data as `bytearray`, not `bytes`. The FETCH extraction loop was
|
||||
checking `isinstance(line, bytes)` which returns `False` for `bytearray`,
|
||||
causing every email body to be silently skipped and the message to appear
|
||||
empty. The check now accepts both types (`isinstance(line, (bytes,
|
||||
bytearray))`) and converts the result to plain `bytes` before returning,
|
||||
so the rest of the pipeline is unaffected. This affected every IMAP
|
||||
account (T-Online, GMX, and others).
|
||||
- **IMAP: fix T-Online BYE "Too many invalid IMAP commands"** — `UID STORE` flag
|
||||
arguments now use RFC 3501–required parentheses: `+FLAGS (\Seen)` and
|
||||
`+FLAGS (\Deleted)`. Strict servers such as T-Online reject the bare
|
||||
|
||||
@@ -383,9 +383,14 @@ class MailProcessor:
|
||||
# [b'<seq> (UID <uid> RFC822 {<size>}', <email_bytes>, b')']
|
||||
# Skip the header line (contains "RFC822") and grab the
|
||||
# first substantive bytes value.
|
||||
#
|
||||
# aioimaplib stores IMAP literal data (the actual email
|
||||
# content) as bytearray, not bytes. We must accept both
|
||||
# types; bytes() converts bytearray so the rest of the
|
||||
# pipeline always receives plain bytes.
|
||||
email_data: Optional[bytes] = None
|
||||
for line in fetch_response.lines:
|
||||
if not isinstance(line, bytes):
|
||||
if not isinstance(line, (bytes, bytearray)):
|
||||
continue
|
||||
if b"RFC822" in line:
|
||||
continue
|
||||
@@ -393,7 +398,7 @@ class MailProcessor:
|
||||
continue
|
||||
if line in (b")", b""):
|
||||
continue
|
||||
email_data = line
|
||||
email_data = bytes(line)
|
||||
break
|
||||
|
||||
if email_data and email_data.strip():
|
||||
|
||||
@@ -479,3 +479,70 @@ class TestFetchImapEmailsUidCommands:
|
||||
|
||||
assert emails == []
|
||||
assert new_uids == []
|
||||
|
||||
async def test_bytearray_literal_data_is_accepted(self, processor, mock_imap):
|
||||
"""aioimaplib stores IMAP literal data as bytearray, not bytes.
|
||||
|
||||
This is the root cause of emails appearing empty on IMAP servers such
|
||||
as T-Online and GMX: the extraction loop previously skipped bytearray
|
||||
objects because ``isinstance(bytearray(...), bytes)`` is False.
|
||||
|
||||
The fix accepts both bytes and bytearray and converts to bytes so the
|
||||
rest of the pipeline receives plain bytes as expected.
|
||||
"""
|
||||
raw_email = b"From: user@t-online.de\r\nSubject: Real email\r\n\r\nBody"
|
||||
# aioimaplib returns the RFC822 literal as bytearray in response.lines
|
||||
bytearray_response = _make_imap_response(
|
||||
result="OK",
|
||||
lines=[
|
||||
b"1 (UID 55 RFC822 {%d}" % len(raw_email),
|
||||
bytearray(raw_email),
|
||||
b")",
|
||||
],
|
||||
)
|
||||
|
||||
mock_imap.search.return_value = _make_imap_response(result="OK", lines=[b"55"])
|
||||
mock_imap.fetch.return_value = _make_uid_list_response([("55", "55")])
|
||||
|
||||
mock_imap.uid = AsyncMock(
|
||||
side_effect=lambda cmd, *args: (
|
||||
bytearray_response if cmd == "fetch" else _make_imap_response()
|
||||
)
|
||||
)
|
||||
|
||||
with patch(
|
||||
"app.services.mail_processor.aioimaplib.IMAP4_SSL",
|
||||
return_value=mock_imap,
|
||||
):
|
||||
emails, new_uids = await processor._fetch_imap_emails(10, set())
|
||||
|
||||
# The email must be extracted and returned as plain bytes
|
||||
assert new_uids == ["55"]
|
||||
assert len(emails) == 1
|
||||
assert emails[0] == raw_email
|
||||
assert isinstance(emails[0], bytes)
|
||||
|
||||
async def test_bytearray_whitespace_literal_is_skipped(self, processor, mock_imap):
|
||||
"""A bytearray literal that is whitespace-only must still be rejected."""
|
||||
bytearray_ws_response = _make_imap_response(
|
||||
result="OK",
|
||||
lines=[b"1 (UID 56 RFC822 {2}", bytearray(b"\r\n"), b")"],
|
||||
)
|
||||
|
||||
mock_imap.search.return_value = _make_imap_response(result="OK", lines=[b"56"])
|
||||
mock_imap.fetch.return_value = _make_uid_list_response([("56", "56")])
|
||||
|
||||
mock_imap.uid = AsyncMock(
|
||||
side_effect=lambda cmd, *args: (
|
||||
bytearray_ws_response if cmd == "fetch" else _make_imap_response()
|
||||
)
|
||||
)
|
||||
|
||||
with patch(
|
||||
"app.services.mail_processor.aioimaplib.IMAP4_SSL",
|
||||
return_value=mock_imap,
|
||||
):
|
||||
emails, new_uids = await processor._fetch_imap_emails(10, set())
|
||||
|
||||
assert emails == []
|
||||
assert new_uids == []
|
||||
|
||||
@@ -4,6 +4,7 @@ Comprehensive task breakdown for repository improvements and production readines
|
||||
|
||||
## ✅ Recently Completed
|
||||
|
||||
- [x] **IMAP: fix all emails appearing empty** — `aioimaplib` stores RFC822 literal data as `bytearray`, not `bytes`. The extraction loop was checking `isinstance(line, bytes)` which returns `False` for `bytearray`, so every email body was silently skipped. Fixed to accept both types and convert to `bytes`. Affected T-Online, GMX, and all IMAP accounts.
|
||||
- [x] **Security: upgrade fastapi/starlette and fix safety CI command** — Upgraded `fastapi` to `0.135.2` (pulls in `starlette>=1.0.0`) fixing 4 DoS CVEs in `starlette<=0.35.1`; replaced deprecated `safety check` with `safety scan`; added `.safety-policy.yml` to suppress unfixable `ecdsa` side-channel CVEs (maintainers won't fix).
|
||||
- [x] **IMAP RFC 3501 flag syntax & aioimaplib UID SEARCH fix**: `_fetch_imap_emails`
|
||||
now uses a plain `SEARCH UNSEEN` + `FETCH (UID)` to resolve sequence numbers to
|
||||
|
||||
Reference in New Issue
Block a user