From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Michael Strawbridge <michael.strawbridge@amd.com>,
Doug Anderson <dianders@chromium.org>,
Emily Shaffer <nasamuffin@google.com>
Subject: [PATCH] send-email: clear the $message_id after validation
Date: Wed, 17 May 2023 14:10:39 -0700 [thread overview]
Message-ID: <xmqqzg62oe9c.fsf@gitster.g> (raw)
Recently git-send-email started parsing the same message twice, once
to validate _all_ the message before sending even the first one, and
then after the validation hook is happy and each message gets sent,
to read the contents to find out where to send to etc.
Unfortunately, the effect of reading the messages for validation
lingered even after the validation is done. Namely $message_id gets
assigned if exists in the input files but the variable is global,
and it is not cleared before pre_process_file runs. This causes
reading a message without a message-id followed by reading a message
with a message-id to misbehave---the sub reports as if the message
had the same id as the previously written one.
Clear the variable before starting to read the headers in
pre_process_file.
Tested-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This time with a minimum test. I eyeballed what variables are
assigned in pre_process_file and it _appears_ to me that most of
them are cleared in the function before it processes one file
(except for $message_num that gets incremented per invocation for
obvious reasons---and it does get reset to 0 before the real loop
calls the function before sending each message). So $message_id
may indeed be the only one that needs fixing.
But that can hardly qualify as an exhaustive verification X-<.
git-send-email.perl | 2 ++
t/t9001-send-email.sh | 17 ++++++++++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 10c450ef68..37dfd4b8c5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1768,6 +1768,8 @@ sub pre_process_file {
$subject = $initial_subject;
$message = "";
$message_num++;
+ undef $message_id;
+
# First unfold multiline header fields
while(<$fh>) {
last if /^\s*$/;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 36bb85d6b4..8d49eff91a 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -47,7 +47,7 @@ clean_fake_sendmail () {
test_expect_success $PREREQ 'Extract patches' '
patches=$(git format-patch -s --cc="One <one@example.com>" --cc=two@example.com -n HEAD^1) &&
- threaded_patches=$(git format-patch -o threaded -s --in-reply-to="format" HEAD^1)
+ threaded_patches=$(git format-patch -o threaded --thread=shallow -s --in-reply-to="format" HEAD^1)
'
# Test no confirm early to ensure remaining tests will not hang
@@ -588,6 +588,21 @@ test_expect_success $PREREQ "--validate hook supports header argument" '
outdir/000?-*.patch
'
+test_expect_success $PREREQ 'clear message-id before parsing a new message' '
+ clean_fake_sendmail &&
+ echo true | write_script my-hooks/sendemail-validate &&
+ test_config core.hooksPath my-hooks &&
+ GIT_SEND_EMAIL_NOTTY=1 \
+ git send-email --validate --to=recipient@example.com \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ $patches $threaded_patches &&
+ id0=$(grep "^Message-ID: " $threaded_patches) &&
+ id1=$(grep "^Message-ID: " msgtxt1) &&
+ id2=$(grep "^Message-ID: " msgtxt2) &&
+ test "z$id0" = "z$id2" &&
+ test "z$id1" != "z$id2"
+'
+
for enc in 7bit 8bit quoted-printable base64
do
test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '
--
2.41.0-rc0-4-g004e0f790f
next reply other threads:[~2023-05-17 21:10 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 21:10 Junio C Hamano [this message]
2023-05-18 1:11 ` [PATCH] send-email: clear the $message_id after validation Michael Strawbridge
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=xmqqzg62oe9c.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=dianders@chromium.org \
--cc=git@vger.kernel.org \
--cc=michael.strawbridge@amd.com \
--cc=nasamuffin@google.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 public 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).