Hi Julian >> Autoconfiguring SMTP server settings is a common feature present in many >> email clients. In order to get the correct SMTP server settings easily, >> this commit adds a `--get-smtp-server` option to `git send-email`. This >> option attempts to fetch the SMTP server settings for a given email address >> via the following steps: >> >> 1. It first tries to fetch the settings from Mozilla's ISPDB at >> `https://autoconfig.thunderbird.net/v1.1/[domain]`. > > We should first check autoconfig and then move to the 3rd party > database, this is how thunderbird[0] and aerc[1] do it. > >> >> 2. If that fails, it attempts to fetch the autoconfig file from the email >> provider's autoconfig URL, which is typically in the format >> `https://autoconfig.[domain]/mail/config-v1.1.xml?emailaddress=[email]`. > > The documentation mentions using `DOMAIN/.well-known/autoconfig/mail/` > as an alternative to the autoconfig subdomain, what do you think about > supporting that? > >> >> 3. If that also fails, it falls back to checking the MX records of the >> domain used in the email address to find the SMTP server. It can be >> useful in case of emails with custom domains. It attempts to guess >> the correct domain for the email from the MX records, and repeats the >> first 2 steps with the guessed domain. >> >> This feature is heavily inpired by the autoconfig feature in Mozilla > > s/inpired/inspired > > >> +sub parse_config { >> + require XML::LibXML; >> + my ($xml, $email) = @_; >> + my $parser = XML::LibXML->new; >> + my $doc = eval { $parser->load_xml(string => $xml) }; >> + die "Failed to parse XML\n" unless $doc; >> + my $config_num = 0; >> + my $smtp_encryption_config; >> + my $smtp_user_config; >> + >> + foreach my $outgoing ($doc->findnodes('//outgoingServer')) { >> + $config_num++; >> + if ($outgoing->findvalue('./socketType') eq 'SSL') { >> + $smtp_encryption_config = 'ssl'; >> + } elsif ($outgoing->findvalue('./socketType') eq 'STARTTLS') { >> + $smtp_encryption_config = 'tls'; >> + } else { >> + $smtp_encryption_config = 'plain'; > > 'plain' is unencrypted, I think this should be accompanied by a big > warning. > >> + } >> + >> + if ($outgoing->findvalue('./username') eq '%EMAILADDRESS%') { >> + $smtp_user_config = $email; >> + } elsif ($outgoing->findvalue('./username') eq '%EMAILLOCALPART%') { >> + $smtp_user_config = (split /@/, $email)[0]; >> + } elsif ($outgoing->findvalue('./username') eq '%EMAILDOMAIN%') { >> + $smtp_user_config = (split /@/, $email)[1]; >> + } else { >> + $smtp_user_config = $outgoing->findvalue('./username'); >> + } >> + >> + print "\nConfiguration $config_num:\n"; >> + print " Server: ", $outgoing->findvalue('./hostname'), "\n"; >> + print " Port: ", $outgoing->findvalue('./port'), "\n"; >> + print " Encryption: ", $smtp_encryption_config, "\n"; >> + print " Username: ", $smtp_user_config, "\n"; > > The new option only gives you the needed SMTP configuration, as a > user you still need to apply them and to do that you will need to > look up how. We could help the user here and give them copy and > paste commands similar to when trying to commit without having an > identity set. > > The XML file also contains authentication details, what do you think > about processing those? That would also allow adding references to the > documentation in case it is OAuth2. > >> + } >> +} >> + >> +if ($get_smtp_server) { >> + require URI::Escape; >> + print "Enter your email address: "; >> + chomp(my $email = <STDIN>); > > Someone sending out emails will most likely already have set up > `user.email` in their gitconfig. We could just use that instead of > prompting for user input, or at least suggest it as a default. > > If you don't have an SMTP server configured then `git send-email` > will default to `localhost` and fail if you are not running a > local SMTP server with: `Unable to initialize SMTP properly. > Check config and use --smtp-debug.`. I would suggest altering the > message pointing the user to the new option I've sent a v3 with hopefully your suggested changes addressed. Let me know if something is missing!
