Junio C Hamano <gitster@xxxxxxxxx> writes:
> So please explain why asking your local MTA (either mailhost or localhost)
> how it is configured to identify to other MTA is a good way to obtain the
> HELO domain you should give $smtp_server. Your answer could be "I expect
> that most people use $smtp_server set to 'mailhost' or 'localhost' and
> have the MTA configured to talk with the outside world." (which by the way
> I do not think is true for most people). Or it could be "Xsmtp MTA that
> is used by many people to send out mails through their ISP's mailserver
> uses the same trick to solve this issue".
The code is based on Perl library Test::Reporter
http://search.cpan.org/~dagolden/Test-Reporter-1.55/lib/Test/Reporter.pm
/usr/share/perl5/Test/Reporter/Mail/Util.pm ==> sub _maildomain ()
Function _maildomain() in Test::Reporter uses much more intesive
tests by scanning all kinds of files, like checking sendmail(1),
smail(1) etc. configuration files before querying the MTA. I used
only a portion of it, those parts that I was confortable with to
handle.
Where to read the FQDN?
git-send-email contacts MTA to see that it thinks the caller host is
named, because -- to my knowledge -- the FQDN information is not
readily available elsewhere in all systems. The MTA does reverse IP
lookup (DNS) for git-send-email to read.
Why FQDN is passed to MTA?
Tightly configured MTAs require that the caller gives a real DNS
domain name that corresponds the IP address in the handshake. This
prevents spammers from trying to hide their identity.
> Side note. Apparently this seems to be a common issue. For example,
> msmtp has this configurable via command line and configuration option:
>
> domain argument
> Use this command to set the argument of the SMTP EHLO (or LMTP
> LHLO) command. The default is localhost (stupid, but working).
> Possible choices are the domain part of your mail address
> (provider.example for joe@xxxxxxxxxxxxxxxx) or the fully quali‐
> fied domain name of your host (if available).
>
> Interestingly, one of the suggestions above is to derive it from the From
> address.
>
> In any case, I want to hear your justification.
>
>> + --smtp-domain <str> * The domain name sent to HELO/EHLO handshake
>
> I think this addition makes sense. I suspect that we would also want to
> have sendemail.smtpdomain configuration variable. They of course need to
> be documented.
Sure.
> +my $MAIL_DOMAIN; # See maildomain()
> Why uppercase?
It might be good to convert all global variables to de facto UPPERCASE.
What do you think? (a separate patch?).
> + return $MAIL_DOMAIN if $MAIL_DOMAIN;
> +
> + my $maildomain;
> Looks like a funny indent here...
SP vs. TAB in diff. Fixed.
> Please drop extra SP immediately after '('
Done.
> But as I already said, I tend to think the logic implemented by this part
> is of dubious validity.
Does the above explation address this? I concluded that if the Perl
Module developers use MTA to find out the FQDN, it must be a working
solution. Especially as the code is in the Test:: module libraries.
>> @@ -917,6 +962,8 @@ X-Mailer: git-send-email $gitversion
>> }
>> }
>>
>> + my $maildomain;
>> +
>> if ($dry_run) {
>> # We don't want to send the email.
>> } elsif ($smtp_server =~ m#^/#) {
>> @@ -936,13 +983,18 @@ X-Mailer: git-send-email $gitversion
>> ...
>> + $maildomain = maildomain() || "localhost.localdomain";
>> + $smtp ||= Net::SMTP::SSL->new($smtp_server,
>> + Hello => $maildomain,
>> + Port => $smtp_server_port);
>
> Why not use the same structure as how lifetime is defined for $smtp
> variable? IOW,
>
> $mail_domain || = maildomain();
> $smtp ||= ...
This is now completely different. See new maildomain().
> without an extra local $maildomain variable? If you do so, then
>
> - You can lose the $maildomain variable local to this function;
It's needed in the error message (see variable's scope):
if (!$smtp) {
die "Unable to initialize SMTP properly. Check config and use --smtp-debug. ",
"VALUES: server=$smtp_server ",
"encryption=$smtp_encryption ",
!! "maildomain=$maildomain",
defined $smtp_server_port ? "port=$smtp_server_port" : "";
> - "return what the user configured" does not have to be in the maildomain
> sub;
Refactored.
> - the maildomain sub can return "localhost.localdomain" as a fallback
> default; and
Refactored.
> - various callsites of maildomain sub do not have to repeat the fallback
> default like your patch does.
>
>> @@ -962,9 +1014,10 @@ X-Mailer: git-send-email $gitversion
>> }
>>
>> if (!$smtp) {
>> - die "Unable to initialize SMTP properly. Check config. ",
>> + die "Unable to initialize SMTP properly. Check config and use --smtp-debug. ",
>
> This part is a good addition, but it needs to be in the earlier patch, no?
The --smtp-debug was introduced in the current patch along with maildomain().
See next message for a reworked patch.
Jari
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html