send-email: Honor multi-part email messages

Let's try to involve Krzysztof Mazur <[email protected]>
who have met the similar problem recently
(according to this thread http://thread.gmane.org/gmane.comp.version-control.git/208297/focus=208310)
I recitate myself:

This function is used to determine "broken" (non-ASCII) headers (to be encode them) The problem is if "Subject" is not broken, but message body contains non-ASCII chars, subject is marked as broken and encoded again.

P.S.
To involved: the beginning of thread is here http://thread.gmane.org/gmane.comp.version-control.git/181743

Alexey Shumkin (1):
send-email: Honor multi-part email messages

 git-send-email.perl   |  5 ++++
 t/t9001-send-email.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

1.8.1.1.10.g9255f3f

"git format-patch --attach/--inline" generates multi-part messages. Every part of such messages can contain non-ASCII characters with its own "Content-Type" and "Content-Transfer-Encoding" headers.
But git-send-mail script interprets a patch-file as one-part message and does not recognize multi-part messages.
So already quoted printable email subject may be encoded as quoted printable again. Due to this bug email subject looks corrupted in email clients.

---
 git-send-email.perl   |  5 ++++
 t/t9001-send-email.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

index 94c7f76..d49befe 100755
@@ -1499,12 +1499,17 @@ sub file_has_nonascii {
 
 sub body_or_subject_has_nonascii {
 	my $fn = shift;
+	my $multipart = 0;
 	open(my $fh, '<', $fn)
 		or die "unable to open $fn: $!\n";
 	while (my $line = <$fh>) {
 		last if $line =~ /^$/;
+		if ($line =~ /^Content-Type:\s*multipart\/mixed.*$/) {
+			$multipart = 1;
+		}
 		return 1 if $line =~ /^Subject.*[^[:ascii:]]/;
 	}
+	return 0 if $multipart;
 	while (my $line = <$fh>) {
 		return 1 if $line =~ /[^[:ascii:]]/;
 	}

1.8.1.1.10.g9255f3f

I don't think that the problem with the Subject is multi-part message specific. The real problem with the Subject is probably that is_rfc2047_quoted() does not detect that the Subject is already quoted.

Of course we still need that explicit multi-part message support to avoid "Which 8bit encoding should I declare [UTF-8]? " message.

	"next unless (body_or_subject_has_nonascii($f)
		     && !file_declares_8bit_cte($f));"

I have not even looked at this problem at all, but seeing this function name:

Makes me think something is very wrong. The subject line should not have anything to do whatsoever with a content-type or content-transfer-encoding header. It should either be rfc2047 encoded or not, and the encoding used does not have to correspond to what is used elsewhere in the message. rfc2047 is very clear that other MIME headers are not necessary to interpret encoded words in headers.

So this loop:

	foreach my $f (@files) {
	        next unless (body_or_subject_has_nonascii($f)
	                     && !file_declares_8bit_cte($f));
	        $broken_encoding{$f} = 1;
	}

does not seem right at all. Only the body depends on the 8bit CTE.

-Peff

I think that is not a "problem" but is a mere symptom.

The remainder of the codeflow of send-email, AFAICS (it's not my code), is not prepared to deal with multipart messages at all. In order to handle multi-part properly, you may still have to fix broken Subject: of the whole thing, and you may also want to fix broken headers inside one part while keeping correctly formatted part intact.

Your patch just stops an early error checking that is meant for a non multi-part message that happens to trigger on a multi-part message in your test case from triggering (i.e. masking a symptom) and let the remaining lines of the multi-part message to codepath that does not do anything special to handle multi-part messages correctly, letting it do whatever it happens to do to a message assuming it is not a multi-part message, no?

In other words, making send-email capable of handling a multi-part might be a worthy thing to do, but I do not think your patch is a good first step for doing so.