{"thread":{"id":"63677","subject":"[PATCH] contrib: Honor symbolic port in git-credential-netrc.","startedAt":"2025-06-20T04:13:18Z","lastAt":"2025-06-26T01:15:54Z","messageCount":31,"participants":["Maxim Cournoyer","Junio C Hamano","Andreas Schwab","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"520462","messageId":"20250620041239.27839-1-maxim@guixotic.coop","threadId":"63677","inReplyTo":null,"subject":"[PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-20T04:12:39Z","receivedAt":"2025-06-20T04:13:18Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Symbolic ports were previously silently dropped, which made it\nimpossible to use them with git-credential-netrc. This is a supported\nuse case according to 'man git-send-email', for --smtp-server-port:\n\n   [...] symbolic port names (e.g. \"submission\" instead of 587) are\n   also accepted.\n---\n contrib/credential/netrc/git-credential-netrc.perl | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 9fb998ae09..ad06000b9f 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -1,4 +1,4 @@\n-#!/usr/bin/perl\n+#!/usr/bin/env perl\n \n use strict;\n use warnings;\n@@ -267,7 +267,9 @@ sub load_netrc {\n \t\tif (!defined $nentry->{machine}) {\n \t\t\tnext;\n \t\t}\n-\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n+\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^[[:alnum:]]+$/) {\n+\t\t\t# Port may be either an integer or a symbolic\n+\t\t\t# name, e.g. \"smtps\".\n \t\t\t$num_port = $nentry->{port};\n \t\t\tdelete $nentry->{port};\n \t\t}\n\nbase-commit: 9520f7d9985d8879bddd157309928fc0679c8e92\n-- \n2.49.0\n\n"},{"id":"520488","messageId":"xmqqmsa27cdn.fsf@gitster.g","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-20T13:48:04Z","receivedAt":"2025-06-20T13:48:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maxim Cournoyer <maxim@guixotic.coop> writes:\n\n> Subject: Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.\n\nPlease downcase \"Honor\" and drop the final full stop, per convention\n(see \"git shortlog --no-merges --since=2.months\" for examples).\n\n> Symbolic ports were previously silently dropped, which made it\n> impossible to use them with git-credential-netrc.\n\nWouldn't it make sense to issue a warning message when a defined\n$nentry->{port} is not unrecognized?  Wouldn't it make sense to\ndo so even before we add this new feature?\n\n> This is a supported\n> use case according to 'man git-send-email', for --smtp-server-port:\n>\n>    [...] symbolic port names (e.g. \"submission\" instead of 587) are\n>    also accepted.\n> ---\n\nMissing sign-off?  See Documentation/SubmittingPatches\n\n>  contrib/credential/netrc/git-credential-netrc.perl | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\n> index 9fb998ae09..ad06000b9f 100755\n> --- a/contrib/credential/netrc/git-credential-netrc.perl\n> +++ b/contrib/credential/netrc/git-credential-netrc.perl\n> @@ -1,4 +1,4 @@\n> -#!/usr/bin/perl\n> +#!/usr/bin/env perl\n\nAn unrelated change to introduce the use of /usr/bin/env in this\npatch is unwelcome.  Besides, this is a source that is processed\nby the nearby Makefile, which uses the toplevel genererate-perl.sh\nto turn the \"#!.../perl\" line to name the correct $PERL_PATH before\nthe build product gets installed, so I suspect that this change is\ntotally unnecessary.\n\n> @@ -267,7 +267,9 @@ sub load_netrc {\n>  \t\tif (!defined $nentry->{machine}) {\n>  \t\t\tnext;\n>  \t\t}\n> -\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n> +\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^[[:alnum:]]+$/) {\n> +\t\t\t# Port may be either an integer or a symbolic\n> +\t\t\t# name, e.g. \"smtps\".\n\nDo we know symbolic port names are always limited to alnums?  Or on\nsome systems some byte values in the fringe, like \"_\" or \"-\", are\nalso allowed?\n\nOverall, I think it is a worthwhile to address this issue.  I just\ndo not want a patch that says \"alnum seems to be good enough to\ncover the ports I happen to care about\" (or a patch does not even\nsay how it came to the conclusion to use :alnum:)---we should do\nbetter and explain this change with something like \"from THIS\nSOURCE, the names users may want to use come from this set of names,\nwhere THESE are defined as valid characters, hence we allow not just\ndigits (for numeric ports), but allowing also :alnum: is sufficient\nto cover these symbolic names\".\n\nThanks.\n\n\n>  \t\t\t$num_port = $nentry->{port};\n>  \t\t\tdelete $nentry->{port};\n>  \t\t}\n>\n> base-commit: 9520f7d9985d8879bddd157309928fc0679c8e92\n"},{"id":"520492","messageId":"87pleyijbg.fsf@igel.home","threadId":"63677","inReplyTo":"xmqqmsa27cdn.fsf@gitster.g","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2025-06-20T14:22:43Z","receivedAt":"2025-06-20T14:31:29Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Jun 20 2025, Junio C Hamano wrote:\n\n> Do we know symbolic port names are always limited to alnums?  Or on\n> some systems some byte values in the fringe, like \"_\" or \"-\", are\n> also allowed?\n\nValid service names are documented in RFC6335.  Specifically it allows\nhyphens, but not underscores.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"520507","messageId":"xmqqtt4a2wr3.fsf@gitster.g","threadId":"63677","inReplyTo":"87pleyijbg.fsf@igel.home","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-20T16:39:12Z","receivedAt":"2025-06-20T16:39:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab <schwab@linux-m68k.org> writes:\n\n> On Jun 20 2025, Junio C Hamano wrote:\n>\n>> Do we know symbolic port names are always limited to alnums?  Or on\n>> some systems some byte values in the fringe, like \"_\" or \"-\", are\n>> also allowed?\n>\n> Valid service names are documented in RFC6335.  Specifically it allows\n> hyphens, but not underscores.\n\nYes, something like that is what I wanted the original contributor\nto write in the proposed log message to explain how the loosened\nrule for accepted \"port\" was chosen (and relize that alnum is not\nsufficient).\n\nThanks.\n"},{"id":"520508","messageId":"aFWwez5OEgLt0vRU@fruit.crustytoothpaste.net","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-06-20T19:03:23Z","receivedAt":"2025-06-20T19:03:25Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-06-20 at 04:12:39, Maxim Cournoyer wrote:\n> Symbolic ports were previously silently dropped, which made it\n> impossible to use them with git-credential-netrc. This is a supported\n> use case according to 'man git-send-email', for --smtp-server-port:\n> \n>    [...] symbolic port names (e.g. \"submission\" instead of 587) are\n>    also accepted.\n\nDoes this work with credential managers in general (that is, in\nnon-email contexts, such as HTTP)?  Also, do credential managers in\ngeneral properly find credentials when they're stored in one form and\nlooked up in another?  If so, is that still true when the lookup is in\nthe URL form (e.g., `smtp://mail.example.com:587/`)?  Is this documented\nto work in the credential manual page?  (To be clear, I would be very\nsurprised if the answer to any of these were \"yes\" because I've\nliterally never seen this usage before with Git, but I am open to\nupdating my knowledge if that's the case.)\n\nIf not, then I think the proper thing to do is to have `git send-email`\nrewrite the name into a port instead of having the netrc credential\nhelper learn to handle non-numeric ports.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"520533","messageId":"87bjqhgr47.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"aFWwez5OEgLt0vRU@fruit.crustytoothpaste.net","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-21T13:29:28Z","receivedAt":"2025-06-21T13:29:45Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi,\n\n\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On 2025-06-20 at 04:12:39, Maxim Cournoyer wrote:\n>> Symbolic ports were previously silently dropped, which made it\n>> impossible to use them with git-credential-netrc. This is a supported\n>> use case according to 'man git-send-email', for --smtp-server-port:\n>> \n>>    [...] symbolic port names (e.g. \"submission\" instead of 587) are\n>>    also accepted.\n>\n> Does this work with credential managers in general (that is, in\n> non-email contexts, such as HTTP)?  Also, do credential managers in\n> general properly find credentials when they're stored in one form and\n> looked up in another?  If so, is that still true when the lookup is in\n> the URL form (e.g., `smtp://mail.example.com:587/`)?  Is this documented\n> to work in the credential manual page?\n\nI'm quite new to the credential-manager of git, so I do not have an\nanswer to these excellent questions.  But, as some perhaps useful\ndatapoint, at least using Emacs's auth-source library with a\n~/.authinfo.gpg file (which is in the netrc format), if you use a\nsymbolic port name, you have to use it everywhere if you want\nauth-source to match it correctly (it doesn't translate smtps to 465 for\nexample). If you put 'port smtps' in the .authinfo.gpg but specify the\nSMTP port in the your Emacs MTA to a integer like 465, it won't match.\n\nThis could be considered a bug in auth-source.el, and git\ncredential-manager can do better by converting all port input values to\ntheir integer form, as you suggested.  Then mismatched configurations\n(e.g.: smtps in netrc and sendemail.smtpServerPort = 465 or vice-versa)\nwould be handled correctly.\n\n> (To be clear, I would be very\n> surprised if the answer to any of these were \"yes\" because I've\n> literally never seen this usage before with Git, but I am open to\n> updating my knowledge if that's the case.)\n>\n> If not, then I think the proper thing to do is to have `git send-email`\n> rewrite the name into a port instead of having the netrc credential\n> helper learn to handle non-numeric ports.\n\nDid I understand correctly with my suggestion/rewording of yours above?\ngit-credential-netrc reads its input from the netrc file, which may well\nhave a symbolic port, so it should itself convert from symbolic to\nactual port numbers, IIUC.\n\n-- \nThanks,\nMaxim\n"},{"id":"520534","messageId":"875xgpgptt.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"87pleyijbg.fsf@igel.home","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-21T13:57:18Z","receivedAt":"2025-06-21T13:57:49Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi,\n\nAndreas Schwab <schwab@linux-m68k.org> writes:\n\n> On Jun 20 2025, Junio C Hamano wrote:\n>\n>> Do we know symbolic port names are always limited to alnums?  Or on\n>> some systems some byte values in the fringe, like \"_\" or \"-\", are\n>> also allowed?\n>\n> Valid service names are documented in RFC6335.  Specifically it allows\n> hyphens, but not underscores.\n\nThanks for the reference. I'm thinking at the moment to improve the\ncheck for a valid port using something like this Scheme code:\n\n--8<---------------cut here---------------start------------->8---\n(define (port? port)\n \"Return the numeric value for PORT, else #f.\"\n (if (and (exact-integer? port)\n          (positive? port)\n          (<= port (1- (expt 2 16))))\n     port\n     (catch 'system-error\n      (lambda () (servent:port (getservbyname port \"\")))\n      (const #f))))\nscheme@(guile-user)> (port? \"465\")\n$14 = #f\nscheme@(guile-user)> (port? 465)\n$15 = 465\nscheme@(guile-user)> (port? \"smtps\")\n$16 = 465\nscheme@(guile-user)> (port? \"unknown\")\n$17 = #f\nscheme@(guile-user)> (port? 120000)\n$18 = #f\n--8<---------------cut here---------------end--------------->8---\n\nSo basically, try to call getservname(2) on a non-numeric port. If this\nfails, the port is invalid, else return the port number.\n\n-- \nThanks,\nMaxim\n"},{"id":"520537","messageId":"aFbVKEbhunWyljkR@fruit.crustytoothpaste.net","threadId":"63677","inReplyTo":"87bjqhgr47.fsf@terra.mail-host-address-is-not-set","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-06-21T15:52:08Z","receivedAt":"2025-06-21T15:52:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-06-21 at 13:29:28, Maxim Cournoyer wrote:\n> I'm quite new to the credential-manager of git, so I do not have an\n> answer to these excellent questions.  But, as some perhaps useful\n> datapoint, at least using Emacs's auth-source library with a\n> ~/.authinfo.gpg file (which is in the netrc format), if you use a\n> symbolic port name, you have to use it everywhere if you want\n> auth-source to match it correctly (it doesn't translate smtps to 465 for\n> example). If you put 'port smtps' in the .authinfo.gpg but specify the\n> SMTP port in the your Emacs MTA to a integer like 465, it won't match.\n> \n> This could be considered a bug in auth-source.el, and git\n> credential-manager can do better by converting all port input values to\n> their integer form, as you suggested.  Then mismatched configurations\n> (e.g.: smtps in netrc and sendemail.smtpServerPort = 465 or vice-versa)\n> would be handled correctly.\n\nYes, I would say that we should be using numeric ports everywhere in the\ncredential protocol.  If `git send-email` receives \"submission\" as the\nport, then it needs to convert that to \"587\" before it even requests a\ncredential.\n\nThe git-credential(1) documentation says this for the `host` entry in\nthe protocol:\n\n    The remote hostname for a network credential. This includes the port\n    number if one was specified (e.g., \"example.com:8088\").\n\nNote that that says \"port number\", not \"symbolic port\".\n\nSo I think we'd need some answers as to what's going over the protocol\nfirst and how it works for built-in Git functionality (e.g., HTTPS)\nbefore we decide if this is a change we want to make.\n\n> Did I understand correctly with my suggestion/rewording of yours above?\n> git-credential-netrc reads its input from the netrc file, which may well\n> have a symbolic port, so it should itself convert from symbolic to\n> actual port numbers, IIUC.\n\nThe netrc format is actually underspecified and libcurl doesn't support\nports at all, so I would not say that using a symbolic port is a good\nidea or reliably supported in general.  In fact, I would say that the\nnetrc credential helper is the only tool I can find that accepts ports\nat all.  I've looked at multiple different tools and manual pages online\nand the `port` or `protocol` key is not even mentioned.\n\nIf we do accept symbolic ports in the netrc file, then we need to\nconvert them to a numeric port before sending anything over the\nprotocol, which I don't believe your patch does.  Perl does offer\n`getservbyname` for this purpose, so it shouldn't be too difficult to\nmake this change.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"520543","messageId":"20250622152535.11837-2-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v2 1/3] contrib: use a more portable shebang for git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-22T15:25:33Z","receivedAt":"2025-06-22T15:25:56Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"While the installed scripts have their Perl shebang set to PERL_PATH,\nit is nevertheless useful to be able to run the uninstalled script for\nmanual tests while developing. This change makes the shebang more\nportable by having the perl command looked from PATH instead of from a\nfixed location.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 9fb998ae09..514f68d00b 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -1,4 +1,4 @@\n-#!/usr/bin/perl\n+#!/usr/bin/env perl\n \n use strict;\n use warnings;\n\nbase-commit: cb3b40381e1d5ee32dde96521ad7cfd68eb308a6\n-- \n2.49.0\n\n"},{"id":"520544","messageId":"20250622152535.11837-1-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v2 0/3] git-credential-netrc: better symbolic port names support","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-22T15:25:32Z","receivedAt":"2025-06-22T15:26:01Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Change tested with 'make test' as well as\n'make -C contrib/credential/netrc testverbose', plus manual tests (such as\nsending this series!)\n\nMaxim Cournoyer (3):\n  contrib: use a more portable shebang for git-credential-netrc\n  contrib: warn for invalid netrc file ports in git-credential-netrc\n  contrib: better support symbolic port names in git-credential-netrc\n\n .../credential/netrc/git-credential-netrc.perl    | 14 +++++++++++---\n contrib/credential/netrc/test.pl                  |  8 ++++----\n git-send-email.perl                               | 11 +++++++++++\n perl/Git.pm                                       | 15 +++++++++++++++\n t/t9001-send-email.sh                             |  7 +++++++\n 5 files changed, 48 insertions(+), 7 deletions(-)\n\nbase-commit: cb3b40381e1d5ee32dde96521ad7cfd68eb308a6\n-- \n2.49.0\n\n"},{"id":"520545","messageId":"20250622152535.11837-4-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v2 3/3] contrib: better support symbolic port names in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-22T15:25:35Z","receivedAt":"2025-06-22T15:26:01Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"To improve support for symbolic port names in netrc files, this\nchanges does the following:\n\n - Treat symbolic port names as ports, not protocols in git-credential-netrc\n - Validate the SMTP server port provided to send-email\n - Convert the above symbolic port names to their numerical values.\n\nBefore this change, it was not possible to have a SMTP server port set\nto \"smtps\" in a netrc file (e.g. Emacs' ~/.authinfo.gpg), as it would\nbe registered as a protocol and break the match for a \"smtp\" protocol\nhost, as queried for by git-send-email.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n .../credential/netrc/git-credential-netrc.perl    | 11 +++++++----\n contrib/credential/netrc/test.pl                  |  8 ++++----\n git-send-email.perl                               | 11 +++++++++++\n perl/Git.pm                                       | 15 +++++++++++++++\n t/t9001-send-email.sh                             |  7 +++++++\n 5 files changed, 44 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 09d77b4f69..72d6b6a974 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -268,13 +268,16 @@ sub load_netrc {\n \t\t\tnext;\n \t\t}\n \t\tif (defined $nentry->{port}) {\n-\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n-\t\t\t\t$num_port = $nentry->{port};\n-\t\t\t\tdelete $nentry->{port};\n-\t\t\t} else {\n+\t\t\t$num_port = Git::is_port($nentry->{port});\n+\t\t\tunless ($num_port) {\n \t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n \t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n \t\t\t}\n+\t\t\t# Since we've already validated and converted\n+\t\t\t# the port to its numerical value, do not\n+\t\t\t# capture it as the `protocol' value, as used\n+\t\t\t# to be the case for symbolic port names.\n+\t\t\tdelete $nentry->{port};\n \t\t}\n \n \t\t# create the new entry for the credential helper protocol\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 67a0ede564..8a7fc2588a 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -45,7 +45,7 @@ BEGIN\n diag \"Testing with invalid data\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n \t\t       \"bad data\");\n-ok(scalar keys %$cred == 4, \"Got first found keys with bad data\");\n+ok(scalar keys %$cred == 3, \"Got first found keys with bad data\");\n \n diag \"Testing netrc file for a missing corovamilkbar entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n@@ -64,12 +64,12 @@ BEGIN\n \n diag \"Testing netrc file for a username-specific entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n-\t\t       { host => 'imap', username => 'bob' });\n+\t\t       { host => 'imap:993', username => 'bob' });\n \n-ok(scalar keys %$cred == 2, \"Got 2 username-specific keys\");\n+# Only the password field gets returned.\n+ok(scalar keys %$cred == 1, \"Got 1 username-specific keys\");\n \n is($cred->{password}, 'bobwillknow', \"Got correct user-specific password\");\n-is($cred->{protocol}, 'imaps', \"Got correct user-specific protocol\");\n \n diag \"Testing netrc file for a host:port-specific entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 659e6c588b..502c7d9e04 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -2101,6 +2101,17 @@ sub initialize_modified_loop_vars {\n \t\t}\n \t}\n \n+\t# Validate the SMTP server port, if provided.\n+\tif (defined $smtp_server_port) {\n+\t\tmy $port = Git::is_port($smtp_server_port);\n+\t\tif ($port) {\n+\t\t\t$smtp_server_port = $port;\n+\t\t} else  {\n+\t\t\tdie sprintf(__(\"error: invalid SMTP port '%s'\\n\"),\n+\t\t\t\t    $smtp_server_port);\n+\t\t}\n+\t}\n+\n \t# Run the loop once again to avoid gaps in the counter due to FIFO\n \t# arguments provided by the user.\n \tmy $num = 1;\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 6f47d653ab..1fa535e1ad 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -1061,6 +1061,21 @@ sub _close_cat_blob {\n \tdelete @$self{@vars};\n }\n \n+# Predicate to check whether PORT is a valid port number or service\n+# name. The numerical value of PORT is returned, else undef if\n+# invalid.\n+sub is_port {\n+    my ($port) = @_;\n+\n+    # Port can be either a positive integer within the 16-bit range...\n+    if ($port =~ /^\\d+$/ && $port > 0 && $port <= (2**16 - 1)) {\n+        return $port;\n+    }\n+\n+    # ... or a symbolic port (service name).\n+    my $num = getservbyname($port, '');\n+    return defined $num ? $num : undef;\n+}\n \n =item credential_read( FILEHANDLE )\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0c1af43f6f..3e749175ab 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -201,6 +201,13 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' '\n \ttest_cmp expected-cc commandline1\n '\n \n+test_expect_failure $PREREQ 'invalid smtp server port value' '\n+\tclean_fake_sendmail &&\n+\tgit send-email -1 --to=recipient@example.com \\\n+                --smtp-server-port=bogus-symbolic-name \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\"\n+'\n+\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n-- \n2.49.0\n\n"},{"id":"520546","messageId":"20250622152535.11837-3-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v2 2/3] contrib: warn for invalid netrc file ports in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-22T15:25:34Z","receivedAt":"2025-06-22T15:26:03Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Invalid ports were previously silently dropped; now a warning message\nis produced.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 514f68d00b..09d77b4f69 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -267,9 +267,14 @@ sub load_netrc {\n \t\tif (!defined $nentry->{machine}) {\n \t\t\tnext;\n \t\t}\n-\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n-\t\t\t$num_port = $nentry->{port};\n-\t\t\tdelete $nentry->{port};\n+\t\tif (defined $nentry->{port}) {\n+\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n+\t\t\t\t$num_port = $nentry->{port};\n+\t\t\t\tdelete $nentry->{port};\n+\t\t\t} else {\n+\t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n+\t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n+\t\t\t}\n \t\t}\n \n \t\t# create the new entry for the credential helper protocol\n-- \n2.49.0\n\n"},{"id":"520549","messageId":"874iw7f86n.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"xmqqmsa27cdn.fsf@gitster.g","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-23T03:28:16Z","receivedAt":"2025-06-23T03:28:33Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi Junio,\n\ntldr; all changes discussed implemented in posted v2.\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Maxim Cournoyer <maxim@guixotic.coop> writes:\n>\n>> Subject: Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.\n>\n> Please downcase \"Honor\" and drop the final full stop, per convention\n> (see \"git shortlog --no-merges --since=2.months\" for examples).\n\nDone.\n\n>> Symbolic ports were previously silently dropped, which made it\n>> impossible to use them with git-credential-netrc.\n>\n> Wouldn't it make sense to issue a warning message when a defined\n> $nentry->{port} is not unrecognized?  Wouldn't it make sense to\n> do so even before we add this new feature?\n\nI agree it's subpar that the current code silently drops the port when\nit doesn't match the expected form. Since port values aren't validated\nin 'git-send-email' at the moment, a proper fix would be to have routine\nto validate ports in a common library and applied everywhere a port is\nread from the user or a config file, ideally, in git-send-email or\nelsewhere.  Maybe it could live in Git.pm ?\n\nEdit: Done.\n\n>> This is a supported\n>> use case according to 'man git-send-email', for --smtp-server-port:\n>>\n>>    [...] symbolic port names (e.g. \"submission\" instead of 587) are\n>>    also accepted.\n>> ---\n>\n> Missing sign-off?  See Documentation/SubmittingPatches\n\nDone.\n\n>>  contrib/credential/netrc/git-credential-netrc.perl | 6 ++++--\n>>  1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/contrib/credential/netrc/git-credential-netrc.perl\n>> b/contrib/credential/netrc/git-credential-netrc.perl\n>> index 9fb998ae09..ad06000b9f 100755\n>> --- a/contrib/credential/netrc/git-credential-netrc.perl\n>> +++ b/contrib/credential/netrc/git-credential-netrc.perl\n>> @@ -1,4 +1,4 @@\n>> -#!/usr/bin/perl\n>> +#!/usr/bin/env perl\n>\n> An unrelated change to introduce the use of /usr/bin/env in this\n> patch is unwelcome.  Besides, this is a source that is processed\n> by the nearby Makefile, which uses the toplevel genererate-perl.sh\n> to turn the \"#!.../perl\" line to name the correct $PERL_PATH before\n> the build product gets installed, so I suspect that this change is\n> totally unnecessary.\n\nIt was necessary on my system to test the uninstalled version, which I\nsimply symlinked to ~/.local/bin/git-credential-netrc for ease of\ntesting. I've split this small change in its own commit. Using env\nin shebangs instead of hard-coded locations is good for portability in\ngeneral, and the generate-perl.sh substitution will work still.\n\n>> @@ -267,7 +267,9 @@ sub load_netrc {\n>>  \t\tif (!defined $nentry->{machine}) {\n>>  \t\t\tnext;\n>>  \t\t}\n>> -\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n>> +\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^[[:alnum:]]+$/) {\n>> +\t\t\t# Port may be either an integer or a symbolic\n>> +\t\t\t# name, e.g. \"smtps\".\n>\n> Do we know symbolic port names are always limited to alnums?  Or on\n> some systems some byte values in the fringe, like \"_\" or \"-\", are\n> also allowed?\n\nLooking at /etc/services on my system, I see hyphens, indeed, e.g.:\n're-mail-ck'.  That's now handled by the `is_port' predicate, which uses\nthe libc `getservbyname' call to determine if a non-numeric port is a\nvalid service/symbolic port name.\n\nI've sent a v2 revision which hopefully includes all of the above\nsuggestion/changes.\n\n-- \nMaxim\n"},{"id":"520550","messageId":"871prbf7wm.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"xmqqmsa27cdn.fsf@gitster.g","subject":"Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-23T03:34:17Z","receivedAt":"2025-06-23T03:34:35Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi Junio,\n\ntldr; all changes discussed implemented in posted v2.\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Maxim Cournoyer <maxim@guixotic.coop> writes:\n>\n>> Subject: Re: [PATCH] contrib: Honor symbolic port in git-credential-netrc.\n>\n> Please downcase \"Honor\" and drop the final full stop, per convention\n> (see \"git shortlog --no-merges --since=2.months\" for examples).\n\nDone.\n\n>> Symbolic ports were previously silently dropped, which made it\n>> impossible to use them with git-credential-netrc.\n>\n> Wouldn't it make sense to issue a warning message when a defined\n> $nentry->{port} is not unrecognized?  Wouldn't it make sense to\n> do so even before we add this new feature?\n\nI agree it's subpar that the current code silently drops the port when\nit doesn't match the expected form. Since port values aren't validated\nin 'git-send-email' at the moment, a proper fix would be to have routine\nto validate ports in a common library and applied everywhere a port is\nread from the user or a config file, ideally, in git-send-email or\nelsewhere.  Maybe it could live in Git.pm ?\n\nEdit: Done.\n\n>> This is a supported\n>> use case according to 'man git-send-email', for --smtp-server-port:\n>>\n>>    [...] symbolic port names (e.g. \"submission\" instead of 587) are\n>>    also accepted.\n>> ---\n>\n> Missing sign-off?  See Documentation/SubmittingPatches\n\nDone.\n\n>>  contrib/credential/netrc/git-credential-netrc.perl | 6 ++++--\n>>  1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/contrib/credential/netrc/git-credential-netrc.perl\n>> b/contrib/credential/netrc/git-credential-netrc.perl\n>> index 9fb998ae09..ad06000b9f 100755\n>> --- a/contrib/credential/netrc/git-credential-netrc.perl\n>> +++ b/contrib/credential/netrc/git-credential-netrc.perl\n>> @@ -1,4 +1,4 @@\n>> -#!/usr/bin/perl\n>> +#!/usr/bin/env perl\n>\n> An unrelated change to introduce the use of /usr/bin/env in this\n> patch is unwelcome.  Besides, this is a source that is processed\n> by the nearby Makefile, which uses the toplevel genererate-perl.sh\n> to turn the \"#!.../perl\" line to name the correct $PERL_PATH before\n> the build product gets installed, so I suspect that this change is\n> totally unnecessary.\n\nIt was necessary on my system to test the uninstalled version, which I\nsimply symlinked to ~/.local/bin/git-credential-netrc for ease of\ntesting. I've split this small change in its own commit. Using env\nin shebangs instead of hard-coded locations is good for portability in\ngeneral, and the generate-perl.sh substitution will work still.\n\n>> @@ -267,7 +267,9 @@ sub load_netrc {\n>>  \t\tif (!defined $nentry->{machine}) {\n>>  \t\t\tnext;\n>>  \t\t}\n>> -\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n>> +\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^[[:alnum:]]+$/) {\n>> +\t\t\t# Port may be either an integer or a symbolic\n>> +\t\t\t# name, e.g. \"smtps\".\n>\n> Do we know symbolic port names are always limited to alnums?  Or on\n> some systems some byte values in the fringe, like \"_\" or \"-\", are\n> also allowed?\n\nLooking at /etc/services on my system, I see hyphens, indeed, e.g.:\n're-mail-ck'.  That's now handled by the `is_port' predicate, which uses\nthe getservbyname(3) call to determine if a non-numeric port is a\nvalid service/symbolic port name.\n\nI've sent a v2 revision which hopefully addresses all of the above\nsuggestions/changes.\n\n-- \nMaxim\n"},{"id":"520579","messageId":"xmqqh6065o9f.fsf@gitster.g","threadId":"63677","inReplyTo":"20250622152535.11837-4-maxim@guixotic.coop","subject":"Re: [PATCH v2 3/3] contrib: better support symbolic port names in git-credential-netrc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-23T18:03:24Z","receivedAt":"2025-06-23T18:03:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maxim Cournoyer <maxim@guixotic.coop> writes:\n\n> diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\n> index 09d77b4f69..72d6b6a974 100755\n> --- a/contrib/credential/netrc/git-credential-netrc.perl\n> +++ b/contrib/credential/netrc/git-credential-netrc.perl\n> @@ -268,13 +268,16 @@ sub load_netrc {\n>  \t\t\tnext;\n>  \t\t}\n>  \t\tif (defined $nentry->{port}) {\n> -\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n> -\t\t\t\t$num_port = $nentry->{port};\n> -\t\t\t\tdelete $nentry->{port};\n> -\t\t\t} else {\n> +\t\t\t$num_port = Git::is_port($nentry->{port});\n> +\t\t\tunless ($num_port) {\n>  \t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n>  \t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n>  \t\t\t}\n> +\t\t\t# Since we've already validated and converted\n> +\t\t\t# the port to its numerical value, do not\n> +\t\t\t# capture it as the `protocol' value, as used\n> +\t\t\t# to be the case for symbolic port names.\n> +\t\t\tdelete $nentry->{port};\n>  \t\t}\n\nOK, so we rewrite textual service names into port number, and\nnormalize the \"host\" member of the entry read from the file into\n\"host:port\" form.  Earlier we did that only for numeric port\nnumbers.  Nice.\n\n> diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\n> index 67a0ede564..8a7fc2588a 100755\n> --- a/contrib/credential/netrc/test.pl\n> +++ b/contrib/credential/netrc/test.pl\n> @@ -45,7 +45,7 @@ BEGIN\n>  diag \"Testing with invalid data\\n\";\n>  $cred = run_credential(['-f', $netrc, 'get'],\n>  \t\t       \"bad data\");\n> -ok(scalar keys %$cred == 4, \"Got first found keys with bad data\");\n> +ok(scalar keys %$cred == 3, \"Got first found keys with bad data\");\n>  \n>  diag \"Testing netrc file for a missing corovamilkbar entry\\n\";\n>  $cred = run_credential(['-f', $netrc, 'get'],\n> @@ -64,12 +64,12 @@ BEGIN\n>  \n>  diag \"Testing netrc file for a username-specific entry\\n\";\n>  $cred = run_credential(['-f', $netrc, 'get'],\n> -\t\t       { host => 'imap', username => 'bob' });\n> +\t\t       { host => 'imap:993', username => 'bob' });\n\nIs this rewriting an existing test, instead of adding a new test to\ntrigger a feature that didn't have a test coverage, while keeping\nthe old test?  I am wondering if we want to ensure that both\n\":port\"-less case and \"host:port\" case keep working even after the\nchange to -netrc credential helper in this patch.\n\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index 6f47d653ab..1fa535e1ad 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -1061,6 +1061,21 @@ sub _close_cat_blob {\n>  \tdelete @$self{@vars};\n>  }\n>  \n> +# Predicate to check whether PORT is a valid port number or service\n> +# name. The numerical value of PORT is returned, else undef if\n> +# invalid.\n\nHmph.  It _can_ be used to validate a random end-user supplied\nstring names a port, either by being a port number in the valid\nrange or by being a valid service name.  But another use case in the\ncode after this patch applied that is equally if not more important\nis to ensure that a valid port specified by the end-user is turned\ninto a port number.  We should not name such a sub as if its primary\nfunctionality is to serve as a Boolean \"is_foo\".  Perhaps call it\nport_num or something?\n\n> +sub is_port {\n> +    my ($port) = @_;\n> +\n> +    # Port can be either a positive integer within the 16-bit range...\n> +    if ($port =~ /^\\d+$/ && $port > 0 && $port <= (2**16 - 1)) {\n> +        return $port;\n> +    }\n> +\n> +    # ... or a symbolic port (service name).\n> +    my $num = getservbyname($port, '');\n> +    return defined $num ? $num : undef;\n\nWouldn't \"return $num\" work here?  getservbyname() would return\n\"undef\" when the given $port is not a valid service name anyway, no?\n\nOr even \"return scalar getservbyname($port, 'tcp')\" without an\nintermediate variable $num?\n\n> +}\n>  \n>  =item credential_read( FILEHANDLE )\n>  \n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 0c1af43f6f..3e749175ab 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -201,6 +201,13 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' '\n>  \ttest_cmp expected-cc commandline1\n>  '\n>  \n> +test_expect_failure $PREREQ 'invalid smtp server port value' '\n> +\tclean_fake_sendmail &&\n> +\tgit send-email -1 --to=recipient@example.com \\\n> +                --smtp-server-port=bogus-symbolic-name \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\"\n> +'\n> +\n>  test_expect_success $PREREQ 'setup expect' \"\n>  cat >expected-show-all-headers <<\\EOF\n>  0001-Second.patch\n"},{"id":"520622","messageId":"20250624014857.3748-2-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v3 1/3] contrib: use a more portable shebang for git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-24T01:48:55Z","receivedAt":"2025-06-24T01:49:39Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"While the installed scripts have their Perl shebang set to PERL_PATH,\nit is nevertheless useful to be able to run the uninstalled script for\nmanual tests while developing. This change makes the shebang more\nportable by having the perl command looked from PATH instead of from a\nfixed location.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 9fb998ae09..514f68d00b 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -1,4 +1,4 @@\n-#!/usr/bin/perl\n+#!/usr/bin/env perl\n \n use strict;\n use warnings;\n-- \n2.49.0\n\n"},{"id":"520621","messageId":"20250624014857.3748-1-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v3 0/3] git-credential-netrc: better symbolic port names support","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-24T01:48:54Z","receivedAt":"2025-06-24T01:49:40Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Most suggestions from Junio have been applied in this revision.\n\nChanges in v3:\n - rename is_port to port_num\n - directly return scalar value from getservbyname in port_num\n\nThanks,\n\nMaxim Cournoyer (3):\n  contrib: use a more portable shebang for git-credential-netrc\n  contrib: warn for invalid netrc file ports in git-credential-netrc\n  contrib: better support symbolic port names in git-credential-netrc\n\n contrib/credential/netrc/git-credential-netrc.perl | 14 +++++++++++---\n contrib/credential/netrc/test.pl                   |  8 ++++----\n git-send-email.perl                                | 11 +++++++++++\n perl/Git.pm                                        | 13 +++++++++++++\n t/t9001-send-email.sh                              |  7 +++++++\n 5 files changed, 46 insertions(+), 7 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"520623","messageId":"20250624014857.3748-4-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v3 3/3] contrib: better support symbolic port names in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-24T01:48:57Z","receivedAt":"2025-06-24T01:49:41Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"To improve support for symbolic port names in netrc files, this\nchanges does the following:\n\n - Treat symbolic port names as ports, not protocols in git-credential-netrc\n - Validate the SMTP server port provided to send-email\n - Convert the above symbolic port names to their numerical values.\n\nBefore this change, it was not possible to have a SMTP server port set\nto \"smtps\" in a netrc file (e.g. Emacs' ~/.authinfo.gpg), as it would\nbe registered as a protocol and break the match for a \"smtp\" protocol\nhost, as queried for by git-send-email.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 11 +++++++----\n contrib/credential/netrc/test.pl                   |  8 ++++----\n git-send-email.perl                                | 11 +++++++++++\n perl/Git.pm                                        | 13 +++++++++++++\n t/t9001-send-email.sh                              |  7 +++++++\n 5 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 09d77b4f69..3c0a532d0e 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -268,13 +268,16 @@ sub load_netrc {\n \t\t\tnext;\n \t\t}\n \t\tif (defined $nentry->{port}) {\n-\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n-\t\t\t\t$num_port = $nentry->{port};\n-\t\t\t\tdelete $nentry->{port};\n-\t\t\t} else {\n+\t\t\t$num_port = Git::port_num($nentry->{port});\n+\t\t\tunless ($num_port) {\n \t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n \t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n \t\t\t}\n+\t\t\t# Since we've already validated and converted\n+\t\t\t# the port to its numerical value, do not\n+\t\t\t# capture it as the `protocol' value, as used\n+\t\t\t# to be the case for symbolic port names.\n+\t\t\tdelete $nentry->{port};\n \t\t}\n \n \t\t# create the new entry for the credential helper protocol\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 67a0ede564..8a7fc2588a 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -45,7 +45,7 @@ BEGIN\n diag \"Testing with invalid data\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n \t\t       \"bad data\");\n-ok(scalar keys %$cred == 4, \"Got first found keys with bad data\");\n+ok(scalar keys %$cred == 3, \"Got first found keys with bad data\");\n \n diag \"Testing netrc file for a missing corovamilkbar entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n@@ -64,12 +64,12 @@ BEGIN\n \n diag \"Testing netrc file for a username-specific entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n-\t\t       { host => 'imap', username => 'bob' });\n+\t\t       { host => 'imap:993', username => 'bob' });\n \n-ok(scalar keys %$cred == 2, \"Got 2 username-specific keys\");\n+# Only the password field gets returned.\n+ok(scalar keys %$cred == 1, \"Got 1 username-specific keys\");\n \n is($cred->{password}, 'bobwillknow', \"Got correct user-specific password\");\n-is($cred->{protocol}, 'imaps', \"Got correct user-specific protocol\");\n \n diag \"Testing netrc file for a host:port-specific entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 659e6c588b..d2cf9b717a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -2101,6 +2101,17 @@ sub initialize_modified_loop_vars {\n \t\t}\n \t}\n \n+\t# Validate the SMTP server port, if provided.\n+\tif (defined $smtp_server_port) {\n+\t\tmy $port = Git::port_num($smtp_server_port);\n+\t\tif ($port) {\n+\t\t\t$smtp_server_port = $port;\n+\t\t} else  {\n+\t\t\tdie sprintf(__(\"error: invalid SMTP port '%s'\\n\"),\n+\t\t\t\t    $smtp_server_port);\n+\t\t}\n+\t}\n+\n \t# Run the loop once again to avoid gaps in the counter due to FIFO\n \t# arguments provided by the user.\n \tmy $num = 1;\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 6f47d653ab..090cf77dab 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -1061,6 +1061,19 @@ sub _close_cat_blob {\n \tdelete @$self{@vars};\n }\n \n+# Given PORT, a port number or service name, return its numerical\n+# value else undef.\n+sub port_num {\n+    my ($port) = @_;\n+\n+    # Port can be either a positive integer within the 16-bit range...\n+    if ($port =~ /^\\d+$/ && $port > 0 && $port <= (2**16 - 1)) {\n+        return $port;\n+    }\n+\n+    # ... or a symbolic port (service name).\n+    return scalar getservbyname($port, '');\n+}\n \n =item credential_read( FILEHANDLE )\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0c1af43f6f..3e749175ab 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -201,6 +201,13 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' '\n \ttest_cmp expected-cc commandline1\n '\n \n+test_expect_failure $PREREQ 'invalid smtp server port value' '\n+\tclean_fake_sendmail &&\n+\tgit send-email -1 --to=recipient@example.com \\\n+                --smtp-server-port=bogus-symbolic-name \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\"\n+'\n+\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n-- \n2.49.0\n\n"},{"id":"520624","messageId":"20250624014857.3748-3-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250620041239.27839-1-maxim@guixotic.coop","subject":"[PATCH v3 2/3] contrib: warn for invalid netrc file ports in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-24T01:48:56Z","receivedAt":"2025-06-24T01:49:43Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Invalid ports were previously silently dropped; now a warning message\nis produced.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 514f68d00b..09d77b4f69 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -267,9 +267,14 @@ sub load_netrc {\n \t\tif (!defined $nentry->{machine}) {\n \t\t\tnext;\n \t\t}\n-\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n-\t\t\t$num_port = $nentry->{port};\n-\t\t\tdelete $nentry->{port};\n+\t\tif (defined $nentry->{port}) {\n+\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n+\t\t\t\t$num_port = $nentry->{port};\n+\t\t\t\tdelete $nentry->{port};\n+\t\t\t} else {\n+\t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n+\t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n+\t\t\t}\n \t\t}\n \n \t\t# create the new entry for the credential helper protocol\n-- \n2.49.0\n\n"},{"id":"520625","messageId":"87o6ud52l8.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"xmqqh6065o9f.fsf@gitster.g","subject":"Re: [PATCH v2 3/3] contrib: better support symbolic port names in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-24T01:51:31Z","receivedAt":"2025-06-24T01:51:40Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi!\n\ntl;dr: I've submitted a v3 with most of your suggestions implemented.\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n[...]\n\n>> diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\n>> index 67a0ede564..8a7fc2588a 100755\n>> --- a/contrib/credential/netrc/test.pl\n>> +++ b/contrib/credential/netrc/test.pl\n>> @@ -45,7 +45,7 @@ BEGIN\n>>  diag \"Testing with invalid data\\n\";\n>>  $cred = run_credential(['-f', $netrc, 'get'],\n>>  \t\t       \"bad data\");\n>> -ok(scalar keys %$cred == 4, \"Got first found keys with bad data\");\n>> +ok(scalar keys %$cred == 3, \"Got first found keys with bad data\");\n>>  \n>>  diag \"Testing netrc file for a missing corovamilkbar entry\\n\";\n>>  $cred = run_credential(['-f', $netrc, 'get'],\n>> @@ -64,12 +64,12 @@ BEGIN\n>>  \n>>  diag \"Testing netrc file for a username-specific entry\\n\";\n>>  $cred = run_credential(['-f', $netrc, 'get'],\n>> -\t\t       { host => 'imap', username => 'bob' });\n>> +\t\t       { host => 'imap:993', username => 'bob' });\n>\n> Is this rewriting an existing test, instead of adding a new test to\n> trigger a feature that didn't have a test coverage, while keeping\n> the old test?  I am wondering if we want to ensure that both\n> \":port\"-less case and \"host:port\" case keep working even after the\n> change to -netrc credential helper in this patch.\n\nThat specific test *is* using a port, but a symbolic one (imaps), which\nused to be captured as the 'protocol' in the Git credential hash/array.\nNow it's captured properly as a port, which is represented in Git\ncredential by joining it with the host name. The test needed adjusting\nfor that.\n\n[...]\n\n> Hmph.  It _can_ be used to validate a random end-user supplied\n> string names a port, either by being a port number in the valid\n> range or by being a valid service name.  But another use case in the\n> code after this patch applied that is equally if not more important\n> is to ensure that a valid port specified by the end-user is turned\n> into a port number.  We should not name such a sub as if its primary\n> functionality is to serve as a Boolean \"is_foo\".  Perhaps call it\n> port_num or something?\n\nNaming is hard :-). I like your suggestion. Done.\n\n>> +sub is_port {\n>> +    my ($port) = @_;\n>> +\n>> +    # Port can be either a positive integer within the 16-bit range...\n>> +    if ($port =~ /^\\d+$/ && $port > 0 && $port <= (2**16 - 1)) {\n>> +        return $port;\n>> +    }\n>> +\n>> +    # ... or a symbolic port (service name).\n>> +    my $num = getservbyname($port, '');\n>> +    return defined $num ? $num : undef;\n>\n> Wouldn't \"return $num\" work here?  getservbyname() would return\n> \"undef\" when the given $port is not a valid service name anyway, no?\n>\n> Or even \"return scalar getservbyname($port, 'tcp')\" without an\n> intermediate variable $num?\n\nI've re-read the doc (perldoc -f getservbyname) and you are right, in a\nscalar context it would return an undef value when the service name was\nnot found in the local database. Done!\n\n-- \nThanks,\nMaxim\n"},{"id":"520652","messageId":"xmqqecv915y7.fsf@gitster.g","threadId":"63677","inReplyTo":"20250624014857.3748-1-maxim@guixotic.coop","subject":"Re: [PATCH v3 0/3] git-credential-netrc: better symbolic port names support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-24T16:04:48Z","receivedAt":"2025-06-24T16:04:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maxim Cournoyer <maxim@guixotic.coop> writes:\n\n> Most suggestions from Junio have been applied in this revision.\n>\n> Changes in v3:\n>  - rename is_port to port_num\n>  - directly return scalar value from getservbyname in port_num\n>\n> Thanks,\n>\n> Maxim Cournoyer (3):\n>   contrib: use a more portable shebang for git-credential-netrc\n>   contrib: warn for invalid netrc file ports in git-credential-netrc\n>   contrib: better support symbolic port names in git-credential-netrc\n>\n>  contrib/credential/netrc/git-credential-netrc.perl | 14 +++++++++++---\n>  contrib/credential/netrc/test.pl                   |  8 ++++----\n>  git-send-email.perl                                | 11 +++++++++++\n>  perl/Git.pm                                        | 13 +++++++++++++\n>  t/t9001-send-email.sh                              |  7 +++++++\n>  5 files changed, 46 insertions(+), 7 deletions(-)\n\nv2 and this iteration both have all messages set as replies to a\nsingle message in the old thread.\n\nPlease make sure in your future submissions:\n\n - [0/n] is a reply to [0/m] of the previous iteration.\n\n - [1/n], [2/n], ... and [n/n] are all replies to [0/n] of the same\n   iteration.\n\n"},{"id":"520654","messageId":"xmqqa55x15vr.fsf@gitster.g","threadId":"63677","inReplyTo":"20250624014857.3748-4-maxim@guixotic.coop","subject":"Re: [PATCH v3 3/3] contrib: better support symbolic port names in git-credential-netrc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-24T16:06:16Z","receivedAt":"2025-06-24T16:06:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maxim Cournoyer <maxim@guixotic.coop> writes:\n\n> +\tgit send-email -1 --to=recipient@example.com \\\n> +                --smtp-server-port=bogus-symbolic-name \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\"\n> +'\n\nThere is a funny indent-with-spaces here.\n"},{"id":"520667","messageId":"87ikkkk84f.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"xmqqecv915y7.fsf@gitster.g","subject":"Re: [PATCH v3 0/3] git-credential-netrc: better symbolic port names support","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-24T23:55:12Z","receivedAt":"2025-06-24T23:55:22Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi,\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n[...]\n\n> v2 and this iteration both have all messages set as replies to a\n> single message in the old thread.\n>\n> Please make sure in your future submissions:\n>\n>  - [0/n] is a reply to [0/m] of the previous iteration.\n>\n>  - [1/n], [2/n], ... and [n/n] are all replies to [0/n] of the same\n>    iteration.\n\nOK. This means I need to submit with 'git send-email' in two steps,\nright?\n\n-- \nThanks,\nMaxim\n"},{"id":"520669","messageId":"xmqqikkkzmzr.fsf@gitster.g","threadId":"63677","inReplyTo":"87ikkkk84f.fsf@terra.mail-host-address-is-not-set","subject":"Re: [PATCH v3 0/3] git-credential-netrc: better symbolic port names support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-25T00:24:56Z","receivedAt":"2025-06-25T00:24:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maxim Cournoyer <maxim@guixotic.coop> writes:\n\n> Hi,\n>\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n> [...]\n>\n>> v2 and this iteration both have all messages set as replies to a\n>> single message in the old thread.\n>>\n>> Please make sure in your future submissions:\n>>\n>>  - [0/n] is a reply to [0/m] of the previous iteration.\n>>\n>>  - [1/n], [2/n], ... and [n/n] are all replies to [0/n] of the same\n>>    iteration.\n>\n> OK. This means I need to submit with 'git send-email' in two steps,\n> right?\n\nI do not think so.  Find description of the \"--in-reply-to\" option\nin the documentation, and read about interactions with \"--thread\"\nand \"--no-chain-reply-to\" there?\n\n    So for example when `--thread` and `--no-chain-reply-to` are specified, the\n    second and subsequent patches will be replies to the first one like in the\n    illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\n\n      [PATCH 0/2] Here is what I did...\n        [PATCH 1/2] Clean up and tests\n        [PATCH 2/2] Implementation\n        [PATCH v2 0/3] Here is a reroll\n          [PATCH v2 1/3] Clean up\n          [PATCH v2 2/3] New tests\n          [PATCH v2 3/3] Implementation\n\n"},{"id":"520670","messageId":"87ecv8k4y9.fsf@terra.mail-host-address-is-not-set","threadId":"63677","inReplyTo":"xmqqikkkzmzr.fsf@gitster.g","subject":"Re: [PATCH v3 0/3] git-credential-netrc: better symbolic port names support","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-25T01:03:42Z","receivedAt":"2025-06-25T01:04:02Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hello,\n\n[...]\n\n>>> v2 and this iteration both have all messages set as replies to a\n>>> single message in the old thread.\n>>>\n>>> Please make sure in your future submissions:\n>>>\n>>>  - [0/n] is a reply to [0/m] of the previous iteration.\n>>>\n>>>  - [1/n], [2/n], ... and [n/n] are all replies to [0/n] of the same\n>>>    iteration.\n>>\n>> OK. This means I need to submit with 'git send-email' in two steps,\n>> right?\n>\n> I do not think so.  Find description of the \"--in-reply-to\" option\n> in the documentation, and read about interactions with \"--thread\"\n> and \"--no-chain-reply-to\" there?\n>\n>     So for example when `--thread` and `--no-chain-reply-to` are specified, the\n>     second and subsequent patches will be replies to the first one like in the\n>     illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\n>\n>       [PATCH 0/2] Here is what I did...\n>         [PATCH 1/2] Clean up and tests\n>         [PATCH 2/2] Implementation\n>         [PATCH v2 0/3] Here is a reroll\n>           [PATCH v2 1/3] Clean up\n>           [PATCH v2 2/3] New tests\n>           [PATCH v2 3/3] Implementation\n\nOK, so as a self-note; this is the default behavior (--thread and\n--no-chain-reply-to) and the thing I got wrong was that --in-reply-to\nshould be set to the Message-ID of the previous revision's cover letter\n(in my recent submissions I had kept the message ID of the original cover\nletter instead). That's also explained in\ndocumentation/myfirstcontribution.adoc.\n\nI'll now send a v4 fixing the white space issue, making sure to\n--in-reply-to=$message-id-of-v3-cover-letter.\n\n-- \nThanks,\nMaxim\n"},{"id":"520699","messageId":"20250625142511.28857-1-maxim@guixotic.coop","threadId":"63677","inReplyTo":"87ecv8k4y9.fsf@terra.mail-host-address-is-not-set","subject":"[PATCH v4 0/3] git-credential-netrc: better symbolic port names support","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-25T14:25:08Z","receivedAt":"2025-06-25T14:25:55Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"This revision fixes a single white space in a new test added in 3/3.\n\nMaxim Cournoyer (3):\n  contrib: use a more portable shebang for git-credential-netrc\n  contrib: warn for invalid netrc file ports in git-credential-netrc\n  contrib: better support symbolic port names in git-credential-netrc\n\n contrib/credential/netrc/git-credential-netrc.perl | 14 +++++++++++---\n contrib/credential/netrc/test.pl                   |  8 ++++----\n git-send-email.perl                                | 11 +++++++++++\n perl/Git.pm                                        | 13 +++++++++++++\n t/t9001-send-email.sh                              |  7 +++++++\n 5 files changed, 46 insertions(+), 7 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"520700","messageId":"20250625142511.28857-3-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250625142511.28857-1-maxim@guixotic.coop","subject":"[PATCH v4 2/3] contrib: warn for invalid netrc file ports in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-25T14:25:10Z","receivedAt":"2025-06-25T14:25:58Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Invalid ports were previously silently dropped; now a warning message\nis produced.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 514f68d00b..09d77b4f69 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -267,9 +267,14 @@ sub load_netrc {\n \t\tif (!defined $nentry->{machine}) {\n \t\t\tnext;\n \t\t}\n-\t\tif (defined $nentry->{port} && $nentry->{port} =~ m/^\\d+$/) {\n-\t\t\t$num_port = $nentry->{port};\n-\t\t\tdelete $nentry->{port};\n+\t\tif (defined $nentry->{port}) {\n+\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n+\t\t\t\t$num_port = $nentry->{port};\n+\t\t\t\tdelete $nentry->{port};\n+\t\t\t} else {\n+\t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n+\t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n+\t\t\t}\n \t\t}\n \n \t\t# create the new entry for the credential helper protocol\n-- \n2.49.0\n\n"},{"id":"520701","messageId":"20250625142511.28857-4-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250625142511.28857-1-maxim@guixotic.coop","subject":"[PATCH v4 3/3] contrib: better support symbolic port names in git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-25T14:25:11Z","receivedAt":"2025-06-25T14:25:59Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"To improve support for symbolic port names in netrc files, this\nchanges does the following:\n\n - Treat symbolic port names as ports, not protocols in git-credential-netrc\n - Validate the SMTP server port provided to send-email\n - Convert the above symbolic port names to their numerical values.\n\nBefore this change, it was not possible to have a SMTP server port set\nto \"smtps\" in a netrc file (e.g. Emacs' ~/.authinfo.gpg), as it would\nbe registered as a protocol and break the match for a \"smtp\" protocol\nhost, as queried for by git-send-email.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 11 +++++++----\n contrib/credential/netrc/test.pl                   |  8 ++++----\n git-send-email.perl                                | 11 +++++++++++\n perl/Git.pm                                        | 13 +++++++++++++\n t/t9001-send-email.sh                              |  7 +++++++\n 5 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 09d77b4f69..3c0a532d0e 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -268,13 +268,16 @@ sub load_netrc {\n \t\t\tnext;\n \t\t}\n \t\tif (defined $nentry->{port}) {\n-\t\t\tif ($nentry->{port} =~ m/^\\d+$/) {\n-\t\t\t\t$num_port = $nentry->{port};\n-\t\t\t\tdelete $nentry->{port};\n-\t\t\t} else {\n+\t\t\t$num_port = Git::port_num($nentry->{port});\n+\t\t\tunless ($num_port) {\n \t\t\t\tprintf(STDERR \"ignoring invalid port `%s' \" .\n \t\t\t\t       \"from netrc file\\n\", $nentry->{port});\n \t\t\t}\n+\t\t\t# Since we've already validated and converted\n+\t\t\t# the port to its numerical value, do not\n+\t\t\t# capture it as the `protocol' value, as used\n+\t\t\t# to be the case for symbolic port names.\n+\t\t\tdelete $nentry->{port};\n \t\t}\n \n \t\t# create the new entry for the credential helper protocol\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 67a0ede564..8a7fc2588a 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -45,7 +45,7 @@ BEGIN\n diag \"Testing with invalid data\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n \t\t       \"bad data\");\n-ok(scalar keys %$cred == 4, \"Got first found keys with bad data\");\n+ok(scalar keys %$cred == 3, \"Got first found keys with bad data\");\n \n diag \"Testing netrc file for a missing corovamilkbar entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n@@ -64,12 +64,12 @@ BEGIN\n \n diag \"Testing netrc file for a username-specific entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\n-\t\t       { host => 'imap', username => 'bob' });\n+\t\t       { host => 'imap:993', username => 'bob' });\n \n-ok(scalar keys %$cred == 2, \"Got 2 username-specific keys\");\n+# Only the password field gets returned.\n+ok(scalar keys %$cred == 1, \"Got 1 username-specific keys\");\n \n is($cred->{password}, 'bobwillknow', \"Got correct user-specific password\");\n-is($cred->{protocol}, 'imaps', \"Got correct user-specific protocol\");\n \n diag \"Testing netrc file for a host:port-specific entry\\n\";\n $cred = run_credential(['-f', $netrc, 'get'],\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 659e6c588b..d2cf9b717a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -2101,6 +2101,17 @@ sub initialize_modified_loop_vars {\n \t\t}\n \t}\n \n+\t# Validate the SMTP server port, if provided.\n+\tif (defined $smtp_server_port) {\n+\t\tmy $port = Git::port_num($smtp_server_port);\n+\t\tif ($port) {\n+\t\t\t$smtp_server_port = $port;\n+\t\t} else  {\n+\t\t\tdie sprintf(__(\"error: invalid SMTP port '%s'\\n\"),\n+\t\t\t\t    $smtp_server_port);\n+\t\t}\n+\t}\n+\n \t# Run the loop once again to avoid gaps in the counter due to FIFO\n \t# arguments provided by the user.\n \tmy $num = 1;\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 6f47d653ab..090cf77dab 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -1061,6 +1061,19 @@ sub _close_cat_blob {\n \tdelete @$self{@vars};\n }\n \n+# Given PORT, a port number or service name, return its numerical\n+# value else undef.\n+sub port_num {\n+    my ($port) = @_;\n+\n+    # Port can be either a positive integer within the 16-bit range...\n+    if ($port =~ /^\\d+$/ && $port > 0 && $port <= (2**16 - 1)) {\n+        return $port;\n+    }\n+\n+    # ... or a symbolic port (service name).\n+    return scalar getservbyname($port, '');\n+}\n \n =item credential_read( FILEHANDLE )\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 0c1af43f6f..e56e0c8d77 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -201,6 +201,13 @@ test_expect_success $PREREQ 'cc trailer with get_maintainer.pl output' '\n \ttest_cmp expected-cc commandline1\n '\n \n+test_expect_failure $PREREQ 'invalid smtp server port value' '\n+\tclean_fake_sendmail &&\n+\tgit send-email -1 --to=recipient@example.com \\\n+\t\t--smtp-server-port=bogus-symbolic-name \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\"\n+'\n+\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n-- \n2.49.0\n\n"},{"id":"520702","messageId":"20250625142511.28857-2-maxim@guixotic.coop","threadId":"63677","inReplyTo":"20250625142511.28857-1-maxim@guixotic.coop","subject":"[PATCH v4 1/3] contrib: use a more portable shebang for git-credential-netrc","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-25T14:25:09Z","receivedAt":"2025-06-25T14:26:09Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"While the installed scripts have their Perl shebang set to PERL_PATH,\nit is nevertheless useful to be able to run the uninstalled script for\nmanual tests while developing. This change makes the shebang more\nportable by having the perl command looked from PATH instead of from a\nfixed location.\n\nSigned-off-by: Maxim Cournoyer <maxim@guixotic.coop>\n---\n contrib/credential/netrc/git-credential-netrc.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl\nindex 9fb998ae09..514f68d00b 100755\n--- a/contrib/credential/netrc/git-credential-netrc.perl\n+++ b/contrib/credential/netrc/git-credential-netrc.perl\n@@ -1,4 +1,4 @@\n-#!/usr/bin/perl\n+#!/usr/bin/env perl\n \n use strict;\n use warnings;\n-- \n2.49.0\n\n"},{"id":"520722","messageId":"xmqq1pr7wyuf.fsf@gitster.g","threadId":"63677","inReplyTo":"20250625142511.28857-1-maxim@guixotic.coop","subject":"Re: [PATCH v4 0/3] git-credential-netrc: better symbolic port names support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-25T16:49:28Z","receivedAt":"2025-06-25T16:49:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maxim Cournoyer <maxim@guixotic.coop> writes:\n\n> This revision fixes a single white space in a new test added in 3/3.\n\nThe contents exactly match what I locally have (as I fixed up the\nprevious round locally before you sent in this iteration).\n\n[v4 0/3] does not look like a reply to [v3 0/3], though.  It has\nthese header lines\n\n    Message-ID: <20250625142511.28857-1-maxim@guixotic.coop>\n    In-Reply-To: <87ecv8k4y9.fsf@terra.mail-host-address-is-not-set>\n    References: <87ecv8k4y9.fsf@terra.mail-host-address-is-not-set>\n\nand refers to the message in the discussion thread of [v3 0/3] in\nwhich you said \"I'll now send a v4 fixing the white space issue,\nmaking sure to --in-reply-to=$message-id-of-v3-cover-letter.\"\n\nNo need to resend this round just to fix the threading, of course.\n\nThanks.\n\n> Maxim Cournoyer (3):\n>   contrib: use a more portable shebang for git-credential-netrc\n>   contrib: warn for invalid netrc file ports in git-credential-netrc\n>   contrib: better support symbolic port names in git-credential-netrc\n>\n>  contrib/credential/netrc/git-credential-netrc.perl | 14 +++++++++++---\n>  contrib/credential/netrc/test.pl                   |  8 ++++----\n>  git-send-email.perl                                | 11 +++++++++++\n>  perl/Git.pm                                        | 13 +++++++++++++\n>  t/t9001-send-email.sh                              |  7 +++++++\n>  5 files changed, 46 insertions(+), 7 deletions(-)\n"},{"id":"520739","messageId":"87sejn2thd.fsf@guixotic.coop","threadId":"63677","inReplyTo":"xmqq1pr7wyuf.fsf@gitster.g","subject":"Re: [PATCH v4 0/3] git-credential-netrc: better symbolic port names support","fromName":"Maxim Cournoyer","fromEmail":"maxim@guixotic.coop","sentAt":"2025-06-26T01:15:42Z","receivedAt":"2025-06-26T01:15:54Z","isPatch":true,"sender":{"key":"maxim@guixotic.coop","avatar":null},"body":"Hi,\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Maxim Cournoyer <maxim@guixotic.coop> writes:\n>\n>> This revision fixes a single white space in a new test added in 3/3.\n>\n> The contents exactly match what I locally have (as I fixed up the\n> previous round locally before you sent in this iteration).\n>\n> [v4 0/3] does not look like a reply to [v3 0/3], though.  It has\n> these header lines\n>\n>     Message-ID: <20250625142511.28857-1-maxim@guixotic.coop>\n>     In-Reply-To: <87ecv8k4y9.fsf@terra.mail-host-address-is-not-set>\n>     References: <87ecv8k4y9.fsf@terra.mail-host-address-is-not-set>\n>\n> and refers to the message in the discussion thread of [v3 0/3] in\n> which you said \"I'll now send a v4 fixing the white space issue,\n> making sure to --in-reply-to=$message-id-of-v3-cover-letter.\"\n\nHm, correct.  I picked the first message I saw as [PATCH v3 0/3] at\nhttps://lore.kernel.org/git/ but it was a reply, no the original.  Maybe\nI'll get it right in a future submission ^^'.\n\nThanks again for the previous comments/review.\n\nCheers,\n\n-- \nMaxim\n"}]}