mail-parser icon indicating copy to clipboard operation
mail-parser copied to clipboard

Improving the representation of RFC8621 groups

Open sftse opened this issue 6 months ago • 0 comments

The way mail-parser currently represents mailboxes and groups is a bit verbose to deal with, and makes subsequent handling more complex and error prone. Because of the arguably awkward way RFC8621 decides to represent single mailboxes #98 as a group by section 4.1.2.4 GroupedAddresses, anybody that does need the fidelity of the addresses parsed as in RFC5322 has to decompose them again.

This PR does not advocate changing the representation to RFC5322, instead suggesting more modestly that the enum be abandoned. Because of the awkwardness of RFC8621 GroupedAddresses users that do match on the enum already must be aware that single mailboxes may have different representations, and have to make sure to handle mailboxes the same whether they are in Address::List or Address::Group. As an example, we needed the RFC5322 representation for which we used this code

#[derive(Clone, Debug, PartialEq, Eq)]
enum Address<'x> {
    Mailbox(crate::Addr<'x>),
    Group(crate::Group<'x>),
}
fn into_addresses(src: crate::Address<'_>) -> Vec<Address<'_>> {
    let mut addresses = Vec::new();
    match src {
        crate::Address::List(addrss) => {
            for addr in addrss {
                addresses.push(Address::Mailbox(addr));
            }
        }
        crate::Address::Group(groups) => {
            for grp in groups
            {
                if grp.name.is_some() {
                    addresses.push(Address::Group(grp));
                } else {
                    for addr in grp.addresses {
                        addresses.push(Address::Mailbox(addr));
                    }
                }
            }
        }
    }
    addresses
}

This exemplifies how the enum is more of a hinderance than a benefit as an API, but the common Address::List case does save one allocation over Address::Group. This PR removes the enum, focusing on the way this simplifies the API, but we can salvage the allocation if necessary.

For reference, this is what the above code would look like after this PR.

fn into_addresses(addr: crate::Address<'_>) -> Vec<Address<'_>> {
    let mut addresses = Vec::new();
    for grp in addr.groups {
        if grp.name.is_some() {
            addresses.push(Address::Group(grp));
        } else {
            for addr in grp.addresses {
                addresses.push(Address::Mailbox(addr));
            }
        }
    }
    addresses
}

sftse avatar May 28 '25 12:05 sftse