| Message ID | 20210830153001.29961-1-mhaeuser@posteo.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e0821134846b10952efc0737b9e32ca8226f0a4d |
| Headers | show |
| Series | send-email: Avoid incorrect header propagation | expand |
Ping? :) On 30/08/2021 17:30, Marvin Häuser wrote: > 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 <peff@peff.net> > Signed-off-by: Marvin Häuser <mhaeuser@posteo.de> > --- > 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 && >
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
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: > >> 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
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 &&