{"thread":{"id":"56406","subject":"[PATCH] send-email: Avoid incorrect header propagation","startedAt":"2021-08-30T15:30:46Z","lastAt":"2021-09-15T18:07:58Z","messageCount":4,"participants":["Marvin Häuser","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"434112","messageId":"20210830153001.29961-1-mhaeuser@posteo.de","threadId":"56406","inReplyTo":null,"subject":"[PATCH] send-email: Avoid incorrect header propagation","fromName":"Marvin Häuser","fromEmail":"mhaeuser@posteo.de","sentAt":"2021-08-30T15:30:01Z","receivedAt":"2021-08-30T15:30:46Z","isPatch":true,"sender":{"key":"mhaeuser@posteo.de","avatar":"https://avatars.githubusercontent.com/u/8659494?v=4"},"body":"If multiple independent patches are sent with send-email, even if the\n\"In-Reply-To\" and \"References\" headers are not managed by --thread or\n--in-reply-to, their values may be propagated from prior patches to\nsubsequent patches with no such headers defined.\n\nTo mitigate this and potential future issues, make sure all global\npatch-specific variables are always either handled by\ncommand-specific code (e.g. threading), or are reset to their default\nvalues for every iteration.\n\nSigned-off-by: Jeff King \nSigned-off-by: Marvin Häuser \n---\n git-send-email.perl | 26 ++++++++++++++++---------\n t/t9001-send-email.sh | 45 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 9 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 25be2ebd2a..e411860b18 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1625,7 +1625,6 @@ sub send_message {\n \n $in_reply_to = $initial_in_reply_to;\n $references = $initial_in_reply_to || '';\n-$subject = $initial_subject;\n $message_num = 0;\n \n # Prepares the email, prompts the user, sends it out\n@@ -1648,6 +1647,7 @@ sub process_file {\n \t@xh = ();\n \tmy $input_format = undef;\n \tmy @header = ();\n+\t$subject = $initial_subject;\n \t$message = \"\";\n \t$message_num++;\n \t# First unfold multiline header fields\n@@ -1854,15 +1854,23 @@ sub process_file {\n \t}\n \n \t# set up for the next message\n-\tif ($thread && $message_was_sent &&\n-\t\t($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||\n-\t\t$message_num == 1)) {\n-\t\t$in_reply_to = $message_id;\n-\t\tif (length $references > 0) {\n-\t\t\t$references .= \"\\n $message_id\";\n-\t\t} else {\n-\t\t\t$references = \"$message_id\";\n+\tif ($thread) {\n+\t\tif ($message_was_sent &&\n+\t\t ($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||\n+\t\t $message_num == 1)) {\n+\t\t\t$in_reply_to = $message_id;\n+\t\t\tif (length $references > 0) {\n+\t\t\t\t$references .= \"\\n $message_id\";\n+\t\t\t} else {\n+\t\t\t\t$references = \"$message_id\";\n+\t\t\t}\n \t\t}\n+\t} elsif (!defined $initial_in_reply_to) {\n+\t\t# --thread and --in-reply-to manage the \"In-Reply-To\" header and by\n+\t\t# extension the \"References\" header. If these commands are not used, reset\n+\t\t# the header values to their defaults.\n+\t\t$in_reply_to = undef;\n+\t\t$references = '';\n \t}\n \t$message_id = undef;\n \t$num_sent++;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3b7540050c..f95177af39 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -2167,6 +2167,51 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n \ttest_cmp expected-list actual-list\n '\n \n+test_expect_success $PREREQ 'set up in-reply-to/references patches' '\n+\tcat >has-reply.patch <<-\\EOF &&\n+\tFrom: A U Thor \n+\tSubject: patch with in-reply-to\n+\tMessage-ID: \n+\tIn-Reply-To: \n+\tReferences: \n+\n+\tThis is the body.\n+\tEOF\n+\tcat >no-reply.patch <<-\\EOF\n+\tFrom: A U Thor \n+\tSubject: patch without in-reply-to\n+\tMessage-ID: \n+\n+\tThis is the body.\n+\tEOF\n+'\n+\n+test_expect_success $PREREQ 'patch reply headers correct with --no-thread' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--no-thread \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\thas-reply.patch no-reply.patch &&\n+\tgrep \"In-Reply-To: \" msgtxt1 &&\n+\tgrep \"References: \" msgtxt1 &&\n+\t! grep replied.to@example.com msgtxt2\n+'\n+\n+test_expect_success $PREREQ 'cmdline in-reply-to used with --no-thread' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--no-thread \\\n+\t\t--in-reply-to=\"\" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\thas-reply.patch no-reply.patch &&\n+\tgrep \"In-Reply-To: \" msgtxt1 &&\n+\tgrep \"References: \" msgtxt1 &&\n+\tgrep \"In-Reply-To: \" msgtxt2 &&\n+\tgrep \"References: \" msgtxt2\n+'\n+\n test_expect_success $PREREQ 'invoke hook' '\n \tmkdir -p .git/hooks &&\n \n-- \n2.31.1\n\n"},{"id":"436002","messageId":"0c753ae6-cc2c-2d70-c510-5370a0b4dcf9@posteo.de","threadId":"56406","inReplyTo":"20210830153001.29961-1-mhaeuser@posteo.de","subject":"Re: [PATCH] send-email: Avoid incorrect header propagation","fromName":"Marvin Häuser","fromEmail":"mhaeuser@posteo.de","sentAt":"2021-09-15T17:00:26Z","receivedAt":"2021-09-15T17:00:34Z","isPatch":true,"sender":{"key":"mhaeuser@posteo.de","avatar":"https://avatars.githubusercontent.com/u/8659494?v=4"},"body":"Ping? :)\n\nOn 30/08/2021 17:30, Marvin Häuser wrote:\n> If multiple independent patches are sent with send-email, even if the\n> \"In-Reply-To\" and \"References\" headers are not managed by --thread or\n> --in-reply-to, their values may be propagated from prior patches to\n> subsequent patches with no such headers defined.\n>\n> To mitigate this and potential future issues, make sure all global\n> patch-specific variables are always either handled by\n> command-specific code (e.g. threading), or are reset to their default\n> values for every iteration.\n>\n> Signed-off-by: Jeff King \n> Signed-off-by: Marvin Häuser \n> ---\n> git-send-email.perl | 26 ++++++++++++++++---------\n> t/t9001-send-email.sh | 45 +++++++++++++++++++++++++++++++++++++++++++\n> 2 files changed, 62 insertions(+), 9 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 25be2ebd2a..e411860b18 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1625,7 +1625,6 @@ sub send_message {\n> \n> $in_reply_to = $initial_in_reply_to;\n> $references = $initial_in_reply_to || '';\n> -$subject = $initial_subject;\n> $message_num = 0;\n> \n> # Prepares the email, prompts the user, sends it out\n> @@ -1648,6 +1647,7 @@ sub process_file {\n> \t@xh = ();\n> \tmy $input_format = undef;\n> \tmy @header = ();\n> +\t$subject = $initial_subject;\n> \t$message = \"\";\n> \t$message_num++;\n> \t# First unfold multiline header fields\n> @@ -1854,15 +1854,23 @@ sub process_file {\n> \t}\n> \n> \t# set up for the next message\n> -\tif ($thread && $message_was_sent &&\n> -\t\t($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||\n> -\t\t$message_num == 1)) {\n> -\t\t$in_reply_to = $message_id;\n> -\t\tif (length $references > 0) {\n> -\t\t\t$references .= \"\\n $message_id\";\n> -\t\t} else {\n> -\t\t\t$references = \"$message_id\";\n> +\tif ($thread) {\n> +\t\tif ($message_was_sent &&\n> +\t\t ($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||\n> +\t\t $message_num == 1)) {\n> +\t\t\t$in_reply_to = $message_id;\n> +\t\t\tif (length $references > 0) {\n> +\t\t\t\t$references .= \"\\n $message_id\";\n> +\t\t\t} else {\n> +\t\t\t\t$references = \"$message_id\";\n> +\t\t\t}\n> \t\t}\n> +\t} elsif (!defined $initial_in_reply_to) {\n> +\t\t# --thread and --in-reply-to manage the \"In-Reply-To\" header and by\n> +\t\t# extension the \"References\" header. If these commands are not used, reset\n> +\t\t# the header values to their defaults.\n> +\t\t$in_reply_to = undef;\n> +\t\t$references = '';\n> \t}\n> \t$message_id = undef;\n> \t$num_sent++;\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 3b7540050c..f95177af39 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -2167,6 +2167,51 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n> \ttest_cmp expected-list actual-list\n> '\n> \n> +test_expect_success $PREREQ 'set up in-reply-to/references patches' '\n> +\tcat >has-reply.patch <<-\\EOF &&\n> +\tFrom: A U Thor \n> +\tSubject: patch with in-reply-to\n> +\tMessage-ID: \n> +\tIn-Reply-To: \n> +\tReferences: \n> +\n> +\tThis is the body.\n> +\tEOF\n> +\tcat >no-reply.patch <<-\\EOF\n> +\tFrom: A U Thor \n> +\tSubject: patch without in-reply-to\n> +\tMessage-ID: \n> +\n> +\tThis is the body.\n> +\tEOF\n> +'\n> +\n> +test_expect_success $PREREQ 'patch reply headers correct with --no-thread' '\n> +\tclean_fake_sendmail &&\n> +\tgit send-email \\\n> +\t\t--no-thread \\\n> +\t\t--to=nobody@example.com \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\thas-reply.patch no-reply.patch &&\n> +\tgrep \"In-Reply-To: \" msgtxt1 &&\n> +\tgrep \"References: \" msgtxt1 &&\n> +\t! grep replied.to@example.com msgtxt2\n> +'\n> +\n> +test_expect_success $PREREQ 'cmdline in-reply-to used with --no-thread' '\n> +\tclean_fake_sendmail &&\n> +\tgit send-email \\\n> +\t\t--no-thread \\\n> +\t\t--in-reply-to=\"\" \\\n> +\t\t--to=nobody@example.com \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\thas-reply.patch no-reply.patch &&\n> +\tgrep \"In-Reply-To: \" msgtxt1 &&\n> +\tgrep \"References: \" msgtxt1 &&\n> +\tgrep \"In-Reply-To: \" msgtxt2 &&\n> +\tgrep \"References: \" msgtxt2\n> +'\n> +\n> test_expect_success $PREREQ 'invoke hook' '\n> \tmkdir -p .git/hooks &&\n> \n\n"},{"id":"436009","messageId":"87o88tn2z5.fsf@evledraar.gmail.com","threadId":"56406","inReplyTo":"0c753ae6-cc2c-2d70-c510-5370a0b4dcf9@posteo.de","subject":"Re: [PATCH] send-email: Avoid incorrect header propagation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-15T17:47:01Z","receivedAt":"2021-09-15T17:49:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Sep 15 2021, Marvin Häuser wrote:\n\n> Ping? :)\n\nHi there. The patch you sent has been picked up after you sent it and\nhas now been merged to the master branch of git.git. See\nhttps://github.com/git/git/commit/bd29bcf9136 and your commit at\nhttps://github.com/git/git/commit/e0821134846\n\nThis means it'll be in the next release, v2.34, set to come out\nmid-to-late November.\n\nThanks a lot for contributing to git, for future submissions you can\nkeep an eye on the What's Cooking summaries that get sent out to the\nlist, for your patch see:\nhttps://lore.kernel.org/git/?q=%22send-email%3A+avoid+incorrect+header+propagation%22\n"},{"id":"436011","messageId":"155e591f-3b85-1819-0e46-017e3bbedef0@posteo.de","threadId":"56406","inReplyTo":"87o88tn2z5.fsf@evledraar.gmail.com","subject":"Re: [PATCH] send-email: Avoid incorrect header propagation","fromName":"Marvin Häuser","fromEmail":"mhaeuser@posteo.de","sentAt":"2021-09-15T18:07:49Z","receivedAt":"2021-09-15T18:07:58Z","isPatch":true,"sender":{"key":"mhaeuser@posteo.de","avatar":"https://avatars.githubusercontent.com/u/8659494?v=4"},"body":"Hey,\n\nSorry, I'm used to getting Reviewed-by mails from other lists. I will \ncheck more carefully next time. :)\nThanks for the merge and the links!\n\nBest regards,\nMarvin\n\nOn 15/09/2021 19:47, Ævar Arnfjörð Bjarmason wrote:\n> On Wed, Sep 15 2021, Marvin Häuser wrote:\n>\n>> Ping? :)\n> Hi there. The patch you sent has been picked up after you sent it and\n> has now been merged to the master branch of git.git. See\n> https://github.com/git/git/commit/bd29bcf9136 and your commit at\n> https://github.com/git/git/commit/e0821134846\n>\n> This means it'll be in the next release, v2.34, set to come out\n> mid-to-late November.\n>\n> Thanks a lot for contributing to git, for future submissions you can\n> keep an eye on the What's Cooking summaries that get sent out to the\n> list, for your patch see:\n> https://lore.kernel.org/git/?q=%22send-email%3A+avoid+incorrect+header+propagation%22\n\n"}]}