Repository navigation
STA-36 - Skip IMAP ENVELOPE group markers instead of turning them into addresses - #123
Conversation
|
bugbot run |
dbezverkhnii
left a comment
There was a problem hiding this comment.
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"])
}
}
#endifPackage.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"],|
bugbot run |
There was a problem hiding this comment.
✅ 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.
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_arraynow 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.importIMAPEnvelopeonly 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 viaLeaseTestTCPEndpoint, and the test target list inPackage.swiftincludes 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.