fix(email): RFC 5322 compliance in message ingest (Message-ID whitespace + In-Reply-To multi-id + unknown-zone Date)

backend/services/email_parser.py

@@ -2,7 +2,8 @@
 from email.message import Message
 from pathlib import Path
 import datetime
-from email.utils import formataddr, getaddresses
+import re
+from email.utils import getaddresses
 from email.utils import parsedate_to_datetime
 from typing import NotRequired, TypedDict
 from .attachment_parser import parse_email_attachment
@@ -39,13 +40,39 @@ def _sanitize_display_text(text: str) -> str:
     return strip_html_markup(_sanitize_nul(text))
 
 
+# Mirror email.utils.formataddr's RFC 5322 display-name quoting: the specials
+# that force a quoted-string, and the characters escaped inside one.
+_ADDRESS_SPECIALS_RE = re.compile(r'[()<>@,;:\\".\[\]]')
+_ADDRESS_QUOTED_ESCAPE_RE = re.compile(r'["\\]')
+
+
+def _format_display_address(display_name: str, address: str) -> str:
+    """Formats an already-decoded display name and address for storage.
+
+    Mirrors ``email.utils.formataddr`` quoting for RFC 5322 special characters
+    but keeps ``display_name`` literal instead of re-encoding a non-ASCII name
+    as an RFC 2047 encoded-word. The ``From``/``To``/``Reply-To`` headers arrive
+    already header-decoded (``policy.default``), and these values are stored for
+    human display, not re-emitted as message headers, so ``formataddr`` would
+    corrupt a decoded name (e.g. Korean) back into ``=?utf-8?b?...?=``.
+    """
+    if not display_name:
+        return address
+    if _ADDRESS_SPECIALS_RE.search(display_name):
+        escaped_name = _ADDRESS_QUOTED_ESCAPE_RE.sub(r"\\\g<0>", display_name)
+        return f'"{escaped_name}" <{address}>'
+    return f"{display_name} <{address}>"
+
+
 def _sanitize_address_display_text(text: str) -> str:
     sanitized_parts: list[str] = []
     for display_name, address in getaddresses([text]):
         safe_display_name = _sanitize_display_text(display_name).strip()
         safe_address = _sanitize_nul(address).strip()
         if safe_address:
-            sanitized_parts.append(formataddr((safe_display_name, safe_address)))
+            sanitized_parts.append(
+                _format_display_address(safe_display_name, safe_address)
+            )
         elif safe_display_name:
             sanitized_parts.append(safe_display_name)
     if sanitized_parts:
@@ -136,6 +163,14 @@ def _extract_date(msg: Message) -> datetime.datetime:
 
     if not parsed_date:
         parsed_date = datetime.datetime.now(datetime.timezone.utc)
+    elif parsed_date.tzinfo is None:
+        # RFC 5322 section 3.3: a "-0000" zone means the time zone is unknown,
+        # for which parsedate_to_datetime returns a naive datetime. Every other
+        # branch here yields a timezone-aware datetime, and mixing naive with
+        # aware datetimes raises TypeError on comparison/sorting and misbinds the
+        # instant when stored in a timestamptz column. Treat the unknown zone as
+        # UTC so the returned value is always timezone-aware.
+        parsed_date = parsed_date.replace(tzinfo=datetime.timezone.utc)
     return parsed_date
 
 

backend/services/threading_service.py

@@ -31,16 +31,32 @@ def generate_email_fingerprint(
 
 
 def normalize_message_id(value: str | None) -> str | None:
-    """Return the canonical persisted form for a Message-ID-like header."""
+    """Return the canonical persisted form for a Message-ID-like header.
+
+    A Message-ID (RFC 5322 section 3.6.4) carries no interior whitespace, but
+    header unfolding (RFC 5322 section 2.2.3) can leave interior spaces or tabs
+    when a folded header is rejoined -- e.g. ``<abc@\\r\\n example.com>`` unfolds
+    to ``<abc@ example.com>``. Collapsing all interior whitespace keeps the
+    folded and unfolded forms of the same Message-ID equal, so de-duplication
+    and threading never split one message into two over a fold boundary.
+    """
     if value is None:
         return None
 
-    normalized = str(value).strip().strip("<>").strip()
+    stripped = str(value).strip().strip("<>")
+    normalized = "".join(stripped.split())
     return normalized or None
 
 
 def extract_reference_ids(value: str | None) -> list[str]:
-    """Extract canonical message IDs from a References header in header order."""
+    """Extract canonical message IDs from a ``1*msg-id`` header in header order.
+
+    RFC 5322 defines both References (section 3.6.4) and In-Reply-To
+    (section 3.6.4) as ``1*msg-id`` -- one or more angle-bracketed Message-IDs,
+    each optionally surrounded by CFWS -- so this extractor applies to either
+    header. Ids are canonicalized with :func:`normalize_message_id` and
+    de-duplicated while preserving header order.
+    """
     if not value:
         return []
 
@@ -107,19 +123,21 @@ async def assign_thread_id(
     Determine the thread_id for a new email based on in_reply_to and references.
     If no existing match is found, generate a new thread_id.
     """
-    in_reply_to = normalize_message_id(email_data.get("in_reply_to"))
+    # In-Reply-To (RFC 5322 section 3.6.4) is 1*msg-id, exactly like References,
+    # and each id may be wrapped in CFWS. Parse it with the same multi-id
+    # extractor rather than treating the whole header as one opaque Message-ID,
+    # so a reply that names several parents -- or a single id trailed by a
+    # comment -- still threads onto an existing ancestor instead of splitting off.
+    in_reply_to_ids = extract_reference_ids(email_data.get("in_reply_to"))
     references = extract_reference_ids(email_data.get("references"))
 
     existing_candidates = []
     # Optimization: Use a set for O(1) membership checks to prevent O(n^2) deduplication of candidates
     seen = set()
-    if in_reply_to:
-        existing_candidates.append(in_reply_to)
-        seen.add(in_reply_to)
-    for ref in references:
-        if ref not in seen:
-            seen.add(ref)
-            existing_candidates.append(ref)
+    for candidate in (*in_reply_to_ids, *references):
+        if candidate not in seen:
+            seen.add(candidate)
+            existing_candidates.append(candidate)
 
     if existing_candidates:
         thread_ids_by_message_id = await _find_existing_thread_ids(
@@ -138,8 +156,8 @@ async def assign_thread_id(
     if references:
         return references[0]
 
-    if in_reply_to:
-        return in_reply_to
+    if in_reply_to_ids:
+        return in_reply_to_ids[0]
 
     msg_id = normalize_message_id(email_data.get("message_id"))
     if msg_id:

backend/tests/test_email_parser.py

@@ -1,12 +1,23 @@
+import base64
 import datetime
 import os
 import tempfile
 from email.message import Message
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 import pytest
-from services.email_parser import _extract_thread_id, _sanitize_nul, parse_eml
-from services.exceptions import EmailParseError
+from services.email_parser import (
+    EmailParseError,
+    _attachment_part_content,
+    _extract_thread_id,
+    _format_display_address,
+    _process_multipart_body,
+    _process_singlepart_body,
+    _sanitize_address_display_text,
+    _sanitize_nul,
+    parse_eml,
+    parse_eml_bytes,
+)
 
 
 def test_parse_eml_basic():
@@ -109,6 +120,107 @@ def test_parse_eml_strips_active_html_from_address_display_fields():
         os.unlink(temp_path)
 
 
+def test_parse_eml_stores_non_ascii_display_names_decoded():
+    # RFC 2047: non-ASCII From/To/Reply-To display names arrive as encoded-words
+    # (e.g. =?UTF-8?B?...?=). policy.default header-decodes them; the stored
+    # display fields must keep the decoded text rather than re-encoding it back
+    # into an encoded-word (formataddr's behavior), which would render every
+    # non-ASCII sender/recipient as garbled =?utf-8?...?= bytes in the UI.
+    from_name = "박성호"
+    to_name = "김천"
+    reply_name = "응답"
+    subject_text = "회 테스트"
+
+    def encoded_word(text: str) -> bytes:
+        token = base64.b64encode(text.encode("utf-8")).decode("ascii")
+        return f"=?UTF-8?B?{token}?=".encode("ascii")
+
+    eml_content = (
+        b"Message-ID: <i18n@test.com>\r\n"
+        b"From: " + encoded_word(from_name) + b" <sender@example.com>\r\n"
+        b"To: " + encoded_word(to_name) + b" <recipient@test.com>\r\n"
+        b"Reply-To: " + encoded_word(reply_name) + b" <reply@test.com>\r\n"
+        b"Subject: " + encoded_word(subject_text) + b"\r\n"
+        b"Date: Mon, 27 Apr 2026 10:00:00 +0000\r\n"
+        b"\r\n"
+        b"Plain body"
+    )
+
+    with tempfile.NamedTemporaryFile(delete=False, suffix=".eml") as f:
+        f.write(eml_content)
+        temp_path = f.name
+
+    try:
+        parsed = parse_eml(temp_path)
+        assert parsed["sender"] == f"{from_name} <sender@example.com>"
+        assert parsed["recipients"] == f"{to_name} <recipient@test.com>"
+        assert parsed["reply_to"] == f"{reply_name} <reply@test.com>"
+        assert parsed["subject"] == subject_text
+        assert "=?" not in parsed["sender"]
+        assert "=?" not in parsed["recipients"]
+    finally:
+        os.unlink(temp_path)
+
+
+def test_sanitize_address_display_text_keeps_decoded_unicode_and_quotes_specials():
+    # A decoded non-ASCII name stays literal (formataddr would re-encode it).
+    assert (
+        _sanitize_address_display_text("박성호 <sender@example.com>")
+        == "박성호 <sender@example.com>"
+    )
+    # A display name containing an RFC 5322 special is quoted so a ", "-joined
+    # multi-address value stays unambiguous.
+    assert (
+        _sanitize_address_display_text('"Doe, John" <j@x.com>')
+        == '"Doe, John" <j@x.com>'
+    )
+    # Multiple addresses with mixed scripts are each formatted and comma-joined.
+    assert (
+        _sanitize_address_display_text("박성호 <a@x.com>, Bob <b@x.com>")
+        == "박성호 <a@x.com>, Bob <b@x.com>"
+    )
+
+
+def test_format_display_address_escapes_quotes_and_handles_empty_name():
+    # No display name -> bare address.
+    assert _format_display_address("", "a@x.com") == "a@x.com"
+    # Non-ASCII name kept literal.
+    assert _format_display_address("박성호", "s@x.com") == "박성호 <s@x.com>"
+    # Embedded quotes/backslashes are escaped inside the quoted-string, matching
+    # email.utils.formataddr's escaping.
+    assert (
+        _format_display_address('Fancy "Q"', "q@x.com") == '"Fancy \\"Q\\"" <q@x.com>'
+    )
+
+
+def test_process_multipart_body_ignores_non_string_part_content():
+    # get_content() can return a non-str (e.g. undecodable bytes) even for a
+    # text/* part; the isinstance guard must drop it rather than concatenate
+    # bytes into the plain/html body.
+    plain_part = MagicMock()
+    plain_part.get_content_type.return_value = "text/plain"
+    plain_part.get_filename.return_value = None
+    plain_part.get_content.return_value = b"not-a-str"
+    html_part = MagicMock()
+    html_part.get_content_type.return_value = "text/html"
+    html_part.get_filename.return_value = None
+    html_part.get_content.return_value = b"not-a-str"
+    msg = MagicMock()
+    msg.walk.return_value = [plain_part, html_part]
+
+    assert _process_multipart_body(msg) == ("", "", [])
+
+
+def test_process_singlepart_body_ignores_non_string_content():
+    # A single-part message whose get_content() returns a non-str yields an
+    # empty body rather than a stringified bytes value.
+    msg = MagicMock()
+    msg.get_content_type.return_value = "text/plain"
+    msg.get_content.return_value = b"not-a-str"
+
+    assert _process_singlepart_body(msg) == ("", "", [])
+
+
 def test_parse_eml_strips_active_html_from_attachment_display_fields():
     eml_content = b"""Message-ID: <attachment-xss@test.com>
 From: sender@test.com
@@ -325,6 +437,33 @@ def test_parse_eml_missing_and_malformed_date():
         os.unlink(temp_path2)
 
 
+def test_parse_eml_unknown_timezone_date_is_timezone_aware():
+    # RFC 5322 section 3.3: a "-0000" zone means the time zone is unknown, for
+    # which parsedate_to_datetime returns a *naive* datetime. Every other parse
+    # path yields an aware datetime, so the parser must normalize this to aware
+    # too -- otherwise sorting/comparing it against another message's date raises
+    # "can't compare offset-naive and offset-aware datetimes" and it misbinds the
+    # instant in a timestamptz column.
+    eml_content = b"""Message-ID: <unknownzone@test.com>
+From: test@test.com
+To: recipient@test.com
+Subject: Unknown zone
+Date: Mon, 27 Apr 2026 10:00:00 -0000
+
+Test."""
+    with tempfile.NamedTemporaryFile(delete=False, suffix=".eml") as f:
+        f.write(eml_content)
+        temp_path = f.name
+
+    try:
+        parsed = parse_eml(temp_path)
+        assert parsed["date"].tzinfo is not None
+        # must not raise offset-naive/aware TypeError
+        assert parsed["date"] <= datetime.datetime.now(datetime.timezone.utc)
+    finally:
+        os.unlink(temp_path)
+
+
 def test_parse_eml_io_error():
     with pytest.raises(EmailParseError):
         parse_eml("/path/to/nonexistent/file.eml")
@@ -381,6 +520,70 @@ def test_extract_thread_id_uses_first_reference_from_long_header():
     assert _extract_thread_id(msg, "<message@test.com>") == "<root@test.com>"
 
 
+def test_sanitize_address_display_text_keeps_name_only_and_falls_back_to_text():
+    # A token with a display name but an empty address part keeps the name
+    # (rather than dropping it), and a header that yields no address at all
+    # falls back to the sanitized raw text.
+    assert _sanitize_address_display_text("Display Name <>") == "Display Name"
+    assert _sanitize_address_display_text("") == ""
+
+
+def test_attachment_part_content_falls_back_to_raw_payload_on_decode_error():
+    # A part whose get_content() cannot decode (unknown charset / malformed
+    # transfer-encoding) falls back to the raw decoded payload, and to "" when
+    # the payload is absent, instead of propagating the decode error.
+    raw_part = MagicMock()
+    raw_part.get_content.side_effect = LookupError("unknown charset")
+    raw_part.get_payload.return_value = b"raw-bytes"
+    assert _attachment_part_content(raw_part) == b"raw-bytes"
+
+    empty_part = MagicMock()
+    empty_part.get_content.side_effect = ValueError("bad encoding")
+    empty_part.get_payload.return_value = None
+    assert _attachment_part_content(empty_part) == ""
+
+
+def test_parse_eml_bytes_parses_provider_bytes_and_wraps_parse_errors():
+    parsed = parse_eml_bytes(
+        b"Message-ID: <bytes@test.com>\r\n"
+        b"From: sender@test.com\r\n"
+        b"To: user@test.com\r\n"
+        b"Subject: Bytes\r\n\r\n"
+        b"Body"
+    )
+    assert parsed["message_id"] == "<bytes@test.com>"
+    assert parsed["subject"] == "Bytes"
+
+    # A parser failure is wrapped as the sanitized public EmailParseError rather
+    # than leaking the internal exception chain at the ingest boundary.
+    with patch(
+        "services.email_parser.message_from_bytes", side_effect=ValueError("boom")
+    ):
+        with pytest.raises(EmailParseError):
+            parse_eml_bytes(b"anything")
+
+
+def test_extract_thread_id_falls_through_whitespace_only_headers():
+    # A References/In-Reply-To header that unfolds to only whitespace is present
+    # but yields no token when split; _extract_thread_id must fall through to the
+    # next source rather than return a blank thread id.
+    fell_to_in_reply_to = Message()
+    fell_to_in_reply_to["References"] = "   "
+    fell_to_in_reply_to["In-Reply-To"] = "<parent@test.com>"
+    assert (
+        _extract_thread_id(fell_to_in_reply_to, "<message@test.com>")
+        == "<parent@test.com>"
+    )
+
+    fell_to_message_id = Message()
+    fell_to_message_id["References"] = "  "
+    fell_to_message_id["In-Reply-To"] = " \t "
+    assert (
+        _extract_thread_id(fell_to_message_id, "<message@test.com>")
+        == "<message@test.com>"
+    )
+
+
 def test_parse_eml_extracts_reply_to_header():
     eml_content = b"""Message-ID: <reply-to@test.com>
 From: Sender Name <sender@test.com>