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

Address parsing, error checking and sanitization #74

Open
hoylen opened this issue Jan 23, 2019 · 7 comments
Open

Address parsing, error checking and sanitization #74

hoylen opened this issue Jan 23, 2019 · 7 comments
Assignees
Labels
help wanted (easy) next This feature/bug is scheduled for the next release.

Comments

@hoylen
Copy link

hoylen commented Jan 23, 2019

A previous version of Mailer (e.g. version 1.1.0) had significant syntax checking of email addresses for validity against RFC 5822, as well as parsing an address into its components. This is no longer available in Mailer v2.2.0.

The old address.dart has been replaced by a basic new address.dart where the address can be any String value.

Is it possible to put the error checking code back? It is very useful to discover that an email address (e.g. the user has entered) is valid; rather than finding out only when the program tries to send an email and it fails (or worse, not finding out it failed).

@close2 close2 self-assigned this Feb 14, 2019
@close2
Copy link
Collaborator

close2 commented Feb 14, 2019

My plan is to move the old validation code into its own library.

I am not sure if mailer should then depend on the mail-address library or if providing an interface for a validator is the way to go.

I personally think that the old verification code allowed to many address formats and would use a simpler and stricter validation code by default.

@close2 close2 added next This feature/bug is scheduled for the next release. help wanted (easy) labels Feb 14, 2019
@hoylen
Copy link
Author

hoylen commented Feb 14, 2019

An "own library" as a part of the Mailer Dart package, or a separate Dart package just containing it?

Actually, I spend some time yesterday updating the old address.dart (the one with strict RFC 5822 verification) to Dart 2, since I needed the ability to parse an email address into its components. If it is to be separate Dart package, I can do that. If it is to be a library in Mailer, I can send you the updated code.

I personally think that the old verification code allowed to many address formats and would use a simpler and stricter validation code by default.

I think what you are calling "old" and "new" are the reverse of the old/new I have linked to in my comment. When I say "old", I'm referring to the implementation in Mailer 1.1.0 that did strict RFC 5822 verification. The current (Mailer 2.2.0) code is the one that allows too many address formats: it is just any String value.

@close2
Copy link
Collaborator

close2 commented Feb 15, 2019

I think it should be separate dart package.

I actually really referred to the verification code in Mailer 1.1.0. It allowed group-addresses and quoted local strings.

The current (v2) validation is of course practically non existent:

bool _validMailAddress(String ma) {
  var split = ma.split('@');
  return split.length == 2 &&
      split.every((part) => part.isNotEmpty && _printableCharsOnly(part));
}

@close2
Copy link
Collaborator

close2 commented Feb 15, 2019

My current plan is to implement the Builder pattern mentioned in another issue with the possibility to specify a validation function:

Message msg = Message.Builder(addressValidator: validationFunction)

another possibility would be to simply allow setting a global variable:

addressValidation = validationFunction;

@hoylen hoylen closed this as completed Feb 18, 2019
@hoylen
Copy link
Author

hoylen commented Feb 18, 2019

Is there a good reason why the address validation needs to be in a separate package?

If the Mailer package is used for sending emails, why would anyone want to use any other validation than (a complete and correctly implemented) validator provided with the package? That is, why would they want to use more loose validator (i.e. one that allows non-email addresses to pass); or use a more strict validator (i.e. one that rejects some valid email addresses)?

I can only think of one reason: to allow someone to use the address validation code when they don't want to use/import the Mailer package. In that situation, Mailer can simply import the Address package. I can't imagine why someone would want to write their own "worse" validator (i.e. one that allows addresses that emails can't be sent to; or one that rejects addresses that are valid).

@hoylen hoylen reopened this Feb 18, 2019
@close2
Copy link
Collaborator

close2 commented Feb 18, 2019

I guess we should use the most correct validator by default.
In my opinion the validator should still be a separate package. For instance Angular apps could then easily use the same validation code.

Why not all valid addresses:
A lot of mail addresses are valid but are practically not used and mostly rejected by SMTP-servers.
Here are some e-mail addresses I would personally disable in my programs:
"Abc@def"@example.com,
email@[123.123.123.123]
" "@example.org

@hoylen
Copy link
Author

hoylen commented Feb 18, 2019

Yes, that makes sense.

Defaulting to a useful validator is good too. That way, users can immediately start using the Mailer package (without having to hunt around for other packages, or write their own validator), but have the flexibility to do so if they want to.

tomyeh added a commit to tomyeh/mailer that referenced this issue May 13, 2020
tomyeh added a commit to tomyeh/mailer that referenced this issue May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted (easy) next This feature/bug is scheduled for the next release.
Projects
None yet
Development

No branches or pull requests

2 participants