Re: [PATCH v3 1/3] send-mail: improve checks for valid_fqdn

[Junio C Hamano <gitster@pobox.com>]

Aditya Garg <gargaditya08@live.com> writes:

> Due to current implementation, I was not able to send emails from
> Ubuntu.

It may be that send-email did not complain, but I have a suspicion
that it the above is only half truth.  We do have an ugly last-ditch
fallback to claim that we are localhost.localdomain, instead of
using a misconfigured maildomain name that servers would not like,
but that name is a meaningless name; from the point of view of the
server, if everybody uses that name, the name loses the meaning as
an identifier.

It is more like due to misconfiguration you couldn't send e-mails,
and by tightening the condition to tell an invalid maildomain name
and have the misconfigured maildomain name that is invalid replaced
with "localhost.localdomain" fallback, you managed to send things
out.

The real fix for individual users may probably be to see how
maildomain_net() and/or maildomain_mta() gives you a bogus
"Macbook.." and fix _that_.  Until that gets fixed, trying to use
"localhost.localdomain" fallback might be a good workaround, but
that is a workaround, not a real solution, isn't it?

When using this fallback, we are at server's mercy; if a server
checks the name you give it against your IP address, for example,
the "localhost.localdomain" fallback may be rejected and you would
need to figure out the real maildomain name the server wants you to
identify as.  What I am worried about this patch the most is if it
is tightening valid_fqdn too much to make a good maildomain name
that real users are currently using to cause them to instead use the
"localhost.localdomain" fallback, and their SMTP servers are not
as lenient as your case and start rejecting their requests.

With a cursory read of this patch, I think basic "at most 63 Alnum
or dash" DNS label defined in the ASCII-only era may be sufficient
for 99% of the users, so I am not too worried by this particular
change (I do not think anybody uses those UUCP!style!addresses
anymore these days), but this patch is an example why we always
want to be careful not to introduce unintended regressions.

> Upon debugging, I found that the SMTP domain was coming out
> to be "MacBook.." and was being considered as valid.

"as valid." -> "as valid, which prevented the fallback
localhost.localdomain from being used."

> As a result
> the script was failing. The debug logs with the failed script are
> below:

Don't give overly long and irrelevant details.  Say something like

	The SMTP exchange started like this:

	<<< 220 ...outlook.office365.com Microsoft ...
	>>> EHLO MacBook..
	<<< 501 5.5.4 Invalid domain name ...

	Notice that an invalid domain name "MacBook.." is sent by
	git-send-email.  We have a fallback code that checks output
	from Net::Domain::domainname() or asking domain method of an
	Net::SMTP instance to detect a misconfigured hostname and
	replace it with fallback "localhost.localdomain", but
	the valid_fqdn apparently is failing to say "MacBook.." is
	not a valid_fqdn.

	With this patch, the rule used in valid_fqdn is tightened,
	the beginning part of the SMTP exchange looked like this:

	<<< 220 ...outlook.office365.com Microsoft ...
	>>> EHLO localhost.localdomain
	<<< 250-250-PN4P287CA0064.outlook.office365.com Hello ...
	<<< 250 ...

	and the server I was using to test this patch was happy with
	the fallback "localhost.localdomain":

perhaps?

Thanks.