[RFC] git-send-email: do not double-escape quotes from mutt

[RFC] git-send-email: do not double-escape quotes from mutt

[Eric Wong]

mutt saves aliases with escaped quotes in the form of:

	alias dot \"Dot U. Sir\" <somebody@example.org>

When we pass through our sanitize_address routine,
we end up with double-escaping:

	 To: "\\\"Dot U. Sir\\\" <somebody@example.org>

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 <remi.lespinet@ensimag.grenoble-inp.fr>
Cc: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 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\\\" <somebody@example.org>" >.mutt &&
+	git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" &&
+	git config sendemail.aliasfiletype mutt &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=sbd \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		outdir/0001-*.patch \
+		2>errors >out &&
+	grep "^!somebody@example\.org!$" commandline1 &&
+	grep -F "To: \"Dot U. Sir\" <somebody@example.org>" out
+'
+
 test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
 	clean_fake_sendmail &&
 	echo "alias sbd  somebody@example.org" >.mailrc &&
-- 
EW

Re: [RFC] git-send-email: do not double-escape quotes from mutt

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> 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\\\" <somebody@example.org>" >.mutt &&
> +	git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" &&
> +	git config sendemail.aliasfiletype mutt &&
> +	git send-email \
> +		--from="Example <nobody@example.com>" \
> +		--to=sbd \
> +		--smtp-server="$(pwd)/fake.sendmail" \
> +		outdir/0001-*.patch \
> +		2>errors >out &&
> +	grep "^!somebody@example\.org!$" commandline1 &&
> +	grep -F "To: \"Dot U. Sir\" <somebody@example.org>" out
> +'
> +
>  test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
>  	clean_fake_sendmail &&
>  	echo "alias sbd  somebody@example.org" >.mailrc &&

Re: [RFC] git-send-email: do not double-escape quotes from mutt

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <normalperson@yhbt.net> 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\" <qn@example.org>   # becomes "Q. N\"ame"
alias dq \"Dub O.\" \"Q\" <dq@example.org>  # 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.

Re: [RFC] git-send-email: do not double-escape quotes from mutt

[Matthieu Moy]

Eric Wong <normalperson@yhbt.net> 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 <normalperson@yhbt.net> writes:

> alias qn \"Q. N\\\"ame\" <qn@example.org>   # becomes "Q. N\"ame"
> alias dq \"Dub O.\" \"Q\" <dq@example.org>  # 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/

[PATCH v2] git-send-email: do not double-escape quotes from mutt

[Eric Wong]

mutt saves aliases with escaped quotes in the form of:

	alias dot \"Dot U. Sir\" <somebody@example.org>

When we pass through our sanitize_address routine,
we end up with double-escaping:

	 To: "\\\"Dot U. Sir\\\" <somebody@example.org>

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 <remi.lespinet@ensimag.grenoble-inp.fr>
Cc: Matthieu Moy <Matthieu.Moy@imag.fr>
---
  Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> 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\\\" <somebody@example.org>" >.mutt &&
+	git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" &&
+	git config sendemail.aliasfiletype mutt &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=sbd \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		outdir/0001-*.patch \
+		2>errors >out &&
+	grep "^!somebody@example\.org!$" commandline1 &&
+	grep -F "To: \"Dot U. Sir\" <somebody@example.org>" out
+'
+
 test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
 	clean_fake_sendmail &&
 	echo "alias sbd  somebody@example.org" >.mailrc &&
-- 
EW

Re: [PATCH v2] git-send-email: do not double-escape quotes from mutt

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> mutt saves aliases with escaped quotes in the form of:
>
> 	alias dot \"Dot U. Sir\" <somebody@example.org>
>
> When we pass through our sanitize_address routine,
> we end up with double-escaping:
>
> 	 To: "\\\"Dot U. Sir\\\" <somebody@example.org>
>
> 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 <remi.lespinet@ensimag.grenoble-inp.fr>
> Cc: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

Forgot to sign-off (I could forge it, though, but anyway)?

>   Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> 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\\\" <somebody@example.org>" >.mutt &&
> +	git config --replace-all sendemail.aliasesfile "$(pwd)/.mutt" &&
> +	git config sendemail.aliasfiletype mutt &&
> +	git send-email \
> +		--from="Example <nobody@example.com>" \
> +		--to=sbd \
> +		--smtp-server="$(pwd)/fake.sendmail" \
> +		outdir/0001-*.patch \
> +		2>errors >out &&
> +	grep "^!somebody@example\.org!$" commandline1 &&
> +	grep -F "To: \"Dot U. Sir\" <somebody@example.org>" out
> +'
> +
>  test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
>  	clean_fake_sendmail &&
>  	echo "alias sbd  somebody@example.org" >.mailrc &&

Re: [PATCH v2] git-send-email: do not double-escape quotes from mutt

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Forgot to sign-off (I could forge it, though, but anyway)?

Oops :x

Signed-off-by: Eric Wong <normalperson@yhbt.net>