diff --git a/backend/services/email_parser.py b/backend/services/email_parser.py index 2d32454ad..66b2b0c25 100644 --- a/backend/services/email_parser.py +++ b/backend/services/email_parser.py @@ -2,13 +2,20 @@ 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 from .exceptions import EmailParseError from .text_safety import strip_html_markup +# Mirrors ``email.utils`` display-name quoting so an already-decoded display +# name keeps identical formatting for ASCII values while avoiding RFC 2047 +# re-encoding (see ``_format_address_display``). +_ADDRESS_SPECIALS_RE = re.compile(r'[][\\()<>@,:;".]') +_ADDRESS_ESCAPE_RE = re.compile(r'[\\"]') + class EmailData(TypedDict): """Parsed email data structure.""" @@ -39,13 +46,30 @@ def _sanitize_display_text(text: str) -> str: return strip_html_markup(_sanitize_nul(text)) +def _format_address_display(display_name: str, address: str) -> str: + """Render ``display_name
`` for a display field. + + Unlike ``email.utils.formataddr``, this never re-encodes a non-ASCII + display name into an RFC 2047 encoded-word: these fields are display + surfaces and RFC 2047 §6.2 requires encoded-words to be shown decoded. + Quoting/escaping matches ``formataddr`` so ASCII values are unchanged. + """ + if not display_name: + return address + escaped = _ADDRESS_ESCAPE_RE.sub(r"\\\g<0>", display_name) + quotes = '"' if _ADDRESS_SPECIALS_RE.search(display_name) else "" + return f"{quotes}{escaped}{quotes} <{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_address_display(safe_display_name, safe_address) + ) elif safe_display_name: sanitized_parts.append(safe_display_name) if sanitized_parts: diff --git a/backend/services/threading_service.py b/backend/services/threading_service.py index 01c738432..c939f5470 100644 --- a/backend/services/threading_service.py +++ b/backend/services/threading_service.py @@ -107,7 +107,13 @@ 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")) + # RFC 5322 §3.6.4 defines In-Reply-To as ``1*msg-id``: it may carry more + # than one message-id. Parse it with the same angle-bracket extractor used + # for References and take the first parsed id as the immediate parent + # (jwz "message threading": extract the first message-id from In-Reply-To). + # Stripping ``<>`` off the whole header would mangle a multi-id value. + in_reply_to_ids = extract_reference_ids(email_data.get("in_reply_to")) + in_reply_to = in_reply_to_ids[0] if in_reply_to_ids else None references = extract_reference_ids(email_data.get("references")) existing_candidates = [] diff --git a/backend/tests/test_email_parser.py b/backend/tests/test_email_parser.py index e44d03127..572378408 100644 --- a/backend/tests/test_email_parser.py +++ b/backend/tests/test_email_parser.py @@ -381,6 +381,41 @@ def test_extract_thread_id_uses_first_reference_from_long_header(): assert _extract_thread_id(msg, "") == "" +def test_parse_eml_decodes_rfc2047_encoded_word_address_display_names(): + """RFC 2047 §6.2 requires encoded-words in a displayed header to be shown + in their decoded form. The From/To/Reply-To display fields are display + surfaces, so a ``=?UTF-8?B?...?=`` (B) or ``=?UTF-8?Q?...?=`` (Q) display + name must be decoded to human-readable text and must NOT leak back into the + stored/displayed value as a raw encoded-word. + """ + eml_content = ( + b"Message-ID: \n" + b"From: =?UTF-8?B?7ZmN6ri464+Z?= \n" + b"To: =?UTF-8?B?7ZmN6ri464+Z?= , " + b"=?UTF-8?Q?Bj=C3=B6rn?= \n" + b"Reply-To: =?UTF-8?Q?Bj=C3=B6rn?= \n" + b"Subject: RFC 2047 address display\n" + b"Date: Mon, 27 Apr 2026 10:00:00 +0000\n" + b"\n" + b"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"] == "홍길동 " + assert parsed["recipients"] == "홍길동 , Björn " + assert parsed["reply_to"] == "Björn " + assert "=?" not in parsed["sender"] + assert "=?" not in parsed["recipients"] + assert "=?" not in parsed["reply_to"] + finally: + os.unlink(temp_path) + + def test_parse_eml_extracts_reply_to_header(): eml_content = b"""Message-ID: From: Sender Name diff --git a/backend/tests/test_threading_service.py b/backend/tests/test_threading_service.py index 9c0d03450..1296ecfba 100644 --- a/backend/tests/test_threading_service.py +++ b/backend/tests/test_threading_service.py @@ -124,6 +124,59 @@ async def test_forwarded_subject_alone_does_not_merge_unrelated_thread(): assert session.execute_count == 0 +@pytest.mark.asyncio +async def test_multi_id_in_reply_to_threads_on_first_parent_message_id(): + """A multi message-id In-Reply-To must join the first parent's thread. + + RFC 5322 §3.6.4 defines ``in-reply-to = "In-Reply-To:" 1*msg-id`` — the + field may legitimately carry more than one message-id. jwz's "message + threading" says to extract the first message-id-looking token from + In-Reply-To. The immediate parent identifier must therefore be that first + id (````), not the raw concatenation of both ids, so the + reply threads onto the already-imported parent. + """ + session = _SequentialSession([[("", "thread-123")]]) + + thread_id = await assign_thread_id( + session, + { + "message_id": "", + "in_reply_to": " ", + "references": None, + }, + user_id="testuser", + organization_id="org-acme", + ) + + assert thread_id == "thread-123" + + +@pytest.mark.asyncio +async def test_multi_id_in_reply_to_deterministic_root_is_first_message_id(): + """A multi message-id In-Reply-To yields the first id as the thread root. + + Per RFC 5322 §3.6.4 In-Reply-To is ``1*msg-id``; when References is absent + and the parent has not been imported yet, the deterministic thread root must + be the first parsed message-id (jwz: use the first message-id from + In-Reply-To), never the mangled ``"parent@example.com> ", + "in_reply_to": " ", + "references": None, + }, + user_id="testuser", + organization_id="org-acme", + ) + + assert thread_id == "parent@example.com" + + @pytest.mark.asyncio async def test_existing_thread_lookup_is_scoped_to_owner_and_organization(): session = _QueryCapturingSession([[("", "thread-123")]])