{"thread":{"id":"41081","subject":"[RFC] git-send-email: do not double-escape quotes from mutt","startedAt":"2015-12-27T02:08:10Z","lastAt":"2016-01-04T22:01:51Z","messageCount":7,"participants":["Eric Wong","Junio C Hamano","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"275016","messageId":"20151227020810.1450-1-normalperson@yhbt.net","threadId":"41081","inReplyTo":null,"subject":"[RFC] git-send-email: do not double-escape quotes from mutt","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2015-12-27T02:08:10Z","receivedAt":"2015-12-27T02:08:10Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"mutt saves aliases with escaped quotes in the form of:\n\n\talias dot \\\"Dot U. Sir\\\" \n\nWhen we pass through our sanitize_address routine,\nwe end up with double-escaping:\n\n\t To: \"\\\\\\\"Dot U. Sir\\\\\\\" \n\nRemove the escaping in mutt only for now, as I am not sure\nif other mailers can do this or if this is better fixed in\nsanitize_address.\n\nCc: Remi Lespinet \nCc: Matthieu Moy \n---\n Cc-ing Remi and Matthieu since they might know better\n about fixing this in sanitize_address instead or if it's\n better being a one-off for mutt.\n\n git-send-email.perl | 10 ++++++++--\n t/t9001-send-email.sh | 15 +++++++++++++++\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex c2abdaa..2d3f51e 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -524,8 +524,14 @@ my %parse_alias = (\n \t\tif (/^\\s*alias\\s+(?:-group\\s+\\S+\\s+)*(\\S+)\\s+(.*)$/) {\n \t\t\tmy ($alias, $addr) = ($1, $2);\n \t\t\t$addr =~ s/#.*$//; # mutt allows # comments\n-\t\t\t # commas delimit multiple addresses\n-\t\t\t$aliases{$alias} = [ split_addrs($addr) ];\n+\t\t\t# commas delimit multiple addresses\n+\t\t\tmy @addr = split_addrs($addr);\n+\n+\t\t\t# quotes may be escaped in the file,\n+\t\t\t# remove them if paired so we do not\n+\t\t\t# double-escape them later.\n+\t\t\ts/^\\\\\"(.*)\\\\\"/\"$1\"/g foreach @addr;\n+\t\t\t$aliases{$alias} = \\@addr\n \t\t}}},\n \tmailrc => sub { my $fh = shift; while (<$fh>) {\n \t\tif (/^alias\\s+(\\S+)\\s+(.*)$/) {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3c49536..834d91a 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' '\n \ttest_cover_addresses \"Cc\"\n '\n \n+test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' '\n+\tclean_fake_sendmail &&\n+\techo \"alias sbd \\\\\\\"Dot U. Sir\\\\\\\" \" >.mutt &&\n+\tgit config --replace-all sendemail.aliasesfile \"$(pwd)/.mutt\" &&\n+\tgit config sendemail.aliasfiletype mutt &&\n+\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=sbd \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\toutdir/0001-*.patch \\\n+\t\t2>errors >out &&\n+\tgrep \"^!somebody@example\\.org!$\" commandline1 &&\n+\tgrep -F \"To: \\\"Dot U. Sir\\\" \" out\n+'\n+\n test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '\n \tclean_fake_sendmail &&\n \techo \"alias sbd somebody@example.org\" >.mailrc &&\n-- \nEW\n"},{"id":"275058","messageId":"xmqqlh8ejj79.fsf@gitster.mtv.corp.google.com","threadId":"41081","inReplyTo":"20151227020810.1450-1-normalperson@yhbt.net","subject":"Re: [RFC] git-send-email: do not double-escape quotes from mutt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-28T22:34:50Z","receivedAt":"2015-12-28T22:34:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> -\t\t\t # commas delimit multiple addresses\n> -\t\t\t$aliases{$alias} = [ split_addrs($addr) ];\n> +\t\t\t# commas delimit multiple addresses\n> +\t\t\tmy @addr = split_addrs($addr);\n> +\n> +\t\t\t# quotes may be escaped in the file,\n> +\t\t\t# remove them if paired so we do not\n> +\t\t\t# double-escape them later.\n> +\t\t\ts/^\\\\\"(.*)\\\\\"/\"$1\"/g foreach @addr;\n> +\t\t\t$aliases{$alias} = \\@addr\n\nCan one address have two or more double-quoted string pieces in it?\nIf that is possible, (.*) above might want to become (.*?) or even\n([^\"]*) to make it less greedy, perhaps?\n\n> \t\t}}},\n> \tmailrc => sub { my $fh = shift; while (<$fh>) {\n> \t\tif (/^alias\\s+(\\S+)\\s+(.*)$/) {\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 3c49536..834d91a 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' '\n> \ttest_cover_addresses \"Cc\"\n> '\n> \n> +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' '\n> +\tclean_fake_sendmail &&\n> +\techo \"alias sbd \\\\\\\"Dot U. Sir\\\\\\\" \" >.mutt &&\n> +\tgit config --replace-all sendemail.aliasesfile \"$(pwd)/.mutt\" &&\n> +\tgit config sendemail.aliasfiletype mutt &&\n> +\tgit send-email \\\n> +\t\t--from=\"Example \" \\\n> +\t\t--to=sbd \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\toutdir/0001-*.patch \\\n> +\t\t2>errors >out &&\n> +\tgrep \"^!somebody@example\\.org!$\" commandline1 &&\n> +\tgrep -F \"To: \\\"Dot U. Sir\\\" \" out\n> +'\n> +\n> test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '\n> \tclean_fake_sendmail &&\n> \techo \"alias sbd somebody@example.org\" >.mailrc &&\n"},{"id":"275065","messageId":"20151229024930.GA12517@dcvr.yhbt.net","threadId":"41081","inReplyTo":"xmqqlh8ejj79.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] git-send-email: do not double-escape quotes from mutt","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2015-12-29T02:49:30Z","receivedAt":"2015-12-29T02:49:30Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> > -\t\t\t # commas delimit multiple addresses\n> > -\t\t\t$aliases{$alias} = [ split_addrs($addr) ];\n> > +\t\t\t# commas delimit multiple addresses\n> > +\t\t\tmy @addr = split_addrs($addr);\n> > +\n> > +\t\t\t# quotes may be escaped in the file,\n> > +\t\t\t# remove them if paired so we do not\n> > +\t\t\t# double-escape them later.\n> > +\t\t\ts/^\\\\\"(.*)\\\\\"/\"$1\"/g foreach @addr;\n> > +\t\t\t$aliases{$alias} = \\@addr\n> \n> Can one address have two or more double-quoted string pieces in it?\n> If that is possible, (.*) above might want to become (.*?) or even\n> ([^\"]*) to make it less greedy, perhaps?\n\nYes. Apparently it's possible to have a double-quote inside the name,\ntoo. mutt understands both of the following:\n\nalias qn \\\"Q. N\\\\\\\"ame\\\" # becomes \"Q. N\\\"ame\"\nalias dq \\\"Dub O.\\\" \\\"Q\\\" # becomes \"Dub O. Q\"\n\nThe \"qn\" case can be taken care of using a simpler replacement\non top my original RFC:\n\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -530,7 +530,7 @@ my %parse_alias = (\n \t\t\t# quotes may be escaped in the file,\n \t\t\t# remove them if paired so we do not\n \t\t\t# double-escape them later.\n-\t\t\ts/^\\\\\"(.*?)\\\\\"/\"$1\"/g foreach @addr;\n+\t\t\ts/\\\\\"/\"/g foreach @addr;\n \t\t\t$aliases{$alias} = \\@addr\n \t\t}}},\n \tmailrc => sub { my $fh = shift; while (<$fh>) {\n\nBut I'm not sure how to handle the \"dq\" case or if that even\nhappens in practice, as attempting to save an alias with \"Dub O.\" \"Q\"\nin the From: header, mutt shortens it to the expected \\\"Dub O. Q\\\"\nwithout extra quotes.\n\nSaving \"qn\" round trips, so the \\\\\\\" in the middle is preserved.\n"},{"id":"275312","messageId":"vpqsi2dxlht.fsf@anie.imag.fr","threadId":"41081","inReplyTo":"20151227020810.1450-1-normalperson@yhbt.net","subject":"Re: [RFC] git-send-email: do not double-escape quotes from mutt","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-01-04T18:11:58Z","receivedAt":"2016-01-04T18:11:58Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Eric Wong writes:\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -524,8 +524,14 @@ my %parse_alias = (\n> \t\tif (/^\\s*alias\\s+(?:-group\\s+\\S+\\s+)*(\\S+)\\s+(.*)$/) {\n> \t\t\tmy ($alias, $addr) = ($1, $2);\n> \t\t\t$addr =~ s/#.*$//; # mutt allows # comments\n> -\t\t\t # commas delimit multiple addresses\n> -\t\t\t$aliases{$alias} = [ split_addrs($addr) ];\n> +\t\t\t# commas delimit multiple addresses\n> +\t\t\tmy @addr = split_addrs($addr);\n> +\n> +\t\t\t# quotes may be escaped in the file,\n> +\t\t\t# remove them if paired so we do not\n> +\t\t\t# double-escape them later.\n\nI think you meant \"remove the escaping\" or simply \"unescape\", not\n\"remove them\", which I'd understand as \"remove the quotes\".\n\nOther than that, the patch looks good to me, including your proposed\nfixup:\n\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -530,7 +530,7 @@ my %parse_alias = (\n \t\t\t# quotes may be escaped in the file,\n \t\t\t# remove them if paired so we do not\n \t\t\t# double-escape them later.\n-\t\t\ts/^\\\\\"(.*?)\\\\\"/\"$1\"/g foreach @addr;\n+\t\t\ts/\\\\\"/\"/g foreach @addr;\n \t\t\t$aliases{$alias} = \\@addr\n\nEric Wong writes:\n\n> alias qn \\\"Q. N\\\\\\\"ame\\\" # becomes \"Q. N\\\"ame\"\n> alias dq \\\"Dub O.\\\" \\\"Q\\\" # becomes \"Dub O. Q\"\n[...]\n> But I'm not sure how to handle the \"dq\" case or if that even\n> happens in practice, as attempting to save an alias with \"Dub O.\" \"Q\"\n> in the From: header, mutt shortens it to the expected \\\"Dub O. Q\\\"\n> without extra quotes.\n\nI wouldn't worry too much about corner-cases, but perhaps some people do\nuse escaped quotes inside escaped quotes. I'd say they get what they\ndeserve ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"275323","messageId":"20160104205330.GA6658@dcvr.yhbt.net","threadId":"41081","inReplyTo":"vpqsi2dxlht.fsf@anie.imag.fr","subject":"[PATCH v2] git-send-email: do not double-escape quotes from mutt","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2016-01-04T20:53:30Z","receivedAt":"2016-01-04T20:53:30Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"mutt saves aliases with escaped quotes in the form of:\n\n\talias dot \\\"Dot U. Sir\\\" \n\nWhen we pass through our sanitize_address routine,\nwe end up with double-escaping:\n\n\t To: \"\\\\\\\"Dot U. Sir\\\\\\\" \n\nRemove the escaping in mutt only for now, as I am not sure\nif other mailers can do this or if this is better fixed in\nsanitize_address.\n\nCc: Remi Lespinet \nCc: Matthieu Moy \n---\n Matthieu Moy wrote:\n > I think you meant \"remove the escaping\" or simply \"unescape\", not\n > \"remove them\", which I'd understand as \"remove the quotes\".\n > \n > Other than that, the patch looks good to me, including your proposed\n > fixup:\n\n Thanks, updated the comment and squashed my fixup\n\n > I wouldn't worry too much about corner-cases, but perhaps some people do\n > use escaped quotes inside escaped quotes. I'd say they get what they\n > deserve ;-).\n\n Agreed :)\n\n git-send-email.perl | 9 +++++++--\n t/t9001-send-email.sh | 15 +++++++++++++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 6caa5b5..d356901 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -524,8 +524,13 @@ my %parse_alias = (\n \t\tif (/^\\s*alias\\s+(?:-group\\s+\\S+\\s+)*(\\S+)\\s+(.*)$/) {\n \t\t\tmy ($alias, $addr) = ($1, $2);\n \t\t\t$addr =~ s/#.*$//; # mutt allows # comments\n-\t\t\t # commas delimit multiple addresses\n-\t\t\t$aliases{$alias} = [ split_addrs($addr) ];\n+\t\t\t# commas delimit multiple addresses\n+\t\t\tmy @addr = split_addrs($addr);\n+\n+\t\t\t# quotes may be escaped in the file,\n+\t\t\t# unescape them so we do not double-escape them later.\n+\t\t\ts/\\\\\"/\"/g foreach @addr;\n+\t\t\t$aliases{$alias} = \\@addr\n \t\t}}},\n \tmailrc => sub { my $fh = shift; while (<$fh>) {\n \t\tif (/^alias\\s+(\\S+)\\s+(.*)$/) {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 3c49536..834d91a 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' '\n \ttest_cover_addresses \"Cc\"\n '\n \n+test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' '\n+\tclean_fake_sendmail &&\n+\techo \"alias sbd \\\\\\\"Dot U. Sir\\\\\\\" \" >.mutt &&\n+\tgit config --replace-all sendemail.aliasesfile \"$(pwd)/.mutt\" &&\n+\tgit config sendemail.aliasfiletype mutt &&\n+\tgit send-email \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=sbd \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\toutdir/0001-*.patch \\\n+\t\t2>errors >out &&\n+\tgrep \"^!somebody@example\\.org!$\" commandline1 &&\n+\tgrep -F \"To: \\\"Dot U. Sir\\\" \" out\n+'\n+\n test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '\n \tclean_fake_sendmail &&\n \techo \"alias sbd somebody@example.org\" >.mailrc &&\n-- \nEW\n"},{"id":"275325","messageId":"xmqqy4c5m3iv.fsf@gitster.mtv.corp.google.com","threadId":"41081","inReplyTo":"20160104205330.GA6658@dcvr.yhbt.net","subject":"Re: [PATCH v2] git-send-email: do not double-escape quotes from mutt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-04T21:35:36Z","receivedAt":"2016-01-04T21:35:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> mutt saves aliases with escaped quotes in the form of:\n>\n> \talias dot \\\"Dot U. Sir\\\" \n>\n> When we pass through our sanitize_address routine,\n> we end up with double-escaping:\n>\n> \t To: \"\\\\\\\"Dot U. Sir\\\\\\\" \n>\n> Remove the escaping in mutt only for now, as I am not sure\n> if other mailers can do this or if this is better fixed in\n> sanitize_address.\n>\n> Cc: Remi Lespinet \n> Cc: Matthieu Moy \n> ---\n\nForgot to sign-off (I could forge it, though, but anyway)?\n\n> Matthieu Moy wrote:\n> > I think you meant \"remove the escaping\" or simply \"unescape\", not\n> > \"remove them\", which I'd understand as \"remove the quotes\".\n> > \n> > Other than that, the patch looks good to me, including your proposed\n> > fixup:\n>\n> Thanks, updated the comment and squashed my fixup\n>\n> > I wouldn't worry too much about corner-cases, but perhaps some people do\n> > use escaped quotes inside escaped quotes. I'd say they get what they\n> > deserve ;-).\n>\n> Agreed :)\n>\n> git-send-email.perl | 9 +++++++--\n> t/t9001-send-email.sh | 15 +++++++++++++++\n> 2 files changed, 22 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 6caa5b5..d356901 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -524,8 +524,13 @@ my %parse_alias = (\n> \t\tif (/^\\s*alias\\s+(?:-group\\s+\\S+\\s+)*(\\S+)\\s+(.*)$/) {\n> \t\t\tmy ($alias, $addr) = ($1, $2);\n> \t\t\t$addr =~ s/#.*$//; # mutt allows # comments\n> -\t\t\t # commas delimit multiple addresses\n> -\t\t\t$aliases{$alias} = [ split_addrs($addr) ];\n> +\t\t\t# commas delimit multiple addresses\n> +\t\t\tmy @addr = split_addrs($addr);\n> +\n> +\t\t\t# quotes may be escaped in the file,\n> +\t\t\t# unescape them so we do not double-escape them later.\n> +\t\t\ts/\\\\\"/\"/g foreach @addr;\n> +\t\t\t$aliases{$alias} = \\@addr\n> \t\t}}},\n> \tmailrc => sub { my $fh = shift; while (<$fh>) {\n> \t\tif (/^alias\\s+(\\S+)\\s+(.*)$/) {\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 3c49536..834d91a 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1527,6 +1527,21 @@ test_expect_success $PREREQ 'cccover adds Cc to all mail' '\n> \ttest_cover_addresses \"Cc\"\n> '\n> \n> +test_expect_success $PREREQ 'escaped quotes in sendemail.aliasfiletype=mutt' '\n> +\tclean_fake_sendmail &&\n> +\techo \"alias sbd \\\\\\\"Dot U. Sir\\\\\\\" \" >.mutt &&\n> +\tgit config --replace-all sendemail.aliasesfile \"$(pwd)/.mutt\" &&\n> +\tgit config sendemail.aliasfiletype mutt &&\n> +\tgit send-email \\\n> +\t\t--from=\"Example \" \\\n> +\t\t--to=sbd \\\n> +\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\toutdir/0001-*.patch \\\n> +\t\t2>errors >out &&\n> +\tgrep \"^!somebody@example\\.org!$\" commandline1 &&\n> +\tgrep -F \"To: \\\"Dot U. Sir\\\" \" out\n> +'\n> +\n> test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '\n> \tclean_fake_sendmail &&\n> \techo \"alias sbd somebody@example.org\" >.mailrc &&\n"},{"id":"275328","messageId":"20160104220151.GA27478@dcvr.yhbt.net","threadId":"41081","inReplyTo":"xmqqy4c5m3iv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] git-send-email: do not double-escape quotes from mutt","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2016-01-04T22:01:51Z","receivedAt":"2016-01-04T22:01:51Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Forgot to sign-off (I could forge it, though, but anyway)?\n\nOops :x\n\nSigned-off-by: Eric Wong \n"}]}