git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Drew DeVault <sir@cmpwn.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2] git-send-email: die if sendmail.* config is set
Date: Sat, 18 Jul 2020 23:07:49 -0700	[thread overview]
Message-ID: <xmqqv9ik9gwa.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <xmqq8sfgbltq.fsf@gitster.c.googlers.com> (Junio C. Hamano's message of "Sat, 18 Jul 2020 13:38:25 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> Drew DeVault <sir@cmpwn.com> writes:
>
>> I've seen several people mis-configure git send-email on their first
>> attempt because they set the sendmail.* config options - not
>> sendemail.*. This patch detects this mistake and bails out with a
>> friendly warning.
>>
>> Signed-off-by: Drew DeVault <sir@cmpwn.com>
>> ---
>>  Documentation/config/sendemail.txt |  5 +++++
>>  git-send-email.perl                |  8 ++++++++
>>  perl/Git.pm                        | 26 ++++++++++++++++++++++++++
>>  3 files changed, 39 insertions(+)

One more thing.  This should be fairly easy to protect from future
breakages by adding three new tests in t/t9001-send-email.sh script.
One would do something like

	test_config sendmail.program sendmail &&
	test_must_fail git send-email ... >err &&
	test_i18ngrep "found configuration options for .sendmail" err

as a positive test, the second would do

	test_config sendmail.program sendmail &&
	test_config sendemail.forbidsendmailvariables false &&
	git send-email ...

to make sure that escape hatch actually works and then the third
would do something like

	test_config resendmail.program resendmail &&
	git send-email ...

to ensure that only variable whose name begins with "sendmail."
triggers the error.

>> +if ($forbid_sendmail_variables && (scalar Git::config_regexp("sendmail.*")) != 0) {
>
> Judging from the way you wrote the "config_regexp" helper function,
> the above regexp matches "sendmailer.foo", "sendmailed.bar", etc., I
> would think, which probably is not what you intended.  
>
> I guess we can write "sendmail[.].*" or "sendmail\\..*" to ensure
> that we are talking about (literally) "sendmail." followed by
> anything?

I didn't know "git config --get-regexp $regexp" did not anchor the
regular expression to the beginning or to the end.  In this case, we
do want to make sure the "sendmail." substring literally appears at
the very beginning of the variable name, and because "--get-regexp"
does not anchor the regular expression to the end, we do not need to
add an explicit "anything goes", i.e. ".*" after it.

IOW, "^sendmail[.]" is the minimal regexp we want to use.  We cannot
afford to lose the "^" to reject "resendmail.program", and we do not
have to add ".*" that would swallow the rest at the end.

Thanks.


  reply	other threads:[~2020-07-19  6:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-18 20:21 Drew DeVault
2020-07-18 20:38 ` Junio C Hamano
2020-07-19  6:07   ` Junio C Hamano [this message]
2020-07-20 17:33 ` Jeff King
2020-07-20 17:40   ` Drew DeVault
2020-07-24  0:42     ` Junio C Hamano
2020-07-24  0:43       ` Drew DeVault
2020-07-24  0:59         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqv9ik9gwa.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sir@cmpwn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).