{"thread":{"id":"25327","subject":"[PATCH] send-email: Clear To: field for every mail","startedAt":"2010-10-04T05:37:12Z","lastAt":"2010-10-04T18:55:43Z","messageCount":9,"participants":["Viresh KUMAR","Stephen Boyd","Junio C Hamano","Ævar Arnfjörð Bjarmason","viresh kumar","Joe Perches"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"152461","messageId":"a9b17bd454e57abb75f6cd2a7da63ec7738f5e7b.1286170305.git.viresh.kumar@st.com","threadId":"25327","inReplyTo":null,"subject":"[PATCH] send-email: Clear To: field for every mail","fromName":"Viresh KUMAR","fromEmail":"viresh.kumar@st.com","sentAt":"2010-10-04T05:37:12Z","receivedAt":"2010-10-04T05:37:12Z","isPatch":true,"sender":{"key":"viresh.kumar@st.com","avatar":null},"body":"While sending multiple patches with a single git-send-email command,\nTo: field is not cleared after every mail. This patch clears To: field\nafter every patch sent.\n\nSigned-off-by: Viresh Kumar \nTested-by: Viresh Kumar \n---\n git-send-email.perl | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 1ccfb80..cf17704 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1150,6 +1150,7 @@ foreach my $t (@files) {\n \tmy $author_encoding;\n \tmy $has_content_type;\n \tmy $body_encoding;\n+\t@to = ();\n \t@cc = ();\n \t@xh = ();\n \tmy $input_format = undef;\n-- \n1.7.2.3\n"},{"id":"152475","messageId":"1286175924-15761-1-git-send-email-bebarino@gmail.com","threadId":"25327","inReplyTo":"a9b17bd454e57abb75f6cd2a7da63ec7738f5e7b.1286170305.git.viresh.kumar@st.com","subject":"[PATCH] send-email: Don't leak To: headers between patches","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2010-10-04T07:05:24Z","receivedAt":"2010-10-04T07:05:24Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"If the first patch in a series has a To: header in the file and the\nsecond patch in the series doesn't the address from the first patch will\nbe part of the To: addresses in the second patch. Fix this by treating the\nto list like the cc list. Have an initial to list come from the command\nline, user input and config options. Then build up a to list from each\npatch and concatenate the two together before sending the patch. Finally,\nreset the list after sending each patch so the To: headers from a patch\ndon't get used for the next one.\n\nReported-by: Viresh Kumar \nSigned-off-by: Stephen Boyd \n---\n\nOn 10/03/2010 10:37 PM, Viresh KUMAR wrote:\n> While sending multiple patches with a single git-send-email command,\n> To: field is not cleared after every mail. This patch clears To: field\n> after every patch sent.\n> \n> Signed-off-by: Viresh Kumar \n> Tested-by: Viresh Kumar \n> ---\n> git-send-email.perl | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 1ccfb80..cf17704 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1150,6 +1150,7 @@ foreach my $t (@files) {\n> \tmy $author_encoding;\n> \tmy $has_content_type;\n> \tmy $body_encoding;\n> +\t@to = ();\n> \t@cc = ();\n> \t@xh = ();\n> \tmy $input_format = undef;\n\nAh, I didn't think about this at all. Your patch doesn't look right though.\nConsider a user adding a --to argument on the command line. The first patch\nwill be sent to the right place but the second one will be sent to nobody.\nRight?\n\nHow about this instead? Based on sb/send-email-use-to-from-input. Junio,\nlooks like this would also fix a similar situation with the to_cmd in\nnext.\n\n git-send-email.perl | 18 ++++++++++--------\n t/t9001-send-email.sh | 15 +++++++++++++++\n 2 files changed, 25 insertions(+), 8 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex d6028ec..7f9eacd 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -138,7 +138,7 @@ sub unique_email_list(@);\n sub cleanup_compose_files();\n \n # Variables we fill in automatically, or via prompting:\n-my (@to,$no_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n+my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n \t$initial_reply_to,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$compose,$time);\n \n@@ -213,7 +213,7 @@ my %config_settings = (\n \"smtpuser\" => \\$smtp_authuser,\n \"smtppass\" => \\$smtp_authpass,\n \t\"smtpdomain\" => \\$smtp_domain,\n- \"to\" => \\@to,\n+ \"to\" => \\@initial_to,\n \"cc\" => \\@initial_cc,\n \"cccmd\" => \\$cc_cmd,\n \"aliasfiletype\" => \\$aliasfiletype,\n@@ -271,7 +271,7 @@ $SIG{INT} = \\&signal_handler;\n my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \"in-reply-to=s\" => \\$initial_reply_to,\n \t\t \"subject=s\" => \\$initial_subject,\n-\t\t \"to=s\" => \\@to,\n+\t\t \"to=s\" => \\@initial_to,\n \t\t \"no-to\" => \\$no_to,\n \t\t \"cc=s\" => \\@initial_cc,\n \t\t \"no-cc\" => \\$no_cc,\n@@ -409,7 +409,7 @@ my ($repoauthor, $repocommitter);\n \n # Verify the user input\n \n-foreach my $entry (@to) {\n+foreach my $entry (@initial_to) {\n \tdie \"Comma in --to entry: $entry'\\n\" unless $entry !~ m/,/;\n }\n \n@@ -711,9 +711,9 @@ if (!defined $sender) {\n \t$prompting++;\n }\n \n-if (!@to) {\n+if (!@initial_to) {\n \tmy $to = ask(\"Who should the emails be sent to? \");\n-\tpush @to, parse_address_line($to) if defined $to; # sanitized/validated later\n+\tpush @initial_to, parse_address_line($to) if defined $to; # sanitized/validated later\n \t$prompting++;\n }\n \n@@ -731,8 +731,8 @@ sub expand_one_alias {\n \treturn $aliases{$alias} ? expand_aliases(@{$aliases{$alias}}) : $alias;\n }\n \n-@to = expand_aliases(@to);\n-@to = (map { sanitize_address($_) } @to);\n+@initial_to = expand_aliases(@initial_to);\n+@initial_to = (map { sanitize_address($_) } @initial_to);\n @initial_cc = expand_aliases(@initial_cc);\n @bcclist = expand_aliases(@bcclist);\n \n@@ -1136,6 +1136,7 @@ foreach my $t (@files) {\n \tmy $author_encoding;\n \tmy $has_content_type;\n \tmy $body_encoding;\n+\t@to = ();\n \t@cc = ();\n \t@xh = ();\n \tmy $input_format = undef;\n@@ -1300,6 +1301,7 @@ foreach my $t (@files) {\n \t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n \t$needs_confirm = \"inform\" if ($needs_confirm && $confirm_unconfigured && @cc);\n \n+\t@to = (@initial_to, @to);\n \t@cc = (@initial_cc, @cc);\n \n \tmy $message_was_sent = send_message();\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 294e31f..13d8d1a 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -971,6 +971,21 @@ test_expect_success $PREREQ 'patches To headers are appended to' '\n \tgrep \"RCPT TO:\" stdout\n '\n \n+test_expect_success $PREREQ 'To headers from files reset each patch' '\n+\tpatch1=`git format-patch -1 --to=\"bodies@example.com\"` &&\n+\tpatch2=`git format-patch -1 --to=\"other@example.com\" HEAD~` &&\n+\ttest_when_finished \"rm $patch1 && rm $patch2\" &&\n+\tgit send-email \\\n+\t\t--dry-run \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=\"nobody@example.com\" \\\n+\t\t--smtp-server relay.example.com \\\n+\t\t$patch1 $patch2 >stdout &&\n+\ttest $(grep -c \"RCPT TO:\" stdout) = 1 &&\n+\ttest $(grep -c \"RCPT TO:\" stdout) = 2 &&\n+\ttest $(grep -c \"RCPT TO:\" stdout) = 1\n+'\n+\n test_expect_success $PREREQ 'setup expect' '\n cat >email-using-8bit < writes:\n\n> While sending multiple patches with a single git-send-email command,\n> To: field is not cleared after every mail. This patch clears To: field\n> after every patch sent.\n>\n> Signed-off-by: Viresh Kumar \n> Tested-by: Viresh Kumar \n> ---\n\nHeh, are people who send patches with only S-o-b by your definition not\ntesting their patches at all ;-)? As far as I can tell, your patch\napplied to 'next' will break t9001 rather badly.\n\nI agree there is a bug that you are trying to address in the series by\nStephen that keeps adding To: address that is read from an earlier output\nof format-patch created with its --to option, but I do not think this is a\nright fix. Have you tested sending a series with a plain format-patch\noutput without extraneous To:, Cc: and such headers?\n\nA normal send-email session takes the recipient address from either --to\nor interactively upfront, and then use those addresses kept in @to\nvariable in the loop, repeatedly. I do not see anything in your patch to\navoid losing these addresses.\n"},{"id":"152477","messageId":"7v39sma0u9.fsf@alter.siamese.dyndns.org","threadId":"25327","inReplyTo":"1286175924-15761-1-git-send-email-bebarino@gmail.com","subject":"Re: [PATCH] send-email: Don't leak To: headers between patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-04T07:11:58Z","receivedAt":"2010-10-04T07:11:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Boyd writes:\n\n> Ah, I didn't think about this at all. Your patch doesn't look right though.\n> Consider a user adding a --to argument on the command line. The first patch\n> will be sent to the right place but the second one will be sent to nobody.\n> Right?\n>\n> How about this instead? Based on sb/send-email-use-to-from-input. Junio,\n> looks like this would also fix a similar situation with the to_cmd in\n> next.\n\nYeah, I haven't applied nor run it, but your fix looks correct. Thanks.\n"},{"id":"152478","messageId":"AANLkTimuP8Myj-PAU76hjtWdOkbzg2WrZwaFNOxRqfsM@mail.gmail.com","threadId":"25327","inReplyTo":"1286175924-15761-1-git-send-email-bebarino@gmail.com","subject":"Re: [PATCH] send-email: Don't leak To: headers between patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-10-04T07:15:13Z","receivedAt":"2010-10-04T07:15:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Oct 4, 2010 at 07:05, Stephen Boyd wrote:\n> If the first patch in a series has a To: header in the file and the\n> second patch in the series doesn't the address from the first patch will\n> be part of the To: addresses in the second patch. Fix this by treating the\n> to list like the cc list. Have an initial to list come from the command\n> line, user input and config options. Then build up a to list from each\n> patch and concatenate the two together before sending the patch. Finally,\n> reset the list after sending each patch so the To: headers from a patch\n> don't get used for the next one.\n\nCouldn't this whole thing be done by:\n\n>  # Variables we fill in automatically, or via prompting:\n> -my (@to,$no_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n> +my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n\nChanging this to an \"our\" variable instead of a \"my\".\n\n>        my $body_encoding;\n> +       @to = ();\n\nThen doing:\n\n local @to = @to;\n\n> +       @to = (@initial_to, @to);\n>        @cc = (@initial_cc, @cc);\n\nAnd keeping this as it is, and should the @cc addresses by accumulated\nacross patches, but not the @to addresses?\n\n> +test_expect_success $PREREQ 'To headers from files reset each patch' '\n> +       patch1=`git format-patch -1 --to=\"bodies@example.com\"` &&\n> +       patch2=`git format-patch -1 --to=\"other@example.com\" HEAD~` &&\n> +       test_when_finished \"rm $patch1 && rm $patch2\" &&\n> +       git send-email \\\n> +               --dry-run \\\n> +               --from=\"Example \" \\\n> +               --to=\"nobody@example.com\" \\\n> +               --smtp-server relay.example.com \\\n> +               $patch1 $patch2 >stdout &&\n> +       test $(grep -c \"RCPT TO:\" stdout) = 1 &&\n> +       test $(grep -c \"RCPT TO:\" stdout) = 2 &&\n> +       test $(grep -c \"RCPT TO:\" stdout) = 1\n> +'\n> +\n"},{"id":"152480","messageId":"AANLkTinnd-ZTqfKvEaDQ6o-gR2oAmvEChSpDps5T0Xsu@mail.gmail.com","threadId":"25327","inReplyTo":"AANLkTimuP8Myj-PAU76hjtWdOkbzg2WrZwaFNOxRqfsM@mail.gmail.com","subject":"Re: [PATCH] send-email: Don't leak To: headers between patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-10-04T07:25:47Z","receivedAt":"2010-10-04T07:25:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Oct 4, 2010 at 07:15, Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Oct 4, 2010 at 07:05, Stephen Boyd wrote:\n>> If the first patch in a series has a To: header in the file and the\n>> second patch in the series doesn't the address from the first patch will\n>> be part of the To: addresses in the second patch. Fix this by treating the\n>> to list like the cc list. Have an initial to list come from the command\n>> line, user input and config options. Then build up a to list from each\n>> patch and concatenate the two together before sending the patch. Finally,\n>> reset the list after sending each patch so the To: headers from a patch\n>> don't get used for the next one.\n>\n> Couldn't this whole thing be done by:\n>\n>>  # Variables we fill in automatically, or via prompting:\n>> -my (@to,$no_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n>> +my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n>\n> Changing this to an \"our\" variable instead of a \"my\".\n>\n>>        my $body_encoding;\n>> +       @to = ();\n>\n> Then doing:\n>\n>    local @to = @to;\n>\n>> +       @to = (@initial_to, @to);\n>>        @cc = (@initial_cc, @cc);\n\nSmall brainfart, you don't have to change it to an \"our\" and use\n\"local\", you can just use \"my\" in that for-loop:\n\n $ perl -E 'my @a = qw(a b); { my @a = (@a, \"c\"); say \"@a\" } say \"@a\"'\n a b c\n a b\n\nThat's a much better solution IMO than the C-like usage of two\nvariables. Lexical shadowing is exactly for this sort of thing.\n"},{"id":"152483","messageId":"4CA98724.2020203@st.com","threadId":"25327","inReplyTo":"7v7hhya0yc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Clear To: field for every mail","fromName":"viresh kumar","fromEmail":"viresh.kumar@st.com","sentAt":"2010-10-04T07:49:56Z","receivedAt":"2010-10-04T07:49:56Z","isPatch":true,"sender":{"key":"viresh.kumar@st.com","avatar":null},"body":"On 10/04/2010 12:39 PM, Junio C Hamano wrote:\n> Heh, are people who send patches with only S-o-b by your definition not\n> testing their patches at all ;-)? As far as I can tell, your patch\n> applied to 'next' will break t9001 rather badly.\n> \n> I agree there is a bug that you are trying to address in the series by\n> Stephen that keeps adding To: address that is read from an earlier output\n> of format-patch created with its --to option, but I do not think this is a\n> right fix. Have you tested sending a series with a plain format-patch\n> output without extraneous To:, Cc: and such headers?\n> \n> A normal send-email session takes the recipient address from either --to\n> or interactively upfront, and then use those addresses kept in @to\n> variable in the loop, repeatedly. I do not see anything in your patch to\n> avoid losing these addresses.\n\nJunio, Stephan,\n\nYa! my patch wasn't good enough. I just tried to solve it the way it was done\nfor cc.\n\n-- \nviresh\n"},{"id":"152490","messageId":"7vpqvq8k1i.fsf@alter.siamese.dyndns.org","threadId":"25327","inReplyTo":"AANLkTinnd-ZTqfKvEaDQ6o-gR2oAmvEChSpDps5T0Xsu@mail.gmail.com","subject":"Re: [PATCH] send-email: Don't leak To: headers between patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-04T08:00:09Z","receivedAt":"2010-10-04T08:00:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> That's a much better solution IMO than the C-like usage of two\n> variables. Lexical shadowing is exactly for this sort of thing.\n\nLexical scoping, yes, shadowing a variable of the same name in the outer\nscope, no. The latter is a readability nightmare in the longer term.\n\nIOW, I would prefer to keep @initial_to and @initial_cc as \"these apply\nglobally to the whole command invocation\", copied to separate @to and @cc\nthat are primed by the former and tweaked per message.\n"},{"id":"152568","messageId":"1286218543.10512.57.camel@Joe-Laptop","threadId":"25327","inReplyTo":"7vpqvq8k1i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] send-email: Don't leak To: headers between patches","fromName":"Joe Perches","fromEmail":"joe@perches.com","sentAt":"2010-10-04T18:55:43Z","receivedAt":"2010-10-04T18:55:43Z","isPatch":true,"sender":{"key":"joe@perches.com","avatar":"https://avatars.githubusercontent.com/u/13122723?v=4"},"body":"On Mon, 2010-10-04 at 01:00 -0700, Junio C Hamano wrote:\n> I would prefer to keep @initial_to and @initial_cc as \"these apply\n> globally to the whole command invocation\", copied to separate @to and @cc\n> that are primed by the former and tweaked per message.\n\nMakes sense to me, thanks.\n\nIs there a need to add initial_to and\ninitial_cc to the test suite for a patch\nseries? I believe the test suite is\ncurrently setup for single patch testing.\n"}]}