Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unintuitive parsing result #98

Open
sftse opened this issue Jan 29, 2025 · 3 comments
Open

Unintuitive parsing result #98

sftse opened this issue Jan 29, 2025 · 3 comments

Comments

@sftse
Copy link
Contributor

sftse commented Jan 29, 2025

mail-parser encodes mailboxes and groups slightly different to RFC5322 3.4 Address Specification, printed below reordered for better reading.

display-name    =   phrase
angle-addr      =   [CFWS] "<" addr-spec ">" [CFWS] / obs-angle-addr
name-addr       =   [display-name] angle-addr
mailbox         =   name-addr / addr-spec
mailbox-list    =   (mailbox *("," mailbox)) / obs-mbox-list
group-list      =   mailbox-list / CFWS / obs-group-list
group           =   display-name ":" [group-list] ";" [CFWS]
address         =   mailbox / group
address-list    =   (address *("," address)) / obs-addr-list

Instead of address-list being a Vec<Address>
with

enum Address {
    Group(..),
    Mailbox(..),
}

mail-parser will parse all addresses either as a Vec<Group> or Vec<Addr>. While this is not a bug per se, this does lead to additional cycles to map the parsed result to the context-free grammar of the RFC.
The header To: <[email protected]>, <[email protected]> will be parsed as two simple addresses, but if a single group gets added, all of the normal addresses will be parsed into their own group without a name. Because of this, subsequent handling of the results returned by mail-parser has to be robust against these different encodings of what should probably be handled semantically the same. This is even more confusing because if the group is in the middle of regular addresses, this will result in three groups:

To: <[email protected]>, group: (some text);, <[email protected]>

If this in intended, it would make more sense to only ever return groups so users are a bit less prone to shooting themselves in the foot.

@mdecimus
Copy link
Member

mdecimus commented Feb 6, 2025

Hi, I need to check the RFC again but I believe this was done to be more aligned with how JMAP returns addresses.

Regarding the PRs you have submitted recently, thank you! This crate was my first Rust project and has a lot of non-idiomatic Rust or "C-flavoured" code so any improvements are appreciated. Just wanted to say that, unless it is a bug, I won't be able to review PRs until later this year as I am quite busy developing the last features of the mail server.

@sftse
Copy link
Contributor Author

sftse commented Feb 7, 2025

Thanks for reviewing, at whatever pace is suitable to you.

While the exact encoding of the addresses may not be a bug, the fact that reordering them results in a different parse tree that is not simply a reordering of the items might be considered a bug, no?

I can file a PR if you think that is something worth fixing.

@mdecimus
Copy link
Member

mdecimus commented Feb 9, 2025

While the exact encoding of the addresses may not be a bug, the fact that reordering them results in a different parse tree that is not simply a reordering of the items might be considered a bug, no?

This was done to match how JMAP casts address lists to either lists of groups or addresses. But I agree it is not intuitive if the user does not care about JMAP, it should return a list consisting of an enum with the variants Group and Mailbox. I'll keep this open so I can look into improving how addresses are represented.

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

No branches or pull requests

2 participants