To improve support for symbolic port names in netrc files, this changes does the following: - Treat symbolic port names as ports, not protocols in git-credential-netrc - Validate the SMTP server port provided to send-email - Convert the above symbolic port names to their numerical values. Before this change, it was not possible to have a SMTP server port set to "smtps" in a netrc file (e.g. Emacs' ~/.authinfo.gpg), as it would be registered as a protocol and break the match for a "smtp" protocol host, as queried for by git-send-email. Signed-off-by: Maxim Cournoyer <maxim@xxxxxxxxxxxxx> --- .../credential/netrc/git-credential-netrc.perl | 11 +++++++---- contrib/credential/netrc/test.pl | 8 ++++---- git-send-email.perl | 11 +++++++++++ perl/Git.pm | 15 +++++++++++++++ t/t9001-send-email.sh | 7 +++++++ 5 files changed, 44 insertions(+), 8 deletions(-) diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl index 09d77b4f69..72d6b6a974 100755 --- a/contrib/credential/netrc/git-credential-netrc.perl +++ b/contrib/credential/netrc/git-credential-netrc.perl @@ -268,13 +268,16 @@ sub load_netrc { next; } if (defined $nentry->{port}) { - if ($nentry->{port} =~ m/^\d+$/) { - $num_port = $nentry->{port}; - delete $nentry->{port}; - } else { + $num_port = Git::is_port($nentry->{port}); + unless ($num_port) { printf(STDERR "ignoring invalid port `%s' " . "from netrc file\n", $nentry->{port}); } + # Since we've already validated and converted + # the port to its numerical value, do not + # capture it as the `protocol' value, as used + # to be the case for symbolic port names. + delete $nentry->{port}; } # create the new entry for the credential helper protocol diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl index 67a0ede564..8a7fc2588a 100755 --- a/contrib/credential/netrc/test.pl +++ b/contrib/credential/netrc/test.pl @@ -45,7 +45,7 @@ BEGIN diag "Testing with invalid data\n"; $cred = run_credential(['-f', $netrc, 'get'], "bad data"); -ok(scalar keys %$cred == 4, "Got first found keys with bad data"); +ok(scalar keys %$cred == 3, "Got first found keys with bad data"); diag "Testing netrc file for a missing corovamilkbar entry\n"; $cred = run_credential(['-f', $netrc, 'get'], @@ -64,12 +64,12 @@ BEGIN diag "Testing netrc file for a username-specific entry\n"; $cred = run_credential(['-f', $netrc, 'get'], - { host => 'imap', username => 'bob' }); + { host => 'imap:993', username => 'bob' }); -ok(scalar keys %$cred == 2, "Got 2 username-specific keys"); +# Only the password field gets returned. +ok(scalar keys %$cred == 1, "Got 1 username-specific keys"); is($cred->{password}, 'bobwillknow', "Got correct user-specific password"); -is($cred->{protocol}, 'imaps', "Got correct user-specific protocol"); diag "Testing netrc file for a host:port-specific entry\n"; $cred = run_credential(['-f', $netrc, 'get'], diff --git a/git-send-email.perl b/git-send-email.perl index 659e6c588b..502c7d9e04 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -2101,6 +2101,17 @@ sub initialize_modified_loop_vars { } } + # Validate the SMTP server port, if provided. + if (defined $smtp_server_port) { + my $port = Git::is_port($smtp_server_port); + if ($port) { + $smtp_server_port = $port; + } else { + die sprintf(__("error: invalid SMTP port '%s'\n"), + $smtp_server_port); + } + } + # Run the loop once again to avoid gaps in the counter due to FIFO # arguments provided by the user. my $num = 1; diff --git a/perl/Git.pm b/perl/Git.pm index 6f47d653ab..1fa535e1ad 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -1061,6 +1061,21 @@ sub _close_cat_blob { delete @$self{@vars}; } +# Predicate to check whether PORT is a valid port number or service +# name. The numerical value of PORT is returned, else undef if +# invalid. +sub is_port { + my ($port) = @_; + + # Port can be either a positive integer within the 16-bit range... + if ($port =~ /^\d+$/ && $port > 0 && $port <= (2**16 - 1)) { + return $port; + } + + # ... or a symbolic port (service name). + my $num = getservbyname($port, ''); + return defined $num ? $num : undef; +} =item credential_read( FILEHANDLE ) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 0c1af43f6f..3e749175ab 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -201,6 +201,13 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' ' test_cmp expected-cc commandline1 ' +test_expect_failure $PREREQ 'invalid smtp server port value' ' + clean_fake_sendmail && + git send-email -1 --to=recipient@xxxxxxxxxxx \ + --smtp-server-port=bogus-symbolic-name \ + --smtp-server="$(pwd)/fake.sendmail" +' + test_expect_success $PREREQ 'setup expect' " cat >expected-show-all-headers <<\EOF 0001-Second.patch -- 2.49.0