{"thread":{"id":"32726","subject":"[PATCH] send-email: Honor multi-part email messages","startedAt":"2013-01-25T15:28:53Z","lastAt":"2013-01-25T22:24:11Z","messageCount":5,"participants":["Alexey Shumkin","Krzysztof Mazur","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"207816","messageId":"cover.1359126360.git.Alex.Crezoff@gmail.com","threadId":"32726","inReplyTo":null,"subject":"[PATCH resent] send-email: Honor multi-part email messages","fromName":"Alexey Shumkin","fromEmail":"alex.crezoff@gmail.com","sentAt":"2013-01-25T15:28:53Z","receivedAt":"2013-01-25T15:28:53Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"Let's try to involve Krzysztof Mazur \nwho have met the similar problem recently\n(according to this thread http://thread.gmane.org/gmane.comp.version-control.git/208297/focus=208310)\nI recitate myself:\n>Well, as I understand \"current\" algorithm:\n>1. It assumes that file is one-part email message\n>2. Function searches non-ASCII characters in Subject header\n>3. If none then it looks non-ASCII characters at message body\n>\n>my changes are to skip looking at message body of a multi-part\n>message as it has parts with their own Content-Type headers\n>\n>The said above in details:\n>1. To set flag when we meet Content-Type: multipart/mixed header\n>2. After we processed all headers and did not found non-ASCII characters\n>in a Subject we take a look at this flag and exit with 0\n>if it is a multi-part message\n\n\n>>I think your patch is wrong. What happens when we see a Subject:\n>>line with a non-ascii on it that causes an early return of the loop\n>>before your new code has a chance to see Content-Type: header?\nThis function is used to determine \"broken\" (non-ASCII) headers (to be encode them)\nThe problem is if \"Subject\" is not broken, but message body contains non-ASCII chars,\nsubject is marked as broken and encoded again.\n\nP.S.\nTo involved: the beginning of thread is here http://thread.gmane.org/gmane.comp.version-control.git/181743\n\nAlexey Shumkin (1):\n send-email: Honor multi-part email messages\n\n git-send-email.perl | 5 ++++\n t/t9001-send-email.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 71 insertions(+)\n\n-- \n1.8.1.1.10.g9255f3f\n"},{"id":"207815","messageId":"4de442db9fd0896f78166e6038b6ea35ed5ab266.1359126360.git.Alex.Crezoff@gmail.com","threadId":"32726","inReplyTo":"cover.1359126360.git.Alex.Crezoff@gmail.com","subject":"[PATCH] send-email: Honor multi-part email messages","fromName":"Alexey Shumkin","fromEmail":"alex.crezoff@gmail.com","sentAt":"2013-01-25T15:28:54Z","receivedAt":"2013-01-25T15:28:54Z","isPatch":true,"sender":{"key":"alex.crezoff@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1183752?v=4"},"body":"\"git format-patch --attach/--inline\" generates multi-part messages.\nEvery part of such messages can contain non-ASCII characters with its own\n\"Content-Type\" and \"Content-Transfer-Encoding\" headers.\nBut git-send-mail script interprets a patch-file as one-part message\nand does not recognize multi-part messages.\nSo already quoted printable email subject may be encoded as quoted printable\nagain. Due to this bug email subject looks corrupted in email clients.\n\nSigned-off-by: Alexey Shumkin \n---\n git-send-email.perl | 5 ++++\n t/t9001-send-email.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 71 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 94c7f76..d49befe 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1499,12 +1499,17 @@ sub file_has_nonascii {\n \n sub body_or_subject_has_nonascii {\n \tmy $fn = shift;\n+\tmy $multipart = 0;\n \topen(my $fh, '<', $fn)\n \t\tor die \"unable to open $fn: $!\\n\";\n \twhile (my $line = <$fh>) {\n \t\tlast if $line =~ /^$/;\n+\t\tif ($line =~ /^Content-Type:\\s*multipart\\/mixed.*$/) {\n+\t\t\t$multipart = 1;\n+\t\t}\n \t\treturn 1 if $line =~ /^Subject.*[^[:ascii:]]/;\n \t}\n+\treturn 0 if $multipart;\n \twhile (my $line = <$fh>) {\n \t\treturn 1 if $line =~ /[^[:ascii:]]/;\n \t}\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 97d6f4c..c7ed370 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1323,4 +1323,70 @@ test_expect_success $PREREQ 'sendemail.aliasfile=~/.mailrc' '\n \tgrep \"^!someone@example\\.org!$\" commandline1\n '\n \n+test_expect_success $PREREQ 'setup multi-part message' '\n+cat >multi-part-email-using-8bit <\n+From: author@example.com\n+Date: Sat, 12 Jun 2010 15:53:58 +0200\n+Subject: [PATCH] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=20?=\n+ =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB?=\n+MIME-Version: 1.0\n+Content-Type: multipart/mixed; boundary=\"------------123\"\n+\n+This is a multi-part message in MIME format.\n+--------------1.7.6.3.4.gf71f\n+Content-Type: text/plain; charset=UTF-8; format=fixed\n+Content-Transfer-Encoding: 8bit\n+\n+This is a message created with \"git format-patch --attach=123\"\n+---\n+ master | 1 +\n+ файл | 1 +\n+ 2 files changed, 2 insertions(+), 0 deletions(-)\n+ create mode 100644 master\n+ create mode 100644 файл\n+\n+\n+--------------123\n+Content-Type: text/x-patch; name=\"0001-.patch\"\n+Content-Transfer-Encoding: 8bit\n+Content-Disposition: attachment; filename=\"0001-.patch\"\n+\n+diff --git a/master b/master\n+new file mode 100644\n+index 0000000..1f7391f\n+--- /dev/null\n++++ b/master\n+@@ -0,0 +1 @@\n++master\n+diff --git a/файл b/файл\n+new file mode 100644\n+index 0000000..44e5cfe\n+--- /dev/null\n++++ b/файл\n+@@ -0,0 +1 @@\n++содержимое файла\n+\n+--------------123--\n+EOF\n+'\n+\n+test_expect_success $PREREQ 'setup expect' '\n+cat >expected <stdout &&\n+\tgrep \"Subject\" msgtxt1 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.8.1.1.10.g9255f3f\n"},{"id":"207818","messageId":"20130125174700.GA3700@shrek.podlesie.net","threadId":"32726","inReplyTo":"4de442db9fd0896f78166e6038b6ea35ed5ab266.1359126360.git.Alex.Crezoff@gmail.com","subject":"Re: [PATCH] send-email: Honor multi-part email messages","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2013-01-25T17:47:00Z","receivedAt":"2013-01-25T17:47:00Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Fri, Jan 25, 2013 at 07:28:54PM +0400, Alexey Shumkin wrote:\n> \"git format-patch --attach/--inline\" generates multi-part messages.\n> Every part of such messages can contain non-ASCII characters with its own\n> \"Content-Type\" and \"Content-Transfer-Encoding\" headers.\n> But git-send-mail script interprets a patch-file as one-part message\n> and does not recognize multi-part messages.\n> So already quoted printable email subject may be encoded as quoted printable\n> again. Due to this bug email subject looks corrupted in email clients.\n\nI don't think that the problem with the Subject is multi-part message\nspecific. The real problem with the Subject is probably that\nis_rfc2047_quoted() does not detect that the Subject is already quoted.\n\nOf course we still need that explicit multi-part message support to\navoid \"Which 8bit encoding should I declare [UTF-8]? \" message.\n\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 94c7f76..d49befe 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1499,12 +1499,17 @@ sub file_has_nonascii {\n> \n> sub body_or_subject_has_nonascii {\n> \tmy $fn = shift;\n> +\tmy $multipart = 0;\n> \topen(my $fh, '<', $fn)\n> \t\tor die \"unable to open $fn: $!\\n\";\n> \twhile (my $line = <$fh>) {\n> \t\tlast if $line =~ /^$/;\n> +\t\tif ($line =~ /^Content-Type:\\s*multipart\\/mixed.*$/) {\n> +\t\t\t$multipart = 1;\n> +\t\t}\n> \t\treturn 1 if $line =~ /^Subject.*[^[:ascii:]]/;\n> \t}\n> +\treturn 0 if $multipart;\n> \twhile (my $line = <$fh>) {\n> \t\treturn 1 if $line =~ /[^[:ascii:]]/;\n> \t}\n\nAfter this change the function name is no longer appropriate.\nMaybe we should join body_or_subject_has_nonascii()\nand file_declares_8bit_cte() because in case of multi-part messages\n\t\"next unless (body_or_subject_has_nonascii($f)\n\t\t && !file_declares_8bit_cte($f));\"\nis not valid anymore. We could also check for broken_encoding\nin single pass.\n\nThanks,\n\nKrzysiek\n"},{"id":"207820","messageId":"7va9rx5dvd.fsf@alter.siamese.dyndns.org","threadId":"32726","inReplyTo":"cover.1359126360.git.Alex.Crezoff@gmail.com","subject":"Re: [PATCH resent] send-email: Honor multi-part email messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-25T18:14:30Z","receivedAt":"2013-01-25T18:14:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexey Shumkin writes:\n\n> This function is used to determine \"broken\" (non-ASCII) headers (to be encode them)\n> The problem is if \"Subject\" is not broken, but message body contains non-ASCII chars,\n> subject is marked as broken and encoded again.\n\nI think that is not a \"problem\" but is a mere symptom.\n\nThe remainder of the codeflow of send-email, AFAICS (it's not my\ncode), is not prepared to deal with multipart messages at all. In\norder to handle multi-part properly, you may still have to fix\nbroken Subject: of the whole thing, and you may also want to fix\nbroken headers inside one part while keeping correctly formatted\npart intact.\n\nYour patch just stops an early error checking that is meant for a\nnon multi-part message that happens to trigger on a multi-part\nmessage in your test case from triggering (i.e. masking a symptom)\nand let the remaining lines of the multi-part message to codepath\nthat does not do anything special to handle multi-part messages\ncorrectly, letting it do whatever it happens to do to a message\nassuming it is not a multi-part message, no?\n\nIn other words, making send-email capable of handling a multi-part\nmight be a worthy thing to do, but I do not think your patch is a\ngood first step for doing so.\n"},{"id":"207848","messageId":"20130125222411.GD23626@sigill.intra.peff.net","threadId":"32726","inReplyTo":"20130125174700.GA3700@shrek.podlesie.net","subject":"Re: [PATCH] send-email: Honor multi-part email messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T22:24:11Z","receivedAt":"2013-01-25T22:24:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 25, 2013 at 06:47:00PM +0100, Krzysztof Mazur wrote:\n\n> On Fri, Jan 25, 2013 at 07:28:54PM +0400, Alexey Shumkin wrote:\n> > \"git format-patch --attach/--inline\" generates multi-part messages.\n> > Every part of such messages can contain non-ASCII characters with its own\n> > \"Content-Type\" and \"Content-Transfer-Encoding\" headers.\n> > But git-send-mail script interprets a patch-file as one-part message\n> > and does not recognize multi-part messages.\n> > So already quoted printable email subject may be encoded as quoted printable\n> > again. Due to this bug email subject looks corrupted in email clients.\n> \n> I don't think that the problem with the Subject is multi-part message\n> specific. The real problem with the Subject is probably that\n> is_rfc2047_quoted() does not detect that the Subject is already quoted.\n\nI have not even looked at this problem at all, but seeing this function\nname:\n\n> > sub body_or_subject_has_nonascii {\n\nMakes me think something is very wrong. The subject line should not have\nanything to do whatsoever with a content-type or\ncontent-transfer-encoding header. It should either be rfc2047 encoded or\nnot, and the encoding used does not have to correspond to what is used\nelsewhere in the message. rfc2047 is very clear that other MIME headers\nare not necessary to interpret encoded words in headers.\n\nSo this loop:\n\n\tforeach my $f (@files) {\n\t next unless (body_or_subject_has_nonascii($f)\n\t && !file_declares_8bit_cte($f));\n\t $broken_encoding{$f} = 1;\n\t}\n\ndoes not seem right at all. Only the body depends on the 8bit CTE.\n\n-Peff\n"}]}