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

MailboxAddress validation #682

Closed
Archanian opened this issue Jun 10, 2021 · 9 comments
Closed

MailboxAddress validation #682

Archanian opened this issue Jun 10, 2021 · 9 comments
Labels
compatibility Compatibility with existing software

Comments

@Archanian
Copy link

Archanian commented Jun 10, 2021

I have come across a specific case of email addresses that are in fact real - those containing a local part ending in a . character - yet I can't seem to find a way to loosen the validation of MailboxAddress to allow such addresses.

For example: foo.bar.@gmail.com - this may not be strictly valid according to RFC spec (?) but Gmail will happily deliver to such a mailbox (and we have in fact had users with gmail mailboxes like this, hence the reason for this request).

I've tried the various settings on ParserOptions - RfcComplianceMode.Loose doesn't seem to do the trick, and nothing else seems to have an effect. Is there something I'm missing? It doesn't seem possible to skip validation on this class, could that be added as a flag, or would it be possible to extend the validation to allow such addresses?

Thanks.

@jstedfast
Copy link
Owner

Seems that it doesn't allow you to create such an account, though:

google-account-creation

@jstedfast
Copy link
Owner

How are users inputting this? Can you just split the string on the '@' character and trim trailing '.'s from the first half?

@Archanian
Copy link
Author

Archanian commented Jun 10, 2021

It's true, you can't create an address like that currently, nor can you send to an address like that via Gmail's UI. However, there are indeed users with addresses like this, we have successfully delivered mail to them (we fallback to some other, older libraries in cases where MailKit fails).

We are generally quite permissive in what we allow users to submit as as mailbox, since basically anything can be in there despite what the RFC might say. This isn't something we want to strip, since we would be excluding legitimate addresses (our product is an email delivery service, so this would be undesirable). It is better to attempt delivery (and bounce if necessary) from our point of view, but MimeKit doesn't seem to have a way to allow it.

@jstedfast jstedfast added the compatibility Compatibility with existing software label Jun 10, 2021
@jstedfast
Copy link
Owner

https://support.google.com/mail/answer/7436150?hl=en#:~:text=Emails%20sent%20to%20any%20dotted%20version%20of%20your,dots%20aren%27t%20why%20you%20got%20someone%20else%27s%20mail.

Is it possible for you to just strip out .'s in the email addresses you are giving to MimeKit if they have an @gmail.com domain?

It sounds like only gmail accounts exhibit this behavior, right?

@atheken
Copy link

atheken commented Jun 12, 2021

Hi @jstedfast, I work with @Archanian, so I can also speak to the issue here:

The problem with modifying the mailbox is that the dot-stripping behavior that gmail or other providers have is not guaranteed. Gmail (and GSuite) domains have had this mailbox behavior since the beginning, but it's dangerous for us to deliver to any address except for the one that our customers provide to us, no matter how well established the current behavior is (i.e. imagine delivering a confidential email to the wrong mailbox because some system routed based on that trailing .).

We do note that many MUAs do block adding such an address to the recipient list. However, mail submitted via SMTP to gmail (and likely other providers) will gladly accept and route these to the "normalized" mailbox. Frequently, these addresses are supplied by the recipient themself, such as putting in an address for an order confirmation email when making a purchase online.

We tend to want to make a best effort to deliver the mail to the destination mailbox, even if it doesn't conform to the RFCs. To handle such cases, it would be helpful to have an escape hatch when creating MailboxAddresses (in the form of setting a lax validation requirement, or being able to explicitly set/override the Address).

(And also, hello again, from PHL)

@jstedfast
Copy link
Owner

I've actually got a patch locally, I just worry about breakages and such.

If I push said changes to a branch, would you guys be able to build said branch and test the patch?

jstedfast added a commit that referenced this issue Jun 12, 2021
Technically, this is illegal in an addr-spec token, but I am told that these occur in the wild.

Fixes issue #682
@atheken
Copy link

atheken commented Jun 14, 2021

Thanks @jstedfast - we've actually been kicking around forking and doing our own package builds, so we can do that and let you know how it goes with this branch.

We're also happy to provide patches for these sorts of things, we just weren't sure if this was outside of the goals/scope of the project.

@jstedfast
Copy link
Owner

@atheken let me know how this is working out for you. If it's working well, I may try to clean it up a bit more and maybe introduce a RfcComplianceMode.Looser or something that this will fall into and then merge.

jstedfast added a commit that referenced this issue Jul 21, 2021
Technically, this is illegal in an addr-spec token, but I am told that these occur in the wild.

Introduces an RfcComplianceMode.Looser enum value to enable this feature.

Fixes issue #682
jstedfast added a commit that referenced this issue Jul 21, 2021
Technically, this is illegal in an addr-spec token, but I am told that these occur in the wild.

Introduces an RfcComplianceMode.Looser enum value to enable this feature.

Fixes issue #682
@jstedfast
Copy link
Owner

I've merged this into master, but added a new RfcComplianceMode.Looser that needs to be used to allow it (set it on parserOptions.AddressParserComplianceMode).

This will likely be pushed in a 2.14 release when I get a chance.

Long term, in a "3.0" release, I plan to make it possible to make changes to the ParserOptions.Default properties (currently throws an exception if you try). This should make it a lot easier to set some global preferences instead of having to keep around a custom ParserOptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility Compatibility with existing software
Projects
None yet
Development

No branches or pull requests

3 participants