[PATCH] git-send-email: Add --no-validate-email option

[PATCH] git-send-email: Add --no-validate-email option

[Stewart Smith]

The perl Email::Valid module gets things right, but this may not always
be what you want, as can be seen in
https://bugzilla.redhat.com/show_bug.cgi?id=2046203

So, add a --validate-email (default, current behavior) and
the inverse --no-validate-email option to be able to skip the check
while still having the Email::Valid perl module installed.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2046203
Suggested-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Stewart Smith <trawets@amazon.com>
---
 git-send-email.perl   | 9 +++++++++
 t/t9902-completion.sh | 1 +
 2 files changed, 10 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..c75b08f9ce 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -103,6 +103,7 @@ sub usage {
     --quiet                        * Output one line of info per email.
     --dry-run                      * Don't actually send the emails.
     --[no-]validate                * Perform patch sanity checks. Default on.
+    --[no-]validate-email          * Perform email address sanity checks. Default on.
     --[no-]format-patch            * understand any non optional arguments as
                                      `git format-patch` ones.
     --force                        * Send even if safety checks would prevent it.
@@ -281,6 +282,7 @@ sub do_edit {
 my $chain_reply_to = 0;
 my $use_xmailer = 1;
 my $validate = 1;
+my $validate_email = 1;
 my $target_xfer_encoding = 'auto';
 my $forbid_sendmail_variables = 1;
 
@@ -293,6 +295,7 @@ sub do_edit {
     "tocover" => \$cover_to,
     "signedoffcc" => \$signed_off_by_cc,
     "validate" => \$validate,
+    "validateemail" => \$validate_email,
     "multiedit" => \$multiedit,
     "annotate" => \$annotate,
     "xmailer" => \$use_xmailer,
@@ -531,6 +534,8 @@ sub config_regexp {
 		    "no-thread" => sub {$thread = 0},
 		    "validate!" => \$validate,
 		    "no-validate" => sub {$validate = 0},
+		    "validate-email!" => \$validate_email,
+		    "no-validate-email" => sub {$validate_email = 0},
 		    "transfer-encoding=s" => \$target_xfer_encoding,
 		    "format-patch!" => \$format_patch,
 		    "no-format-patch" => sub {$format_patch = 0},
@@ -1132,6 +1137,10 @@ sub extract_valid_address {
 	# check for a local address:
 	return $address if ($address =~ /^($local_part_regexp)$/);
 
+	# Email::Valid isn't always correct, so support a way to bypass
+	# See https://bugzilla.redhat.com/show_bug.cgi?id=2046203
+	return 1 if not $validate_email;
+
 	$address =~ s/^\s*<(.*)>\s*$/$1/;
 	my $have_email_valid = eval { require Email::Valid; 1 };
 	if ($have_email_valid) {
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 31526e6b64..6e363c46f3 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2302,6 +2302,7 @@ test_expect_success PERL 'send-email' '
 	EOF
 	test_completion "git send-email --val" <<-\EOF &&
 	--validate Z
+	--validate-email Z
 	EOF
 	test_completion "git send-email ma" "main "
 '
-- 
2.36.1

Re: [PATCH] git-send-email: Add --no-validate-email option

[Ævar Arnfjörð Bjarmason]

On Sun, Jun 19 2022, Stewart Smith wrote:

> The perl Email::Valid module gets things right, but this may not always
> be what you want, as can be seen in
> https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> So, add a --validate-email (default, current behavior) and
> the inverse --no-validate-email option to be able to skip the check
> while still having the Email::Valid perl module installed.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2046203
> Suggested-by: Todd Zullinger <tmz@pobox.com>
> Signed-off-by: Stewart Smith <trawets@amazon.com>
> ---
>  git-send-email.perl   | 9 +++++++++
>  t/t9902-completion.sh | 1 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..c75b08f9ce 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -103,6 +103,7 @@ sub usage {
>      --quiet                        * Output one line of info per email.
>      --dry-run                      * Don't actually send the emails.
>      --[no-]validate                * Perform patch sanity checks. Default on.
> +    --[no-]validate-email          * Perform email address sanity checks. Default on.
>      --[no-]format-patch            * understand any non optional arguments as
>                                       `git format-patch` ones.
>      --force                        * Send even if safety checks would prevent it.
> @@ -281,6 +282,7 @@ sub do_edit {
>  my $chain_reply_to = 0;
>  my $use_xmailer = 1;
>  my $validate = 1;
> +my $validate_email = 1;
>  my $target_xfer_encoding = 'auto';
>  my $forbid_sendmail_variables = 1;
>  
> @@ -293,6 +295,7 @@ sub do_edit {
>      "tocover" => \$cover_to,
>      "signedoffcc" => \$signed_off_by_cc,
>      "validate" => \$validate,
> +    "validateemail" => \$validate_email,
>      "multiedit" => \$multiedit,
>      "annotate" => \$annotate,
>      "xmailer" => \$use_xmailer,
> @@ -531,6 +534,8 @@ sub config_regexp {
>  		    "no-thread" => sub {$thread = 0},
>  		    "validate!" => \$validate,
>  		    "no-validate" => sub {$validate = 0},
> +		    "validate-email!" => \$validate_email,
> +		    "no-validate-email" => sub {$validate_email = 0},
>  		    "transfer-encoding=s" => \$target_xfer_encoding,
>  		    "format-patch!" => \$format_patch,
>  		    "no-format-patch" => sub {$format_patch = 0},
> @@ -1132,6 +1137,10 @@ sub extract_valid_address {
>  	# check for a local address:
>  	return $address if ($address =~ /^($local_part_regexp)$/);
>  
> +	# Email::Valid isn't always correct, so support a way to bypass
> +	# See https://bugzilla.redhat.com/show_bug.cgi?id=2046203
> +	return 1 if not $validate_email;
> +
>  	$address =~ s/^\s*<(.*)>\s*$/$1/;
>  	my $have_email_valid = eval { require Email::Valid; 1 };
>  	if ($have_email_valid) {
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 31526e6b64..6e363c46f3 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -2302,6 +2302,7 @@ test_expect_success PERL 'send-email' '
>  	EOF
>  	test_completion "git send-email --val" <<-\EOF &&
>  	--validate Z
> +	--validate-email Z
>  	EOF
>  	test_completion "git send-email ma" "main "
>  '

I don't think this patch is what we want to fix this problem: The
git-send-email script should ultimately be trying to pass an address to
a MTA. If you look into its history of Email::Valid use you'll see that
we initially used it unconditionally, then later conditionally to get
rid of the dependency.

I think a better change is to simply get rid of the Email::Valid
dependency, but I'm not 100% sure if there aren't edge cases where our
parsing there isn't something we rely on in other cases.

But in the meantime a more narrow change that I believe solves the issue
for you is:
	
	diff --git a/git-send-email.perl b/git-send-email.perl
	index 5861e99a6eb..1168da43ef2 100755
	--- a/git-send-email.perl
	+++ b/git-send-email.perl
	@@ -1135,7 +1135,11 @@ sub extract_valid_address {
	 	$address =~ s/^\s*<(.*)>\s*$/$1/;
	 	my $have_email_valid = eval { require Email::Valid; 1 };
	 	if ($have_email_valid) {
	-		return scalar Email::Valid->address($address);
	+		my $email = Email::Valid->address(
	+			-address => $address,
	+			-localpart => 0,
	+		);
	+		return $email if $email;
	 	}
	 
	 	# less robust/correct than the monster regexp in Email::Valid,

Of that we just need that "-localpart => 1" part to fix this specific
problem, but I think having the "return $email if $email" is also more
correct, and would solve this bug even without the "-localpart => 0",
i.e. we'll always fall through to trying to parse the address with our
regex.

In any case I think this could really use a corresponding update to the
t/t9001-send-email.sh script, i.e. to test an address with a long local
part.

Re: [PATCH] git-send-email: Add --no-validate-email option

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 4036 bytes --]

On 2022-06-20 at 00:44:27, Stewart Smith wrote:
> The perl Email::Valid module gets things right, but this may not always
> be what you want, as can be seen in
> https://bugzilla.redhat.com/show_bug.cgi?id=2046203

You should explain this in the body of the message, since we generally
want to know the rationale behind the change even if RedHat moves away
from Bugzilla in the future.

You could say something like this:

  The Perl Email::Valid module correctly checks whether an email address
  is syntactically valid.  However, in some cases, people have email
  addresses which are not syntactically valid, such as those where the
  local-part is more than 64 octets, and would like to use those
  addresses despite that fact.

> So, add a --validate-email (default, current behavior) and
> the inverse --no-validate-email option to be able to skip the check
> while still having the Email::Valid perl module installed.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2046203

I don't believe we generally include Fixes headers for external bugs.

> Suggested-by: Todd Zullinger <tmz@pobox.com>
> Signed-off-by: Stewart Smith <trawets@amazon.com>
> ---
>  git-send-email.perl   | 9 +++++++++
>  t/t9902-completion.sh | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..c75b08f9ce 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -103,6 +103,7 @@ sub usage {
>      --quiet                        * Output one line of info per email.
>      --dry-run                      * Don't actually send the emails.
>      --[no-]validate                * Perform patch sanity checks. Default on.
> +    --[no-]validate-email          * Perform email address sanity checks. Default on.
>      --[no-]format-patch            * understand any non optional arguments as
>                                       `git format-patch` ones.
>      --force                        * Send even if safety checks would prevent it.
> @@ -281,6 +282,7 @@ sub do_edit {
>  my $chain_reply_to = 0;
>  my $use_xmailer = 1;
>  my $validate = 1;
> +my $validate_email = 1;
>  my $target_xfer_encoding = 'auto';
>  my $forbid_sendmail_variables = 1;
>  
> @@ -293,6 +295,7 @@ sub do_edit {
>      "tocover" => \$cover_to,
>      "signedoffcc" => \$signed_off_by_cc,
>      "validate" => \$validate,
> +    "validateemail" => \$validate_email,
>      "multiedit" => \$multiedit,
>      "annotate" => \$annotate,
>      "xmailer" => \$use_xmailer,
> @@ -531,6 +534,8 @@ sub config_regexp {
>  		    "no-thread" => sub {$thread = 0},
>  		    "validate!" => \$validate,
>  		    "no-validate" => sub {$validate = 0},
> +		    "validate-email!" => \$validate_email,
> +		    "no-validate-email" => sub {$validate_email = 0},
>  		    "transfer-encoding=s" => \$target_xfer_encoding,
>  		    "format-patch!" => \$format_patch,
>  		    "no-format-patch" => sub {$format_patch = 0},
> @@ -1132,6 +1137,10 @@ sub extract_valid_address {
>  	# check for a local address:
>  	return $address if ($address =~ /^($local_part_regexp)$/);
>  
> +	# Email::Valid isn't always correct, so support a way to bypass
> +	# See https://bugzilla.redhat.com/show_bug.cgi?id=2046203

Email::Valid is in fact correct.  However, the email which you want to
use doesn't conform to the RFC and isn't valid.  So this should probably
say something like, "Allow people to use an email address which is not
valid according to the RFCs if the server accepts it."

I think this patch would be fine as it stands with those changes. Unlike
Ævar, I don't think we should get rid of Email::Valid, just like I don't
think we should get rid of the transfer encoding checks.  I support
warning people before sending invalid emails, especially since I believe
the address in question would not be deliverable through some mail
servers (such as mine).
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] git-send-email: Add --no-validate-email option

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2022-06-20 at 00:44:27, Stewart Smith wrote:
>> The perl Email::Valid module gets things right, but this may not always
>> be what you want, as can be seen in
>> https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> You should explain this in the body of the message, since we generally
> want to know the rationale behind the change even if RedHat moves away
> from Bugzilla in the future.
>
> You could say something like this:
>
>   The Perl Email::Valid module correctly checks whether an email address
>   is syntactically valid.  However, in some cases, people have email
>   addresses which are not syntactically valid, such as those where the
>   local-part is more than 64 octets, and would like to use those
>   addresses despite that fact.
>
>> So, add a --validate-email (default, current behavior) and
>> the inverse --no-validate-email option to be able to skip the check
>> while still having the Email::Valid perl module installed.
>> 
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> I don't believe we generally include Fixes headers for external bugs.

All good comments; I have nothing to add here.

>> +	# Email::Valid isn't always correct, so support a way to bypass
>> +	# See https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> Email::Valid is in fact correct.  However, the email which you want to
> use doesn't conform to the RFC and isn't valid.  So this should probably
> say something like, "Allow people to use an email address which is not
> valid according to the RFCs if the server accepts it."

I had exactly the same reaction.  Again, I have nothing to add here.

Thanks.

>
> I think this patch would be fine as it stands with those changes. Unlike
> Ævar, I don't think we should get rid of Email::Valid, just like I don't
> think we should get rid of the transfer encoding checks.  I support
> warning people before sending invalid emails, especially since I believe
> the address in question would not be deliverable through some mail
> servers (such as mine).

Re: [PATCH] git-send-email: Add --no-validate-email option

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 21 2022, brian m. carlson wrote:

> [[PGP Signed Part:Undecided]]
> On 2022-06-20 at 00:44:27, Stewart Smith wrote:
>> The perl Email::Valid module gets things right, but this may not always
>> be what you want, as can be seen in
>> https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> You should explain this in the body of the message, since we generally
> want to know the rationale behind the change even if RedHat moves away
> from Bugzilla in the future.
>
> You could say something like this:
>
>   The Perl Email::Valid module correctly checks whether an email address
>   is syntactically valid.  However, in some cases, people have email
>   addresses which are not syntactically valid, such as those where the
>   local-part is more than 64 octets, and would like to use those
>   addresses despite that fact.
>
>> So, add a --validate-email (default, current behavior) and
>> the inverse --no-validate-email option to be able to skip the check
>> while still having the Email::Valid perl module installed.
>> 
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> I don't believe we generally include Fixes headers for external bugs.
>
>> Suggested-by: Todd Zullinger <tmz@pobox.com>
>> Signed-off-by: Stewart Smith <trawets@amazon.com>
>> ---
>>  git-send-email.perl   | 9 +++++++++
>>  t/t9902-completion.sh | 1 +
>>  2 files changed, 10 insertions(+)
>> 
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 5861e99a6e..c75b08f9ce 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -103,6 +103,7 @@ sub usage {
>>      --quiet                        * Output one line of info per email.
>>      --dry-run                      * Don't actually send the emails.
>>      --[no-]validate                * Perform patch sanity checks. Default on.
>> +    --[no-]validate-email          * Perform email address sanity checks. Default on.
>>      --[no-]format-patch            * understand any non optional arguments as
>>                                       `git format-patch` ones.
>>      --force                        * Send even if safety checks would prevent it.
>> @@ -281,6 +282,7 @@ sub do_edit {
>>  my $chain_reply_to = 0;
>>  my $use_xmailer = 1;
>>  my $validate = 1;
>> +my $validate_email = 1;
>>  my $target_xfer_encoding = 'auto';
>>  my $forbid_sendmail_variables = 1;
>>  
>> @@ -293,6 +295,7 @@ sub do_edit {
>>      "tocover" => \$cover_to,
>>      "signedoffcc" => \$signed_off_by_cc,
>>      "validate" => \$validate,
>> +    "validateemail" => \$validate_email,
>>      "multiedit" => \$multiedit,
>>      "annotate" => \$annotate,
>>      "xmailer" => \$use_xmailer,
>> @@ -531,6 +534,8 @@ sub config_regexp {
>>  		    "no-thread" => sub {$thread = 0},
>>  		    "validate!" => \$validate,
>>  		    "no-validate" => sub {$validate = 0},
>> +		    "validate-email!" => \$validate_email,
>> +		    "no-validate-email" => sub {$validate_email = 0},
>>  		    "transfer-encoding=s" => \$target_xfer_encoding,
>>  		    "format-patch!" => \$format_patch,
>>  		    "no-format-patch" => sub {$format_patch = 0},
>> @@ -1132,6 +1137,10 @@ sub extract_valid_address {
>>  	# check for a local address:
>>  	return $address if ($address =~ /^($local_part_regexp)$/);
>>  
>> +	# Email::Valid isn't always correct, so support a way to bypass
>> +	# See https://bugzilla.redhat.com/show_bug.cgi?id=2046203
>
> Email::Valid is in fact correct.  However, the email which you want to
> use doesn't conform to the RFC and isn't valid.  So this should probably
> say something like, "Allow people to use an email address which is not
> valid according to the RFCs if the server accepts it."

That's fair, but that rationale is quite disconnected from how the code
works now. You happen to get that check if you have Email::Valid
installed, otherwise not.

So if it's a use-case we care about we should make it a hard dependency.

> I think this patch would be fine as it stands with those changes. Unlike
> Ævar, I don't think we should get rid of Email::Valid, just like I don't
> think we should get rid of the transfer encoding checks.  I support
> warning people before sending invalid emails, especially since I believe
> the address in question would not be deliverable through some mail
> servers (such as mine).

Would this be addressed by instead opening a connection to the server,
and seeing if it is willing to accept these addresess on a "RCPT TO"
line?

Which I think is what would happen anyway as you try to send the E-Mail,
I'm not sure what distinction you're drawing here (but I haven't looked
deeply into the control flow here).

I.e. if your MTA isn't going to accept an address that we regex match,
isn't it going to error when you try to send the mail, why isn't it
better and more reliable to offload more of that sort of validation to
the MTA?

As this report notes that can lead to cases where there's a mismatch
between the two, i.e. we can't format the E-Mail, but the MTA would be
happy to send it for us.

Re: [PATCH] git-send-email: Add --no-validate-email option

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

On 2022-06-21 at 22:12:24, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Jun 21 2022, brian m. carlson wrote:
> 
> > Email::Valid is in fact correct.  However, the email which you want to
> > use doesn't conform to the RFC and isn't valid.  So this should probably
> > say something like, "Allow people to use an email address which is not
> > valid according to the RFCs if the server accepts it."
> 
> That's fair, but that rationale is quite disconnected from how the code
> works now. You happen to get that check if you have Email::Valid
> installed, otherwise not.
> 
> So if it's a use-case we care about we should make it a hard dependency.

Git has traditionally tried to avoid having lots of hard dependencies on
Perl modules.  For example, Perl modules are a hassle with Homebrew.

Most packagers prefer to enable the full suite of Perl modules, but it
is a bit nicer to not make it mandatory.  However, if you feel strongly,
we can change that.

> > I think this patch would be fine as it stands with those changes. Unlike
> > Ævar, I don't think we should get rid of Email::Valid, just like I don't
> > think we should get rid of the transfer encoding checks.  I support
> > warning people before sending invalid emails, especially since I believe
> > the address in question would not be deliverable through some mail
> > servers (such as mine).
> 
> Would this be addressed by instead opening a connection to the server,
> and seeing if it is willing to accept these addresess on a "RCPT TO"
> line?

No, because that tells you whether your smarthost will accept it.  There
are often multiple different parties involved in SMTP (including various
filtering programs, smarthosts, and relays) and all you'll know is
whether the first one of them accepts it.  I have seen systems where
there could well be four or five stages of processing an email before
it even left the host.

This is no different than with things like lines longer than 998 octets,
SMTPUTF8, or various other SMTP protocol issues.  The only surefire way
to know that your email will be accepted by the remote system is to
speak the protocol properly.  If people want an option to break the
protocol, that's fine, but we should try to avoid doing that by default.

The benefit to using Email::Valid in most cases is it prevents lots of
obvious mistakes, where the email address is clearly syntactically
invalid and undeliverable, especially due to bad author and committer
metadata.  The Linux kernel history shows that this is not uncommon and
it's useful to avoid this problem so you don't have to blow up people's
inboxes with a v2 right away just because you got an invalid address on
v1.  (Says the guy who has had to do almost exactly this.)
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] git-send-email: Add --no-validate-email option

[Ævar Arnfjörð Bjarmason]

On Wed, Jun 22 2022, brian m. carlson wrote:

> [[PGP Signed Part:Undecided]]
> On 2022-06-21 at 22:12:24, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Tue, Jun 21 2022, brian m. carlson wrote:
>> 
>> > Email::Valid is in fact correct.  However, the email which you want to
>> > use doesn't conform to the RFC and isn't valid.  So this should probably
>> > say something like, "Allow people to use an email address which is not
>> > valid according to the RFCs if the server accepts it."
>> 
>> That's fair, but that rationale is quite disconnected from how the code
>> works now. You happen to get that check if you have Email::Valid
>> installed, otherwise not.
>> 
>> So if it's a use-case we care about we should make it a hard dependency.
>
> Git has traditionally tried to avoid having lots of hard dependencies on
> Perl modules.  For example, Perl modules are a hassle with Homebrew.
>
> Most packagers prefer to enable the full suite of Perl modules, but it
> is a bit nicer to not make it mandatory.  However, if you feel strongly,
> we can change that.

Not really, since I'm proposing to remove it :)

But if we wanted to have our cake & eat it too we could use the
perl/FromCPAN tree for this purpose, i.e. ship the module with git, or
copy over the relevant parts of the check.

>> > I think this patch would be fine as it stands with those changes. Unlike
>> > Ævar, I don't think we should get rid of Email::Valid, just like I don't
>> > think we should get rid of the transfer encoding checks.  I support
>> > warning people before sending invalid emails, especially since I believe
>> > the address in question would not be deliverable through some mail
>> > servers (such as mine).
>> 
>> Would this be addressed by instead opening a connection to the server,
>> and seeing if it is willing to accept these addresess on a "RCPT TO"
>> line?
>
> No, because that tells you whether your smarthost will accept it.  There
> are often multiple different parties involved in SMTP (including various
> filtering programs, smarthosts, and relays) and all you'll know is
> whether the first one of them accepts it.  I have seen systems where
> there could well be four or five stages of processing an email before
> it even left the host.

Yes, it wouldn't be a perfect solution by any means, but I understood
your upthread "servers (such as mine)" to mean your local relay.

> This is no different than with things like lines longer than 998 octets,
> SMTPUTF8, or various other SMTP protocol issues.  The only surefire way
> to know that your email will be accepted by the remote system is to
> speak the protocol properly.  If people want an option to break the
> protocol, that's fine, but we should try to avoid doing that by default.

I'm not suggesting that we avoid doing it, but that we outsource the
problem to the SMTP server which we're going to be talking to shortly
anyway.

> The benefit to using Email::Valid in most cases is it prevents lots of
> obvious mistakes, where the email address is clearly syntactically
> invalid and undeliverable, especially due to bad author and committer
> metadata.  The Linux kernel history shows that this is not uncommon and
> it's useful to avoid this problem so you don't have to blow up people's
> inboxes with a v2 right away just because you got an invalid address on
> v1.  (Says the guy who has had to do almost exactly this.)

I've done that too, but not in a way where either Email::Valid or "mail
from" would help me, i.e. I just typo'd the local part of a gmail
address or whatever, which you won't know about until you get the
bounces (or incorrect delivery).

But it really seems like you're replying to a suggestion I'm not making,
which is that we just don't do any validation at all.

I'm suggesting that we replace our own validation with that of the SMTP
server's, yes they're don't 1=1 correspond, but I think the part of the
Venn diagram of where that matters is too small to worry about.

It has the advantage of side-stepping issues with not having
Email::Valid, as well as those cases where we're being overzelous about
RFC validation, but our local SMTP is willing to try to deliver the
mail.

It's not like authors of MTAs haven't heard of that character limit, but
they're also aware that that certain parts of the spec are loosely
enforced, and that trying delivery is often better than rejecting a mail
out of RFC pedantry.

Re: [PATCH] git-send-email: Add --no-validate-email option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I'm suggesting that we replace our own validation with that of the SMTP
> server's, yes they're don't 1=1 correspond, but I think the part of the
> Venn diagram of where that matters is too small to worry about.
>
> It has the advantage of side-stepping issues with not having
> Email::Valid, as well as those cases where we're being overzelous about
> RFC validation, but our local SMTP is willing to try to deliver the
> mail.
>
> It's not like authors of MTAs haven't heard of that character limit, but
> they're also aware that that certain parts of the spec are loosely
> enforced, and that trying delivery is often better than rejecting a mail
> out of RFC pedantry.

I am not sure if that is a healthy direction to go.

If a local outbound relay is written with the knowledge that it will
never be talking to the SMTP at the final mailbox directly, I would
expect that it may not implement any validation at all, relying on
the "next hop" smarthost to reject anything invalid it throws at it.

So...