[PATCH] send-email: Avoid incorrect header propagation

Subsystems: the rest

STALE1782d

4 messages, 2 authors, 2021-09-15 · open the first message on its own page

[PATCH] send-email: Avoid incorrect header propagation

From: Marvin Häuser <hidden>
Date: 2021-08-30 15:30:46

If multiple independent patches are sent with send-email, even if the
"In-Reply-To" and "References" headers are not managed by --thread or
--in-reply-to, their values may be propagated from prior patches to
subsequent patches with no such headers defined.

To mitigate this and potential future issues, make sure all global
patch-specific variables are always either handled by
command-specific code (e.g. threading), or are reset to their default
values for every iteration.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Marvin Häuser <redacted>
---
 git-send-email.perl   | 26 ++++++++++++++++---------
 t/t9001-send-email.sh | 45 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 25be2ebd2a..e411860b18 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1625,7 +1625,6 @@ sub send_message {
 
 $in_reply_to = $initial_in_reply_to;
 $references = $initial_in_reply_to || '';
-$subject = $initial_subject;
 $message_num = 0;
 
 # Prepares the email, prompts the user, sends it out
@@ -1648,6 +1647,7 @@ sub process_file {
 	@xh = ();
 	my $input_format = undef;
 	my @header = ();
+	$subject = $initial_subject;
 	$message = "";
 	$message_num++;
 	# First unfold multiline header fields
@@ -1854,15 +1854,23 @@ sub process_file {
 	}
 
 	# set up for the next message
-	if ($thread && $message_was_sent &&
-		($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||
-		$message_num == 1)) {
-		$in_reply_to = $message_id;
-		if (length $references > 0) {
-			$references .= "\n $message_id";
-		} else {
-			$references = "$message_id";
+	if ($thread) {
+		if ($message_was_sent &&
+		  ($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||
+		  $message_num == 1)) {
+			$in_reply_to = $message_id;
+			if (length $references > 0) {
+				$references .= "\n $message_id";
+			} else {
+				$references = "$message_id";
+			}
 		}
+	} elsif (!defined $initial_in_reply_to) {
+		# --thread and --in-reply-to manage the "In-Reply-To" header and by
+		# extension the "References" header. If these commands are not used, reset
+		# the header values to their defaults.
+		$in_reply_to = undef;
+		$references = '';
 	}
 	$message_id = undef;
 	$num_sent++;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3b7540050c..f95177af39 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2167,6 +2167,51 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
 	test_cmp expected-list actual-list
 '
 
+test_expect_success $PREREQ 'set up in-reply-to/references patches' '
+	cat >has-reply.patch <<-\EOF &&
+	From: A U Thor <author@example.com>
+	Subject: patch with in-reply-to
+	Message-ID: <patch.with.in.reply.to@example.com>
+	In-Reply-To: <replied.to@example.com>
+	References: <replied.to@example.com>
+
+	This is the body.
+	EOF
+	cat >no-reply.patch <<-\EOF
+	From: A U Thor <author@example.com>
+	Subject: patch without in-reply-to
+	Message-ID: <patch.without.in.reply.to@example.com>
+
+	This is the body.
+	EOF
+'
+
+test_expect_success $PREREQ 'patch reply headers correct with --no-thread' '
+	clean_fake_sendmail &&
+	git send-email \
+		--no-thread \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		has-reply.patch no-reply.patch &&
+	grep "In-Reply-To: <replied.to@example.com>" msgtxt1 &&
+	grep "References: <replied.to@example.com>" msgtxt1 &&
+	! grep replied.to@example.com msgtxt2
+'
+
+test_expect_success $PREREQ 'cmdline in-reply-to used with --no-thread' '
+	clean_fake_sendmail &&
+	git send-email \
+		--no-thread \
+		--in-reply-to="<cmdline.reply@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		has-reply.patch no-reply.patch &&
+	grep "In-Reply-To: <cmdline.reply@example.com>" msgtxt1 &&
+	grep "References: <cmdline.reply@example.com>" msgtxt1 &&
+	grep "In-Reply-To: <cmdline.reply@example.com>" msgtxt2 &&
+	grep "References: <cmdline.reply@example.com>" msgtxt2
+'
+
 test_expect_success $PREREQ 'invoke hook' '
 	mkdir -p .git/hooks &&
 
-- 
2.31.1

Re: [PATCH] send-email: Avoid incorrect header propagation

From: Marvin Häuser <hidden>
Date: 2021-09-15 17:00:34

Ping? :)

On 30/08/2021 17:30, Marvin Häuser wrote:
quoted hunk
If multiple independent patches are sent with send-email, even if the
"In-Reply-To" and "References" headers are not managed by --thread or
--in-reply-to, their values may be propagated from prior patches to
subsequent patches with no such headers defined.

To mitigate this and potential future issues, make sure all global
patch-specific variables are always either handled by
command-specific code (e.g. threading), or are reset to their default
values for every iteration.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Marvin Häuser <redacted>
---
  git-send-email.perl   | 26 ++++++++++++++++---------
  t/t9001-send-email.sh | 45 +++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 25be2ebd2a..e411860b18 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1625,7 +1625,6 @@ sub send_message {
  
  $in_reply_to = $initial_in_reply_to;
  $references = $initial_in_reply_to || '';
-$subject = $initial_subject;
  $message_num = 0;
  
  # Prepares the email, prompts the user, sends it out
@@ -1648,6 +1647,7 @@ sub process_file {
  	@xh = ();
  	my $input_format = undef;
  	my @header = ();
+	$subject = $initial_subject;
  	$message = "";
  	$message_num++;
  	# First unfold multiline header fields
@@ -1854,15 +1854,23 @@ sub process_file {
  	}
  
  	# set up for the next message
-	if ($thread && $message_was_sent &&
-		($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||
-		$message_num == 1)) {
-		$in_reply_to = $message_id;
-		if (length $references > 0) {
-			$references .= "\n $message_id";
-		} else {
-			$references = "$message_id";
+	if ($thread) {
+		if ($message_was_sent &&
+		  ($chain_reply_to || !defined $in_reply_to || length($in_reply_to) == 0 ||
+		  $message_num == 1)) {
+			$in_reply_to = $message_id;
+			if (length $references > 0) {
+				$references .= "\n $message_id";
+			} else {
+				$references = "$message_id";
+			}
  		}
+	} elsif (!defined $initial_in_reply_to) {
+		# --thread and --in-reply-to manage the "In-Reply-To" header and by
+		# extension the "References" header. If these commands are not used, reset
+		# the header values to their defaults.
+		$in_reply_to = undef;
+		$references = '';
  	}
  	$message_id = undef;
  	$num_sent++;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3b7540050c..f95177af39 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2167,6 +2167,51 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '
  	test_cmp expected-list actual-list
  '
  
+test_expect_success $PREREQ 'set up in-reply-to/references patches' '
+	cat >has-reply.patch <<-\EOF &&
+	From: A U Thor <author@example.com>
+	Subject: patch with in-reply-to
+	Message-ID: <patch.with.in.reply.to@example.com>
+	In-Reply-To: <replied.to@example.com>
+	References: <replied.to@example.com>
+
+	This is the body.
+	EOF
+	cat >no-reply.patch <<-\EOF
+	From: A U Thor <author@example.com>
+	Subject: patch without in-reply-to
+	Message-ID: <patch.without.in.reply.to@example.com>
+
+	This is the body.
+	EOF
+'
+
+test_expect_success $PREREQ 'patch reply headers correct with --no-thread' '
+	clean_fake_sendmail &&
+	git send-email \
+		--no-thread \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		has-reply.patch no-reply.patch &&
+	grep "In-Reply-To: <replied.to@example.com>" msgtxt1 &&
+	grep "References: <replied.to@example.com>" msgtxt1 &&
+	! grep replied.to@example.com msgtxt2
+'
+
+test_expect_success $PREREQ 'cmdline in-reply-to used with --no-thread' '
+	clean_fake_sendmail &&
+	git send-email \
+		--no-thread \
+		--in-reply-to="<cmdline.reply@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		has-reply.patch no-reply.patch &&
+	grep "In-Reply-To: <cmdline.reply@example.com>" msgtxt1 &&
+	grep "References: <cmdline.reply@example.com>" msgtxt1 &&
+	grep "In-Reply-To: <cmdline.reply@example.com>" msgtxt2 &&
+	grep "References: <cmdline.reply@example.com>" msgtxt2
+'
+
  test_expect_success $PREREQ 'invoke hook' '
  	mkdir -p .git/hooks &&
  

Re: [PATCH] send-email: Avoid incorrect header propagation

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-15 17:49:40

On Wed, Sep 15 2021, Marvin Häuser wrote:
Ping? :)
Hi there. The patch you sent has been picked up after you sent it and
has now been merged to the master branch of git.git. See
https://github.com/git/git/commit/bd29bcf9136 and your commit at
https://github.com/git/git/commit/e0821134846

This means it'll be in the next release, v2.34, set to come out
mid-to-late November.

Thanks a lot for contributing to git, for future submissions you can
keep an eye on the What's Cooking summaries that get sent out to the
list, for your patch see:
https://lore.kernel.org/git/?q=%22send-email%3A+avoid+incorrect+header+propagation%22

Re: [PATCH] send-email: Avoid incorrect header propagation

From: Marvin Häuser <hidden>
Date: 2021-09-15 18:07:58

Hey,

Sorry, I'm used to getting Reviewed-by mails from other lists. I will 
check more carefully next time. :)
Thanks for the merge and the links!

Best regards,
Marvin

On 15/09/2021 19:47, Ævar Arnfjörð Bjarmason wrote:
On Wed, Sep 15 2021, Marvin Häuser wrote:
quoted
Ping? :)
Hi there. The patch you sent has been picked up after you sent it and
has now been merged to the master branch of git.git. See
https://github.com/git/git/commit/bd29bcf9136 and your commit at
https://github.com/git/git/commit/e0821134846

This means it'll be in the next release, v2.34, set to come out
mid-to-late November.

Thanks a lot for contributing to git, for future submissions you can
keep an eye on the What's Cooking summaries that get sent out to the
list, for your patch see:
https://lore.kernel.org/git/?q=%22send-email%3A+avoid+incorrect+header+propagation%22

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help