Skip to content

STA-36 - Skip IMAP ENVELOPE group markers instead of turning them into addresses - #123

Merged
dbezverkhnii merged 2 commits into
spark2from
fix/STA-36-address-group-marker
Oct 23, 2026
Merged

dbezverkhnii merged 2 commits into
spark2from
fix/STA-36-address-group-marker

Conversation

@vs-savchenko

@vs-savchenko vs-savchenko commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Medium Risk
Changes how fetched message headers populate recipient fields for all IMAP ENVELOPE address lists, with a documented edge case that drops ambiguous domain-less entries before groups.

Overview
Fixes IMAP ENVELOPE parsing so RFC 3501 group markers are not turned into fake recipients. imap_mailbox_list_to_address_array now skips lone (NIL NIL NIL NIL) end markers (e.g. Strato on empty fields), skips group-start entries when a group end follows, and keeps only group members—matching RFC 822 group behavior.

importIMAPEnvelope only sets Reply-To, To, Cc, and Bcc when the filtered list is non-empty, so empty marker-only lists leave those headers unset. Swift integration tests (IMAPEnvelopeAddressTests) exercise UID FETCH with synthetic ENVELOPE data via LeaseTestTCPEndpoint, and the test target list in Package.swift includes the new file.

Tradeoff: a domain-less mailbox immediately before a group cannot be distinguished from a group name in ENVELOPE; that entry is dropped so group labels never appear as recipients.

Reviewed by Cursor Bugbot for commit 08f1e47. Bugbot is set up for automated code reviews on this repo. Configure here.

@dbezverkhnii

Copy link
Copy Markdown
Collaborator

bugbot run

@cursor cursor Bot 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.

Stale Bugbot comment from a previous run.

@dbezverkhnii dbezverkhnii left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The fix removes Strato's lone ((NIL NIL NIL NIL)): a test against this branch passes, and the same test fails on the parent commit. Two points before merge: the NIL-host check also drops addresses that are not group markers (inline below), and the change has no tests. The mailcore2 - Windows prebuilt check is red, as expected for a change under src/: the prebuilt needs publishing (see "Publishing the Windows prebuilt" in AGENTS.md).

It may be worth adding the tests below as unittest/IMAPEnvelopeAddressTests.swift, plus the file in the MailCoreTests sources in Package.swift. They fetch an ENVELOPE from LeaseTestTCPEndpoint, so they run on the macOS CI job. On this branch 3 of 4 pass; with the narrowed check from the inline comment all 4 pass, and the full suite (76 tests) stays green.

unittest/IMAPEnvelopeAddressTests.swift
//
//  IMAPEnvelopeAddressTests.swift
//  mailcore2
//
//  Addresses that MessageHeader::importIMAPEnvelope() takes from an IMAP ENVELOPE, where a NIL
//  host name marks the start or the end of a group (RFC 3501 7.4.2).
//

// Darwin only: the tests need a POSIX listening socket, and the Android job builds the test target
// without running it. Nothing here is platform-specific beyond that socket.
#if canImport(Darwin)

import Dispatch
import Foundation
import XCTest

#if SWIFT_PACKAGE
import CMailCore
#endif

@testable import MailCore

final class IMAPEnvelopeAddressTests: XCTestCase {

    private static let from = "((\"Sender Name\" NIL \"sender\" \"example.com\"))"
    private static let recipient = "((NIL NIL \"recipient\" \"example.com\"))"
    private static let groupEnd = "((NIL NIL NIL NIL))"

    /// The header of message 1 as the server describes it with `envelope`, fetched over IMAP.
    private func fetchHeader(envelope: String) throws -> MCOMessageHeader? {
        let endpoint = try LeaseTestTCPEndpoint(greeting: "* OK [CAPABILITY IMAP4rev1] LeaseTestTCPEndpoint ready\r\n",
                                                answers: ["LOGIN": "",
                                                          "CAPABILITY": "* CAPABILITY IMAP4rev1\r\n",
                                                          "LIST": "* LIST (\\Noselect) \"/\" \"\"\r\n",
                                                          "SELECT": "* 1 EXISTS\r\n* OK [UIDVALIDITY 1]\r\n",
                                                          "UID": "* 1 FETCH (UID 1 ENVELOPE \(envelope))\r\n"])
        defer { endpoint.stop() }

        let session = MCOIMAPSession()
        session.hostname = "127.0.0.1"
        session.port = UInt32(endpoint.port)
        session.connectionType = ConnectionTypeClear
        session.username = "user"
        session.password = "password"
        session.timeout = 60
        session.maximumConnections = 1

        var header: MCOMessageHeader?
        let fetched = expectation(description: "fetch")
        session.fetchMessagesByUid(folder: "INBOX", kind: .headers, uids: MCOIndexSet(index: 1)).start { error, messages, _ in
            XCTAssertNil(error)
            XCTAssertEqual(messages?.count, 1)
            header = messages?.first?.header
            fetched.fulfill()
        }
        waitForExpectations(timeout: 30)
        return header
    }

    private func envelope(sender: String = from, replyTo: String = "NIL", to: String = recipient,
                          cc: String = "NIL", bcc: String = "NIL") -> String {
        return "(\"Fri, 09 Oct 2026 12:41:28 +0000\" \"Subject\" \(Self.from) \(sender) \(replyTo) \(to) \(cc) \(bcc) NIL \"<id@example.com>\")"
    }

    private func mailboxes(_ addresses: [MCOAddress]?) -> [String] {
        return (addresses ?? []).map { $0.mailbox }
    }

    /// Strato sends a lone group end marker for every empty address field instead of NIL.
    func testLoneGroupEndMarkerIsNoAddress() throws {
        let header = try XCTUnwrap(fetchHeader(envelope: envelope(sender: Self.groupEnd, replyTo: Self.groupEnd,
                                                                  cc: Self.groupEnd, bcc: Self.groupEnd)))

        XCTAssertEqual(header.from?.mailbox, "sender@example.com")
        XCTAssertNil(header.sender)
        XCTAssertEqual(mailboxes(header.replyTo), [])
        XCTAssertEqual(mailboxes(header.to), ["recipient@example.com"])
        XCTAssertEqual(mailboxes(header.cc), [])
        XCTAssertEqual(mailboxes(header.bcc), [])
    }

    func testGroupContributesOnlyItsMembers() throws {
        let to = "((NIL NIL \"team\" NIL)(NIL NIL \"a\" \"example.com\")(NIL NIL \"b\" \"example.com\")(NIL NIL NIL NIL))"
        let header = try XCTUnwrap(fetchHeader(envelope: envelope(to: to)))

        XCTAssertEqual(mailboxes(header.to), ["a@example.com", "b@example.com"])
    }

    /// `To: undisclosed-recipients:;` - as when the same header is parsed from RFC 822, a group
    /// with no members contributes nothing.
    func testEmptyGroupIsNoAddress() throws {
        let to = "((NIL NIL \"undisclosed-recipients\" NIL)(NIL NIL NIL NIL))"
        let header = try XCTUnwrap(fetchHeader(envelope: envelope(to: to)))

        XCTAssertEqual(mailboxes(header.to), [])
    }

    /// An address without a domain, with no group end marker after it, is not a group start: it
    /// stays a recipient, as it does when `To: root` is parsed from RFC 822.
    func testAddressWithoutHostOutsideGroupIsKept() throws {
        let header = try XCTUnwrap(fetchHeader(envelope: envelope(to: "((NIL NIL \"root\" NIL))")))

        XCTAssertEqual(mailboxes(header.to), ["root"])
    }
}

#endif
Package.swift
-        sources: ["CertificateUtilsTests.swift", "IMAPAutomaticDisconnectDelayTests.swift", "IMAPConnectionLeaseTests.swift", "IMAPConnectionOwnerLifetimeTests.swift", "IMAPIdleCancellationTests.swift", "IMAPInterruptCurrentCommandTests.swift", "IMAPLoginTests.swift", "LeaseTestTCPEndpoint.swift", "LibetpanHelperTests.swift", "SMTPCheckAccountTests.swift", "unittest.swift"],
+        sources: ["CertificateUtilsTests.swift", "IMAPAutomaticDisconnectDelayTests.swift", "IMAPConnectionLeaseTests.swift", "IMAPConnectionOwnerLifetimeTests.swift", "IMAPEnvelopeAddressTests.swift", "IMAPIdleCancellationTests.swift", "IMAPInterruptCurrentCommandTests.swift", "IMAPLoginTests.swift", "LeaseTestTCPEndpoint.swift", "LibetpanHelperTests.swift", "SMTPCheckAccountTests.swift", "unittest.swift"],

Comment thread src/core/abstract/MCMessageHeader.cpp Outdated
@vs-savchenko

Copy link
Copy Markdown
Collaborator Author

bugbot run

@cursor cursor Bot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 08f1e47. Configure here.

@dbezverkhnii
dbezverkhnii self-requested a review October 23, 2026 08:59
@dbezverkhnii
dbezverkhnii merged commit 78e0d81 into spark2 Oct 23, 2026
9 of 10 checks passed
@dbezverkhnii
dbezverkhnii deleted the fix/STA-36-address-group-marker branch October 23, 2026 09:06
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.

2 participants