# [RFC] git-send-email: do not double-escape quotes from mutt 7 messages from 2015-12-27 to 2016-01-04. Participants: Eric Wong, Junio C Hamano, Matthieu Moy. Thread: https://gitlist.dev/t/41081 ## Eric Wong, 2015-12-27 02:08 Subject: [RFC] git-send-email: do not double-escape quotes from mutt Message-ID: <20151227020810.1450-1-normalperson@yhbt.net> URL: https://gitlist.dev/e/20151227020810.1450-1-normalperson%40yhbt.net ``` mutt saves aliases with escaped quotes in the form of: alias dot \"Dot U. Sir\" When we pass through our sanitize_address routine, we end up with double-escaping: To: "\\\"Dot U. Sir\\\" Remove the escaping in mutt only for now, as I am not sure if other mailers can do this or if this is better fixed in sanitize_address. Cc: Remi Lespinet Cc: Matthieu Moy --- Cc-ing Remi and Matthieu since they might know better about fixing this in sanitize_address instead or if it's better being a one-off for mutt. git-send-email.perl | 10 ++++++++-- t/t9001-send-email.sh | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index c2abdaa..2d3f51e 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -524,8 +524,14 @@ my %parse_alias = ( if (/^\s*alias\s+(?:-group\s+\S+\s+)*(\S+)\s+(.*)$/) { my ($alias, $addr) = ($1, $2); $addr =~ s/#.*$//; # mutt allows # comments - # commas delimit multiple addresses - $aliases{$alias} = [ split_addrs($addr) ]; + # commas delimit multiple addresses + my @addr = split_addrs($addr); + + # quotes may be escaped in the file, + # remove them if paired so we do not + # double-escape them later. + s/^\\"(.*)\\"/"$1"/g foreach @addr; + $aliases{$alias} = \@addr }}}, mailrc => sub { my $fh = shift; while (<$fh>) { if (/^alias\s+(\S+)\s+(.*)$/) { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 3c49536..834d91a 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' ' test_cover_addresses "Cc" ' +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' ' + clean_fake_sendmail && + echo "alias sbd \\\"Dot U. Sir\\\" " >.mutt && + git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" && + git config sendemail.aliasfiletype mutt && + git send-email \ + --from="Example " \ + --to=sbd \ + --smtp-server="$(pwd)/fake.sendmail" \ + outdir/0001-*.patch \ + 2>errors >out && + grep "^!somebody@example\.org!$" commandline1 && + grep -F "To: \"Dot U. Sir\" " out +' + test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' clean_fake_sendmail && echo "alias sbd somebody@example.org" >.mailrc && -- EW ``` ## Junio C Hamano, 2015-12-28 22:34 Subject: Re: [RFC] git-send-email: do not double-escape quotes from mutt Message-ID: URL: https://gitlist.dev/e/xmqqlh8ejj79.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20151227020810.1450-1-normalperson@yhbt.net> ``` Eric Wong writes: > - # commas delimit multiple addresses > - $aliases{$alias} = [ split_addrs($addr) ]; > + # commas delimit multiple addresses > + my @addr = split_addrs($addr); > + > + # quotes may be escaped in the file, > + # remove them if paired so we do not > + # double-escape them later. > + s/^\\"(.*)\\"/"$1"/g foreach @addr; > + $aliases{$alias} = \@addr Can one address have two or more double-quoted string pieces in it? If that is possible, (.*) above might want to become (.*?) or even ([^"]*) to make it less greedy, perhaps? > }}}, > mailrc => sub { my $fh = shift; while (<$fh>) { > if (/^alias\s+(\S+)\s+(.*)$/) { > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index 3c49536..834d91a 100755 > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' ' > test_cover_addresses "Cc" > ' > > +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' ' > + clean_fake_sendmail && > + echo "alias sbd \\\"Dot U. Sir\\\" " >.mutt && > + git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" && > + git config sendemail.aliasfiletype mutt && > + git send-email \ > + --from="Example " \ > + --to=sbd \ > + --smtp-server="$(pwd)/fake.sendmail" \ > + outdir/0001-*.patch \ > + 2>errors >out && > + grep "^!somebody@example\.org!$" commandline1 && > + grep -F "To: \"Dot U. Sir\" " out > +' > + > test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' > clean_fake_sendmail && > echo "alias sbd somebody@example.org" >.mailrc && ``` ## Eric Wong, 2015-12-29 02:49 Subject: Re: [RFC] git-send-email: do not double-escape quotes from mutt Message-ID: <20151229024930.GA12517@dcvr.yhbt.net> URL: https://gitlist.dev/e/20151229024930.GA12517%40dcvr.yhbt.net In-Reply-To: ``` Junio C Hamano wrote: > Eric Wong writes: > > > - # commas delimit multiple addresses > > - $aliases{$alias} = [ split_addrs($addr) ]; > > + # commas delimit multiple addresses > > + my @addr = split_addrs($addr); > > + > > + # quotes may be escaped in the file, > > + # remove them if paired so we do not > > + # double-escape them later. > > + s/^\\"(.*)\\"/"$1"/g foreach @addr; > > + $aliases{$alias} = \@addr > > Can one address have two or more double-quoted string pieces in it? > If that is possible, (.*) above might want to become (.*?) or even > ([^"]*) to make it less greedy, perhaps? Yes. Apparently it's possible to have a double-quote inside the name, too. mutt understands both of the following: alias qn \"Q. N\\\"ame\" # becomes "Q. N\"ame" alias dq \"Dub O.\" \"Q\" # becomes "Dub O. Q" The "qn" case can be taken care of using a simpler replacement on top my original RFC: --- a/git-send-email.perl +++ b/git-send-email.perl @@ -530,7 +530,7 @@ my %parse_alias = ( # quotes may be escaped in the file, # remove them if paired so we do not # double-escape them later. - s/^\\"(.*?)\\"/"$1"/g foreach @addr; + s/\\"/"/g foreach @addr; $aliases{$alias} = \@addr }}}, mailrc => sub { my $fh = shift; while (<$fh>) { But I'm not sure how to handle the "dq" case or if that even happens in practice, as attempting to save an alias with "Dub O." "Q" in the From: header, mutt shortens it to the expected \"Dub O. Q\" without extra quotes. Saving "qn" round trips, so the \\\" in the middle is preserved. ``` ## Matthieu Moy, 2016-01-04 18:11 Subject: Re: [RFC] git-send-email: do not double-escape quotes from mutt Message-ID: URL: https://gitlist.dev/e/vpqsi2dxlht.fsf%40anie.imag.fr In-Reply-To: <20151227020810.1450-1-normalperson@yhbt.net> ``` Eric Wong writes: > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -524,8 +524,14 @@ my %parse_alias = ( > if (/^\s*alias\s+(?:-group\s+\S+\s+)*(\S+)\s+(.*)$/) { > my ($alias, $addr) = ($1, $2); > $addr =~ s/#.*$//; # mutt allows # comments > - # commas delimit multiple addresses > - $aliases{$alias} = [ split_addrs($addr) ]; > + # commas delimit multiple addresses > + my @addr = split_addrs($addr); > + > + # quotes may be escaped in the file, > + # remove them if paired so we do not > + # double-escape them later. I think you meant "remove the escaping" or simply "unescape", not "remove them", which I'd understand as "remove the quotes". Other than that, the patch looks good to me, including your proposed fixup: --- a/git-send-email.perl +++ b/git-send-email.perl @@ -530,7 +530,7 @@ my %parse_alias = ( # quotes may be escaped in the file, # remove them if paired so we do not # double-escape them later. - s/^\\"(.*?)\\"/"$1"/g foreach @addr; + s/\\"/"/g foreach @addr; $aliases{$alias} = \@addr Eric Wong writes: > alias qn \"Q. N\\\"ame\" # becomes "Q. N\"ame" > alias dq \"Dub O.\" \"Q\" # becomes "Dub O. Q" [...] > But I'm not sure how to handle the "dq" case or if that even > happens in practice, as attempting to save an alias with "Dub O." "Q" > in the From: header, mutt shortens it to the expected \"Dub O. Q\" > without extra quotes. I wouldn't worry too much about corner-cases, but perhaps some people do use escaped quotes inside escaped quotes. I'd say they get what they deserve ;-). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Eric Wong, 2016-01-04 20:53 Subject: [PATCH v2] git-send-email: do not double-escape quotes from mutt Message-ID: <20160104205330.GA6658@dcvr.yhbt.net> URL: https://gitlist.dev/e/20160104205330.GA6658%40dcvr.yhbt.net In-Reply-To: ``` mutt saves aliases with escaped quotes in the form of: alias dot \"Dot U. Sir\" When we pass through our sanitize_address routine, we end up with double-escaping: To: "\\\"Dot U. Sir\\\" Remove the escaping in mutt only for now, as I am not sure if other mailers can do this or if this is better fixed in sanitize_address. Cc: Remi Lespinet Cc: Matthieu Moy --- Matthieu Moy wrote: > I think you meant "remove the escaping" or simply "unescape", not > "remove them", which I'd understand as "remove the quotes". > > Other than that, the patch looks good to me, including your proposed > fixup: Thanks, updated the comment and squashed my fixup > I wouldn't worry too much about corner-cases, but perhaps some people do > use escaped quotes inside escaped quotes. I'd say they get what they > deserve ;-). Agreed :) git-send-email.perl | 9 +++++++-- t/t9001-send-email.sh | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 6caa5b5..d356901 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -524,8 +524,13 @@ my %parse_alias = ( if (/^\s*alias\s+(?:-group\s+\S+\s+)*(\S+)\s+(.*)$/) { my ($alias, $addr) = ($1, $2); $addr =~ s/#.*$//; # mutt allows # comments - # commas delimit multiple addresses - $aliases{$alias} = [ split_addrs($addr) ]; + # commas delimit multiple addresses + my @addr = split_addrs($addr); + + # quotes may be escaped in the file, + # unescape them so we do not double-escape them later. + s/\\"/"/g foreach @addr; + $aliases{$alias} = \@addr }}}, mailrc => sub { my $fh = shift; while (<$fh>) { if (/^alias\s+(\S+)\s+(.*)$/) { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 3c49536..834d91a 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' ' test_cover_addresses "Cc" ' +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' ' + clean_fake_sendmail && + echo "alias sbd \\\"Dot U. Sir\\\" " >.mutt && + git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" && + git config sendemail.aliasfiletype mutt && + git send-email \ + --from="Example " \ + --to=sbd \ + --smtp-server="$(pwd)/fake.sendmail" \ + outdir/0001-*.patch \ + 2>errors >out && + grep "^!somebody@example\.org!$" commandline1 && + grep -F "To: \"Dot U. Sir\" " out +' + test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' clean_fake_sendmail && echo "alias sbd somebody@example.org" >.mailrc && -- EW ``` ## Junio C Hamano, 2016-01-04 21:35 Subject: Re: [PATCH v2] git-send-email: do not double-escape quotes from mutt Message-ID: URL: https://gitlist.dev/e/xmqqy4c5m3iv.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20160104205330.GA6658@dcvr.yhbt.net> ``` Eric Wong writes: > mutt saves aliases with escaped quotes in the form of: > > alias dot \"Dot U. Sir\" > > When we pass through our sanitize_address routine, > we end up with double-escaping: > > To: "\\\"Dot U. Sir\\\" > > Remove the escaping in mutt only for now, as I am not sure > if other mailers can do this or if this is better fixed in > sanitize_address. > > Cc: Remi Lespinet > Cc: Matthieu Moy > --- Forgot to sign-off (I could forge it, though, but anyway)? > Matthieu Moy wrote: > > I think you meant "remove the escaping" or simply "unescape", not > > "remove them", which I'd understand as "remove the quotes". > > > > Other than that, the patch looks good to me, including your proposed > > fixup: > > Thanks, updated the comment and squashed my fixup > > > I wouldn't worry too much about corner-cases, but perhaps some people do > > use escaped quotes inside escaped quotes. I'd say they get what they > > deserve ;-). > > Agreed :) > > git-send-email.perl | 9 +++++++-- > t/t9001-send-email.sh | 15 +++++++++++++++ > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index 6caa5b5..d356901 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -524,8 +524,13 @@ my %parse_alias = ( > if (/^\s*alias\s+(?:-group\s+\S+\s+)*(\S+)\s+(.*)$/) { > my ($alias, $addr) = ($1, $2); > $addr =~ s/#.*$//; # mutt allows # comments > - # commas delimit multiple addresses > - $aliases{$alias} = [ split_addrs($addr) ]; > + # commas delimit multiple addresses > + my @addr = split_addrs($addr); > + > + # quotes may be escaped in the file, > + # unescape them so we do not double-escape them later. > + s/\\"/"/g foreach @addr; > + $aliases{$alias} = \@addr > }}}, > mailrc => sub { my $fh = shift; while (<$fh>) { > if (/^alias\s+(\S+)\s+(.*)$/) { > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index 3c49536..834d91a 100755 > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' ' > test_cover_addresses "Cc" > ' > > +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' ' > + clean_fake_sendmail && > + echo "alias sbd \\\"Dot U. Sir\\\" " >.mutt && > + git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" && > + git config sendemail.aliasfiletype mutt && > + git send-email \ > + --from="Example " \ > + --to=sbd \ > + --smtp-server="$(pwd)/fake.sendmail" \ > + outdir/0001-*.patch \ > + 2>errors >out && > + grep "^!somebody@example\.org!$" commandline1 && > + grep -F "To: \"Dot U. Sir\" " out > +' > + > test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' > clean_fake_sendmail && > echo "alias sbd somebody@example.org" >.mailrc && ``` ## Eric Wong, 2016-01-04 22:01 Subject: Re: [PATCH v2] git-send-email: do not double-escape quotes from mutt Message-ID: <20160104220151.GA27478@dcvr.yhbt.net> URL: https://gitlist.dev/e/20160104220151.GA27478%40dcvr.yhbt.net In-Reply-To: ``` Junio C Hamano wrote: > Forgot to sign-off (I could forge it, though, but anyway)? Oops :x Signed-off-by: Eric Wong ```