From 3ece9bf0f9e24909b090cf348d89e8920bd4f82f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 17 May 2023 14:10:39 -0700 Subject: [PATCH 1/2] send-email: clear the $message_id after validation 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 Signed-off-by: Junio C Hamano --- 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 10c450ef689..37dfd4b8c57 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 36bb85d6b4a..8d49eff91a3 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 " --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" ' From 20bd08aefb20168c6c227d2bfd1965761f9201ea Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 17 May 2023 14:10:39 -0700 Subject: [PATCH 2/2] t9001: mark the script as no longer leak checker clean The test uses "format-patch --thread" which is known to leak the generated message ID list. Plugging these leaks involves straightening out the memory ownership rules around rev_info.message_id and rev_info.ref_message_ids, and is beyond the scope of send-email fix, so for now mark the test as leaky to unblock the topic before the release. Signed-off-by: Junio C Hamano --- t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 8d49eff91a3..20511032263 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -4,7 +4,7 @@ test_description='git send-email' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME -TEST_PASSES_SANITIZE_LEAK=true +# no longer TEST_PASSES_SANITIZE_LEAK=true - format-patch --thread leaks . ./test-lib.sh # May be altered later in the test