{"thread":{"id":"10882","subject":"[BUG] encoding problem with format-patch + send-email","startedAt":"2007-11-15T10:57:26Z","lastAt":"2007-11-21T06:58:38Z","messageCount":9,"participants":["Uwe Kleine-König","Jeff King","Junio C Hamano","Brian Swetland"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"59968","messageId":"20071115105726.GA18555@bre-cln-ukleine.digi.com","threadId":"10882","inReplyTo":null,"subject":"[BUG] encoding problem with format-patch + send-email","fromName":"Uwe Kleine-König","fromEmail":"uwe.kleine-koenig@digi.com","sentAt":"2007-11-15T10:57:26Z","receivedAt":"2007-11-15T10:57:26Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nBrian just stumbled over a problem with format-patch + send-email.\n\nformat-patch only adds Content-Type and Content-Transfer-Encoding\nheaders iff the body needs it.\n\nsend-email adds \"From: A. U. Thor \" to the body if sender\nand From: in the patch to send differ.\n\nBoth is just fine, but if the author has some non-ascii characters in\nher name but the body is ascii-only the resulting mail is broken.\n\nWhat about adding the Content-Type and Content-Transfer-Encoding headers\nin any case?\n\nBest regards\nUwe\n\n-- \nUwe Kleine-König, Software Engineer\nDigi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany\nTax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962\n"},{"id":"60077","messageId":"20071116104907.GA13087@sigill.intra.peff.net","threadId":"10882","inReplyTo":"20071115105726.GA18555@bre-cln-ukleine.digi.com","subject":"Re: [BUG] encoding problem with format-patch + send-email","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-16T10:49:09Z","receivedAt":"2007-11-16T10:49:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 15, 2007 at 11:57:26AM +0100, Uwe Kleine-König wrote:\n\n> send-email adds \"From: A. U. Thor \" to the body if sender\n> and From: in the patch to send differ.\n> \n> Both is just fine, but if the author has some non-ascii characters in\n> her name but the body is ascii-only the resulting mail is broken.\n\nI posted an untested fix to this and discussed the issue in\n\n http://article.gmane.org/gmane.comp.version-control.git/64426\n http://article.gmane.org/gmane.comp.version-control.git/64436\n\nbut nobody seems to have been interested after that (I don't even use\ngit-send-email myself).\n\nBelow is an updated patch (there was a typo in one of the regexes in the\noriginal) that meets my limited testing for the all-utf8 case (I don't\nknow how people actually use alternate encodings with git, if at all, so\nI don't know that I can put together a good test case). My test case was\nsomething like:\n\n git-clone git test && cd test\n echo junk >>Makefile\n git-commit -m junk --author 'Uwe Kleine-König ' -a\n git-format-patch HEAD^\n git-send-email 0001-junk.patch\n\n> What about adding the Content-Type and Content-Transfer-Encoding headers\n> in any case?\n\nYou could probably add them unconditionally, but it would be nice to\nhave them match the encoding, so you'd still want to pick them out of\nthe rfc2047 encoding in the from header.\n\n-Peff\n\n-- >8 --\ngit-send-email: add charset header if we add encoded 'From'\n\nWe sometimes pick out the original rfc822 'From' header and\ninclude it in the body of the message. If the original\nauthor's name needs encoding, then we should specify that in\nthe content-type header.\n\nIf we already had a content-type header in the mail, then we\nmay need to re-encode. The logic is there to detect\nthis case, but it doesn't actually do the re-encoding.\n\nSigned-off-by: Jeff King \n---\n git-send-email.perl | 34 +++++++++++++++++++++++++++++++---\n 1 files changed, 31 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f9bd2e5..fd0a4ad 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -514,11 +514,13 @@ $time = time - scalar $#files;\n \n sub unquote_rfc2047 {\n \tlocal ($_) = @_;\n-\tif (s/=\\?utf-8\\?q\\?(.*)\\?=/$1/g) {\n+\tmy $encoding;\n+\tif (s/=\\?([^?]+)\\?q\\?(.*)\\?=/$2/g) {\n+\t\t$encoding = $1;\n \t\ts/_/ /g;\n \t\ts/=([0-9A-F]{2})/chr(hex($1))/eg;\n \t}\n-\treturn \"$_\";\n+\treturn wantarray ? ($_, $encoding) : $_;\n }\n \n # use the simplest quoting being able to handle the recipient\n@@ -667,6 +669,9 @@ foreach my $t (@files) {\n \topen(F,\"<\",$t) or die \"can't open file $t\";\n \n \tmy $author = undef;\n+\tmy $author_encoding;\n+\tmy $has_content_type;\n+\tmy $body_encoding;\n \t@cc = @initial_cc;\n \t@xh = ();\n \tmy $input_format = undef;\n@@ -692,12 +697,20 @@ foreach my $t (@files) {\n \t\t\t\t\t\tnext if ($suppress_from);\n \t\t\t\t\t}\n \t\t\t\t\telsif ($1 eq 'From') {\n-\t\t\t\t\t\t$author = unquote_rfc2047($2);\n+\t\t\t\t\t\t($author, $author_encoding)\n+\t\t\t\t\t\t = unquote_rfc2047($2);\n \t\t\t\t\t}\n \t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t\t$2, $_) unless $quiet;\n \t\t\t\t\tpush @cc, $2;\n \t\t\t\t}\n+\t\t\t\telsif (/^Content-type:/i) {\n+\t\t\t\t\t$has_content_type = 1;\n+\t\t\t\t\tif (/charset=\"?[^ \"]+/) {\n+\t\t\t\t\t\t$body_encoding = $1;\n+\t\t\t\t\t}\n+\t\t\t\t\tpush @xh, $_;\n+\t\t\t\t}\n \t\t\t\telsif (!/^Date:\\s/ && /^[-A-Za-z]+:\\s+\\S/) {\n \t\t\t\t\tpush @xh, $_;\n \t\t\t\t}\n@@ -756,6 +769,21 @@ foreach my $t (@files) {\n \n \tif (defined $author) {\n \t\t$message = \"From: $author\\n\\n$message\";\n+\t\tif (defined $author_encoding) {\n+\t\t\tif ($has_content_type) {\n+\t\t\t\tif ($body_encoding eq $author_encoding) {\n+\t\t\t\t\t# ok, we already have the right encoding\n+\t\t\t\t}\n+\t\t\t\telse {\n+\t\t\t\t\t# uh oh, we should re-encode\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tpush @xh,\n+\t\t\t\t 'MIME-Version: 1.0',\n+\t\t\t\t \"Content-Type: text/plain; charset=$author_encoding\";\n+\t\t\t}\n+\t\t}\n \t}\n \n \tsend_message();\n-- \n1.5.3.1.47.g88b7d-dirty\n"},{"id":"60079","messageId":"20071116111450.GA22783@bre-cln-ukleine.digi.com","threadId":"10882","inReplyTo":"20071116104907.GA13087@sigill.intra.peff.net","subject":"Re: [BUG] encoding problem with format-patch + send-email","fromName":"Uwe Kleine-König","fromEmail":"uwe.kleine-koenig@digi.com","sentAt":"2007-11-16T11:14:50Z","receivedAt":"2007-11-16T11:14:50Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Jeff,\n\n> sub unquote_rfc2047 {\n> \tlocal ($_) = @_;\n> -\tif (s/=\\?utf-8\\?q\\?(.*)\\?=/$1/g) {\n> +\tmy $encoding;\n> +\tif (s/=\\?([^?]+)\\?q\\?(.*)\\?=/$2/g) {\n> +\t\t$encoding = $1;\n> \t\ts/_/ /g;\n> \t\ts/=([0-9A-F]{2})/chr(hex($1))/eg;\n> \t}\n> -\treturn \"$_\";\n> +\treturn wantarray ? ($_, $encoding) : $_;\n> }\nI don't know perl very well, but that wantarray seems hacky. (Something\nin my head wants to have it always return ($_, $encoding) and fix all\ncallers. :-)\n\n> [...]\n> @@ -756,6 +769,21 @@ foreach my $t (@files) {\n> \n> \tif (defined $author) {\n> \t\t$message = \"From: $author\\n\\n$message\";\n> +\t\tif (defined $author_encoding) {\n> +\t\t\tif ($has_content_type) {\n> +\t\t\t\tif ($body_encoding eq $author_encoding) {\n> +\t\t\t\t\t# ok, we already have the right encoding\n> +\t\t\t\t}\n> +\t\t\t\telse {\n> +\t\t\t\t\t# uh oh, we should re-encode\nIMHO we should bail here or do the recoding (and bail if that fails).\nOTH this patch improves send-emails behaviour because currently it\ndoesn't bother at all and with this patch it could at least fix the\ncommon cases.\n\nSo\n\n\tAcked-by: Uwe Kleine-König \n\nbut note I only read the code, I didn't run it.\n\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\telse {\n> +\t\t\t\tpush @xh,\n> +\t\t\t\t 'MIME-Version: 1.0',\n> +\t\t\t\t \"Content-Type: text/plain; charset=$author_encoding\";\n> +\t\t\t}\n> +\t\t}\n> \t}\n\nBest regards\nUwe\n\n-- \nUwe Kleine-König, Software Engineer\nDigi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany\nTax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962\n"},{"id":"60122","messageId":"7vlk8xwvbu.fsf@gitster.siamese.dyndns.org","threadId":"10882","inReplyTo":"20071116104907.GA13087@sigill.intra.peff.net","subject":"Re: [BUG] encoding problem with format-patch + send-email","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-17T00:48:37Z","receivedAt":"2007-11-17T00:48:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> git-send-email: add charset header if we add encoded 'From'\n\nThanks.\n\n> If we already had a content-type header in the mail, then we\n> may need to re-encode. The logic is there to detect\n> this case, but it doesn't actually do the re-encoding.\n\nAlthough the charset on rfc2047 encoded header fields can be\nindependent of the charset in the body, I think you need to do\ncrazy things to do so.\n\nWill queue.\n"},{"id":"60285","messageId":"20071119104950.GB8645@informatik.uni-freiburg.de","threadId":"10882","inReplyTo":"7vlk8xwvbu.fsf@gitster.siamese.dyndns.org","subject":"Re: [BUG] encoding problem with format-patch + send-email","fromName":"Uwe Kleine-König","fromEmail":"ukleinek@informatik.uni-freiburg.de","sentAt":"2007-11-19T10:49:50Z","receivedAt":"2007-11-19T10:49:50Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Jeff\n\nBrian sent another mail to the linux-arm-kernel mailing list, now\nspotting:\n\n\tContent-Type: text/plain; charset=UTF-8\n\nbut no Content-Transfer-Encoding:. This yield a 7bit mail with 8bit\ncharacters.\n\nI think we should add\n\n\tContent-Transfer-Encoding: 8bit\n\n, too.\n\nBest regards\nUwe\n\n-- \nUwe Kleine-König\n\n$ dc << EOF\n[d1-d1]\n> Hello Jeff\n> \n> Brian sent another mail to the linux-arm-kernel mailing list, now\n> spotting:\n> \n> \tContent-Type: text/plain; charset=UTF-8\n> \n> but no Content-Transfer-Encoding:. This yield a 7bit mail with 8bit\n> characters.\n\nI actually tacked the Content-Type on by hand on that one.\n\nI haven't had a chance to try the updated send-email, so it may well\ndo the right thing.\n\nBrian\n"},{"id":"60407","messageId":"20071120125404.GB7998@sigill.intra.peff.net","threadId":"10882","inReplyTo":"20071119104950.GB8645@informatik.uni-freiburg.de","subject":"[PATCH] send-email: add transfer encoding header with content-type","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-20T12:54:04Z","receivedAt":"2007-11-20T12:54:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We add the content-type header only when we have non-7bit\ncharacters from the 'From' header, so we really need to\nspecify the encoding (in other cases, where the commit text\nneeded a content-type, git-format-patch will already have\nadded the encoding header).\n\nSigned-off-by: Jeff King \n---\nOn Mon, Nov 19, 2007 at 11:49:50AM +0100, Uwe Kleine-König wrote:\n\n> but no Content-Transfer-Encoding:. This yield a 7bit mail with 8bit\n> characters.\n> \n> I think we should add\n> \n> \tContent-Transfer-Encoding: 8bit\n\nEven though Brian's mail turned out to be hand-generated, this problem\ndoes exist in git-send-email. I don't know why I didn't add the encoding\nheader in the first place, since it is clearly required.\n\nJunio, I think this is maint-worthy.\n\n git-send-email.perl | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex fd0a4ad..d7b8391 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -781,7 +781,8 @@ foreach my $t (@files) {\n \t\t\telse {\n \t\t\t\tpush @xh,\n \t\t\t\t 'MIME-Version: 1.0',\n-\t\t\t\t \"Content-Type: text/plain; charset=$author_encoding\";\n+\t\t\t\t \"Content-Type: text/plain; charset=$author_encoding\",\n+\t\t\t\t 'Content-Transfer-Encoding: 8bit';\n \t\t\t}\n \t\t}\n \t}\n-- \n1.5.3.6.1784.gd1b1d-dirty\n"},{"id":"60409","messageId":"20071120134934.GA546@informatik.uni-freiburg.de","threadId":"10882","inReplyTo":"20071120125404.GB7998@sigill.intra.peff.net","subject":"Re: [PATCH] send-email: add transfer encoding header with content-type","fromName":"Uwe Kleine-König","fromEmail":"ukleinek@informatik.uni-freiburg.de","sentAt":"2007-11-20T13:49:34Z","receivedAt":"2007-11-20T13:49:34Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nJeff King wrote:\n> We add the content-type header only when we have non-7bit\n> characters from the 'From' header, so we really need to\n> specify the encoding (in other cases, where the commit text\n> needed a content-type, git-format-patch will already have\n> added the encoding header).\n> \n> Signed-off-by: Jeff King \nAcked-by: Uwe Kleine-König \n\n-- \nUwe Kleine-König\n\n$ dc << EOF\n[d1-d1 writes:\n\n>> I think we should add\n>> \n>> \tContent-Transfer-Encoding: 8bit\n>\n> Even though Brian's mail turned out to be hand-generated, this problem\n> does exist in git-send-email. I don't know why I didn't add the encoding\n> header in the first place, since it is clearly required.\n>\n> Junio, I think this is maint-worthy.\n\nYeah, looks good.\n"}]}