[PATCH 0/3] Automatic transfer encoding for patches

[PATCH 0/3] Automatic transfer encoding for patches

[brian m. carlson]

git send-email has the --validate option, which is the default, to check
if a patch has lines longer than RFC 5321 allows (998 octets).  However,
it doesn't take into account the fact that using certain transfer
encodings makes this issue moot, and it also doesn't provide any helpful
clue to the user about what to do when this fails.

This series introduces an "auto" value for --transfer-encoding that uses
8bit when possible (i.e. when lines are 998 octets or shorter) and
quoted-printable otherwise; it then makes this the default behavior.  It
also makes --validate aware of transfer encoding so it doesn't complain
when using quoted-printable or base64.

git am already understands how to handle patches in any valid transfer
encoding.

As a result of this series, we always produce MIME messages with a
Content-Transfer-Encoding header.  I don't think this will cause any
problems, but if someone knows of a reason why it would, please shout
loudly.

brian m. carlson (3):
  send-email: add an auto option for transfer encoding
  send-email: accept long lines with suitable transfer encoding
  send-email: automatically determine transfer-encoding

 Documentation/git-send-email.txt | 15 +++++----
 git-send-email.perl              | 36 ++++++++++----------
 t/t9001-send-email.sh            | 58 ++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 24 deletions(-)

[PATCH 1/3] send-email: add an auto option for transfer encoding

[brian m. carlson]

For most patches, using a transfer encoding of 8bit provides good
compatibility with most servers and makes it as easy as possible to view
patches.  However, there are some patches for which 8bit is not a valid
encoding: RFC 5321 specifies that a message must not have lines
exceeding 998 octets.

Add a transfer encoding value, auto, which indicates that a patch should
use 8bit where allowed and quoted-printable otherwise.  Choose
quoted-printable instead of base64, since base64-encoded plain text is
treated as suspicious by some spam filters.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt | 11 +++++++----
 git-send-email.perl              | 12 +++++++-----
 t/t9001-send-email.sh            | 24 ++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 4f3efde80c..385c7de9e2 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -137,15 +137,18 @@ Note that no attempts whatsoever are made to validate the encoding.
 	Specify encoding of compose message. Default is the value of the
 	'sendemail.composeencoding'; if that is unspecified, UTF-8 is assumed.
 
---transfer-encoding=(7bit|8bit|quoted-printable|base64)::
+--transfer-encoding=(7bit|8bit|quoted-printable|base64|auto)::
 	Specify the transfer encoding to be used to send the message over SMTP.
 	7bit will fail upon encountering a non-ASCII message.  quoted-printable
 	can be useful when the repository contains files that contain carriage
 	returns, but makes the raw patch email file (as saved from a MUA) much
 	harder to inspect manually.  base64 is even more fool proof, but also
-	even more opaque.  Default is the value of the `sendemail.transferEncoding`
-	configuration value; if that is unspecified, git will use 8bit and not
-	add a Content-Transfer-Encoding header.
+	even more opaque.  auto will use 8bit when possible, and quoted-printable
+	otherwise.
++
+Default is the value of the `sendemail.transferEncoding` configuration
+value; if that is unspecified, git will use 8bit and not add a
+Content-Transfer-Encoding header.
 
 --xmailer::
 --no-xmailer::
diff --git a/git-send-email.perl b/git-send-email.perl
index 8ec70e58ed..a76953c310 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1739,9 +1739,8 @@ sub process_file {
 	}
 	if (defined $target_xfer_encoding) {
 		$xfer_encoding = '8bit' if not defined $xfer_encoding;
-		$message = apply_transfer_encoding(
+		($message, $xfer_encoding) = apply_transfer_encoding(
 			$message, $xfer_encoding, $target_xfer_encoding);
-		$xfer_encoding = $target_xfer_encoding;
 	}
 	if (defined $xfer_encoding) {
 		push @xh, "Content-Transfer-Encoding: $xfer_encoding";
@@ -1852,13 +1851,16 @@ sub apply_transfer_encoding {
 	$message = MIME::Base64::decode($message)
 		if ($from eq 'base64');
 
+	$to = ($message =~ /.{999,}/) ? 'quoted-printable' :'8bit'
+		if $to eq 'auto';
+
 	die __("cannot send message as 7bit")
 		if ($to eq '7bit' and $message =~ /[^[:ascii:]]/);
-	return $message
+	return ($message, $to)
 		if ($to eq '7bit' or $to eq '8bit');
-	return MIME::QuotedPrint::encode($message, "\n", 0)
+	return (MIME::QuotedPrint::encode($message, "\n", 0), $to)
 		if ($to eq 'quoted-printable');
-	return MIME::Base64::encode($message, "\n")
+	return (MIME::Base64::encode($message, "\n"), $to)
 		if ($to eq 'base64');
 	die __("invalid transfer encoding");
 }
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e80eacbb1b..6cdcbcb19e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -456,6 +456,30 @@ test_expect_success $PREREQ 'allow long lines with --no-validate' '
 		2>errors
 '
 
+test_expect_success $PREREQ 'short lines with auto encoding are 8bit' '
+	clean_fake_sendmail &&
+	git send-email \
+		--from="A <author@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--transfer-encoding=auto \
+		$patches &&
+	grep "Content-Transfer-Encoding: 8bit" msgtxt1
+'
+
+test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable' '
+	clean_fake_sendmail &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--transfer-encoding=auto \
+		--no-validate \
+		longline.patch \
+		2>errors &&
+	grep "Content-Transfer-Encoding: quoted-printable" msgtxt1
+'
+
 test_expect_success $PREREQ 'Invalid In-Reply-To' '
 	clean_fake_sendmail &&
 	git send-email \

[PATCH 2/3] send-email: accept long lines with suitable transfer encoding

[brian m. carlson]

With --validate (which is the default), we warn about lines exceeding
998 characters due to the limits specified in RFC 5321.  However, if
we're using a suitable transfer encoding (quoted-printable or base64),
we're guaranteed not to have lines exceeding 76 characters, so there's
no need to fail in this case.  The auto transfer encoding handles this
specific case, so accept it as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt |  5 +++--
 git-send-email.perl              |  8 ++++++--
 t/t9001-send-email.sh            | 13 +++++++++++++
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 385c7de9e2..f44fb4b81e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -401,8 +401,9 @@ have been specified, in which case default to 'compose'.
 +
 --
 		*	Invoke the sendemail-validate hook if present (see linkgit:githooks[5]).
-		*	Warn of patches that contain lines longer than 998 characters; this
-			is due to SMTP limits as described by http://www.ietf.org/rfc/rfc2821.txt.
+		*	Warn of patches that contain lines longer than 998 characters unless
+			a suitable transfer encoding is used; this is due to SMTP limits as
+			described by http://www.ietf.org/rfc/rfc2821.txt.
 --
 +
 Default is the value of `sendemail.validate`; if this is not set,
diff --git a/git-send-email.perl b/git-send-email.perl
index a76953c310..4ea30c4070 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -645,7 +645,7 @@ sub is_format_patch_arg {
 if ($validate) {
 	foreach my $f (@files) {
 		unless (-p $f) {
-			my $error = validate_patch($f);
+			my $error = validate_patch($f, $target_xfer_encoding);
 			$error and die sprintf(__("fatal: %s: %s\nwarning: no patches were sent\n"),
 						  $f, $error);
 		}
@@ -1879,7 +1879,7 @@ sub unique_email_list {
 }
 
 sub validate_patch {
-	my $fn = shift;
+	my ($fn, $xfer_encoding) = @_;
 
 	if ($repo) {
 		my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),
@@ -1899,6 +1899,10 @@ sub validate_patch {
 		return $hook_error if $hook_error;
 	}
 
+	# Any long lines will be automatically fixed if we use a suitable transfer
+	# encoding.
+	return if $xfer_encoding =~ /^(?:auto|quoted-printable|base64)$/;
+
 	open(my $fh, '<', $fn)
 		or die sprintf(__("unable to open %s: %s\n"), $fn, $!);
 	while (my $line = <$fh>) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6cdcbcb19e..6a0ace386b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -480,6 +480,19 @@ test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable'
 	grep "Content-Transfer-Encoding: quoted-printable" msgtxt1
 '
 
+test_expect_success $PREREQ '--validate passes with certain encodings' '
+	for enc in auto quoted-printable base64
+	do
+		git send-email \
+			--from="Example <nobody@example.com>" \
+			--to=nobody@example.com \
+			--smtp-server="$(pwd)/fake.sendmail" \
+			--transfer-encoding=$enc \
+			--validate \
+			$patches longline.patch
+	done
+'
+
 test_expect_success $PREREQ 'Invalid In-Reply-To' '
 	clean_fake_sendmail &&
 	git send-email \

[PATCH 3/3] send-email: automatically determine transfer-encoding

[brian m. carlson]

git send-email, when invoked without a --transfer-encoding option, sends
8bit data without a MIME version or a transfer encoding.  This has
several downsides.

First, unless the transfer encoding is specified, it defaults to 7bit,
meaning that non-ASCII data isn't allowed.  Second, if lines longer than
998 bytes are used, we will send an message that is invalid according to
RFC 5321.  The --validate option, which is the default, catches this
issue, but it isn't clear to many people how to resolve this.

To solve these issues, default the transfer encoding to "auto", so that
we explicitly specify 8bit encoding when lines don't exceed 998 bytes
and quoted-printable otherwise.  This means that we now always emit
Content-Transfer-Encoding and MIME-Version headers, so remove the
conditionals from this portion of the code.

It is unlikely that the unconditional inclusion of these two headers
will affect the deliverability of messages in anything but a positive
way, since MIME is already widespread and well understood by most email
programs.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt |  3 +--
 git-send-email.perl              | 18 ++++++------------
 t/t9001-send-email.sh            | 21 +++++++++++++++++++++
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index f44fb4b81e..1a240b52fc 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -147,8 +147,7 @@ Note that no attempts whatsoever are made to validate the encoding.
 	otherwise.
 +
 Default is the value of the `sendemail.transferEncoding` configuration
-value; if that is unspecified, git will use 8bit and not add a
-Content-Transfer-Encoding header.
+value; if that is unspecified, default to `auto`.
 
 --xmailer::
 --no-xmailer::
diff --git a/git-send-email.perl b/git-send-email.perl
index 4ea30c4070..c4fae17787 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -231,7 +231,7 @@ sub do_edit {
 my (@suppress_cc);
 my ($auto_8bit_encoding);
 my ($compose_encoding);
-my ($target_xfer_encoding);
+my $target_xfer_encoding = 'auto';
 
 my ($debug_net_smtp) = 0;		# Net::SMTP, see send_message()
 
@@ -1737,17 +1737,11 @@ sub process_file {
 			}
 		}
 	}
-	if (defined $target_xfer_encoding) {
-		$xfer_encoding = '8bit' if not defined $xfer_encoding;
-		($message, $xfer_encoding) = apply_transfer_encoding(
-			$message, $xfer_encoding, $target_xfer_encoding);
-	}
-	if (defined $xfer_encoding) {
-		push @xh, "Content-Transfer-Encoding: $xfer_encoding";
-	}
-	if (defined $xfer_encoding or $has_content_type) {
-		unshift @xh, 'MIME-Version: 1.0' unless $has_mime_version;
-	}
+	$xfer_encoding = '8bit' if not defined $xfer_encoding;
+	($message, $xfer_encoding) = apply_transfer_encoding(
+		$message, $xfer_encoding, $target_xfer_encoding);
+	push @xh, "Content-Transfer-Encoding: $xfer_encoding";
+	unshift @xh, 'MIME-Version: 1.0' unless $has_mime_version;
 
 	$needs_confirm = (
 		$confirm eq "always" or
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6a0ace386b..7edce50e65 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -225,6 +225,8 @@ X-Mailer: X-MAILER-STRING
 In-Reply-To: <unique-message-id@example.com>
 References: <unique-message-id@example.com>
 Reply-To: Reply <reply@example.com>
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -415,6 +417,7 @@ test_expect_success $PREREQ 'reject long lines' '
 		--from="Example <nobody@example.com>" \
 		--to=nobody@example.com \
 		--smtp-server="$(pwd)/fake.sendmail" \
+		--transfer-encoding=8bit \
 		$patches longline.patch \
 		2>errors &&
 	grep longline.patch errors
@@ -610,6 +613,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -654,6 +659,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -689,6 +696,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -715,6 +724,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -749,6 +760,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -780,6 +793,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -811,6 +826,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -846,6 +863,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -874,6 +893,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF

Re: [PATCH 1/3] send-email: add an auto option for transfer encoding

[Eric Sunshine]

On Thu, Jul 5, 2018 at 10:24 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> For most patches, using a transfer encoding of 8bit provides good
> compatibility with most servers and makes it as easy as possible to view
> patches.  However, there are some patches for which 8bit is not a valid
> encoding: RFC 5321 specifies that a message must not have lines
> exceeding 998 octets.
>
> Add a transfer encoding value, auto, which indicates that a patch should
> use 8bit where allowed and quoted-printable otherwise.  Choose
> quoted-printable instead of base64, since base64-encoded plain text is
> treated as suspicious by some spam filters.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -1852,13 +1851,16 @@ sub apply_transfer_encoding {
> +       $to = ($message =~ /.{999,}/) ? 'quoted-printable' :'8bit'
> +               if $to eq 'auto';

Style: space after colon: 'quoted-printable' : '8bit'

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> +test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable' '
> +       clean_fake_sendmail &&
> +       git send-email \
> +               --from="Example <nobody@example.com>" \
> +               --to=nobody@example.com \
> +               --smtp-server="$(pwd)/fake.sendmail" \
> +               --transfer-encoding=auto \
> +               --no-validate \
> +               longline.patch \
> +               2>errors &&

Why capture stderr to a file then ignore the file?

> +       grep "Content-Transfer-Encoding: quoted-printable" msgtxt1
> +'

Re: [PATCH 2/3] send-email: accept long lines with suitable transfer encoding

[Eric Sunshine]

On Thu, Jul 5, 2018 at 10:24 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> With --validate (which is the default), we warn about lines exceeding
> 998 characters due to the limits specified in RFC 5321.  However, if
> we're using a suitable transfer encoding (quoted-printable or base64),
> we're guaranteed not to have lines exceeding 76 characters, so there's
> no need to fail in this case.  The auto transfer encoding handles this
> specific case, so accept it as well.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> @@ -401,8 +401,9 @@ have been specified, in which case default to 'compose'.
> -               *       Warn of patches that contain lines longer than 998 characters; this
> -                       is due to SMTP limits as described by http://www.ietf.org/rfc/rfc2821.txt.
> +               *       Warn of patches that contain lines longer than 998 characters unless
> +                       a suitable transfer encoding is used; this is due to SMTP limits as
> +                       described by http://www.ietf.org/rfc/rfc2821.txt.

A reader might like to know what "a suitable transfer encoding" is.
Perhaps add a "such as 'auto', 'quoted-printable' and 'base64'"
comment?

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> @@ -480,6 +480,19 @@ test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable'
> +test_expect_success $PREREQ '--validate passes with certain encodings' '
> +       for enc in auto quoted-printable base64
> +       do
> +               git send-email \
> +                       --from="Example <nobody@example.com>" \
> +                       --to=nobody@example.com \
> +                       --smtp-server="$(pwd)/fake.sendmail" \
> +                       --transfer-encoding=$enc \
> +                       --validate \
> +                       $patches longline.patch
> +       done
> +'

If you turn this inside out, it will be easier to figure out which
encoding fails (if one ever does). That is:

    for enc in auto quoted-printable base64
    do
        test_expect_success $PREREQ "--validate passes with encoding $enc" '
            git send-email ...
        '
    done

Re: [PATCH 2/3] send-email: accept long lines with suitable transfer encoding

[Drew DeVault]

On 2018-07-06  2:23 AM, brian m. carlson wrote:
> diff --git a/git-send-email.perl b/git-send-email.perl
> index a76953c310..4ea30c4070 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1899,6 +1899,10 @@ sub validate_patch {
>  		return $hook_error if $hook_error;
>  	}
>  
> +	# Any long lines will be automatically fixed if we use a suitable transfer
> +	# encoding.
> +	return if $xfer_encoding =~ /^(?:auto|quoted-printable|base64)$/;

Rather than returning in this case I'd sooner wrap the length check in
this test. If additional checks are added in the future it'd be too easy
to accidentally skip them if the transfer encoding is quoted-printable.

Re: [PATCH 0/3] Automatic transfer encoding for patches

[Drew DeVault]

Overall this series looks good. Thanks for putting the patches together!

Re: [PATCH 2/3] send-email: accept long lines with suitable transfer encoding

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

On Fri, Jul 06, 2018 at 08:26:04AM -0400, Drew DeVault wrote:
> On 2018-07-06  2:23 AM, brian m. carlson wrote:
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > index a76953c310..4ea30c4070 100755
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -1899,6 +1899,10 @@ sub validate_patch {
> >  		return $hook_error if $hook_error;
> >  	}
> >  
> > +	# Any long lines will be automatically fixed if we use a suitable transfer
> > +	# encoding.
> > +	return if $xfer_encoding =~ /^(?:auto|quoted-printable|base64)$/;
> 
> Rather than returning in this case I'd sooner wrap the length check in
> this test. If additional checks are added in the future it'd be too easy
> to accidentally skip them if the transfer encoding is quoted-printable.

Okay, that's fair.  I can do that.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: [PATCH 1/3] send-email: add an auto option for transfer encoding

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

On Fri, Jul 06, 2018 at 02:01:25AM -0400, Eric Sunshine wrote:
> On Thu, Jul 5, 2018 at 10:24 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > For most patches, using a transfer encoding of 8bit provides good
> > compatibility with most servers and makes it as easy as possible to view
> > patches.  However, there are some patches for which 8bit is not a valid
> > encoding: RFC 5321 specifies that a message must not have lines
> > exceeding 998 octets.
> >
> > Add a transfer encoding value, auto, which indicates that a patch should
> > use 8bit where allowed and quoted-printable otherwise.  Choose
> > quoted-printable instead of base64, since base64-encoded plain text is
> > treated as suspicious by some spam filters.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > @@ -1852,13 +1851,16 @@ sub apply_transfer_encoding {
> > +       $to = ($message =~ /.{999,}/) ? 'quoted-printable' :'8bit'
> > +               if $to eq 'auto';
> 
> Style: space after colon: 'quoted-printable' : '8bit'

Will fix.

> > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> > +test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable' '
> > +       clean_fake_sendmail &&
> > +       git send-email \
> > +               --from="Example <nobody@example.com>" \
> > +               --to=nobody@example.com \
> > +               --smtp-server="$(pwd)/fake.sendmail" \
> > +               --transfer-encoding=auto \
> > +               --no-validate \
> > +               longline.patch \
> > +               2>errors &&
> 
> Why capture stderr to a file then ignore the file?

Copy-paste error from an earlier test.  Will fix.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

[PATCH v2 0/4] Automatic transfer encoding for patches

[brian m. carlson]

This series introduces an "auto" value for git send-email
--transfer-encoding that uses 8bit when possible (i.e. when lines are
998 octets or shorter) and quoted-printable otherwise; it then makes
this the default behavior.  It also makes --validate aware of transfer
encoding so it doesn't complain when using quoted-printable or base64.

Changes from v1:
* Update commit messages to refer to RFC 5322.
* Add a missing space.
* Remove the needless capture of stderr.
* Define "suitable transfer encoding".
* Invert test to better capture failures.
* Wrap --validate code in an if block instead of returning early.
* Update documentation to reflect correct, modern RFC.

brian m. carlson (4):
  send-email: add an auto option for transfer encoding
  send-email: accept long lines with suitable transfer encoding
  send-email: automatically determine transfer-encoding
  docs: correct RFC specifying email line length

 Documentation/git-send-email.txt | 17 ++++++----
 git-send-email.perl              | 46 +++++++++++++-------------
 t/t9001-send-email.sh            | 57 ++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 29 deletions(-)

[PATCH v2 1/4] send-email: add an auto option for transfer encoding

[brian m. carlson]

For most patches, using a transfer encoding of 8bit provides good
compatibility with most servers and makes it as easy as possible to view
patches.  However, there are some patches for which 8bit is not a valid
encoding: RFC 5322 specifies that a message must not have lines
exceeding 998 octets.

Add a transfer encoding value, auto, which indicates that a patch should
use 8bit where allowed and quoted-printable otherwise.  Choose
quoted-printable instead of base64, since base64-encoded plain text is
treated as suspicious by some spam filters.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt | 11 +++++++----
 git-send-email.perl              | 12 +++++++-----
 t/t9001-send-email.sh            | 23 +++++++++++++++++++++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 4f3efde80c..385c7de9e2 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -137,15 +137,18 @@ Note that no attempts whatsoever are made to validate the encoding.
 	Specify encoding of compose message. Default is the value of the
 	'sendemail.composeencoding'; if that is unspecified, UTF-8 is assumed.
 
---transfer-encoding=(7bit|8bit|quoted-printable|base64)::
+--transfer-encoding=(7bit|8bit|quoted-printable|base64|auto)::
 	Specify the transfer encoding to be used to send the message over SMTP.
 	7bit will fail upon encountering a non-ASCII message.  quoted-printable
 	can be useful when the repository contains files that contain carriage
 	returns, but makes the raw patch email file (as saved from a MUA) much
 	harder to inspect manually.  base64 is even more fool proof, but also
-	even more opaque.  Default is the value of the `sendemail.transferEncoding`
-	configuration value; if that is unspecified, git will use 8bit and not
-	add a Content-Transfer-Encoding header.
+	even more opaque.  auto will use 8bit when possible, and quoted-printable
+	otherwise.
++
+Default is the value of the `sendemail.transferEncoding` configuration
+value; if that is unspecified, git will use 8bit and not add a
+Content-Transfer-Encoding header.
 
 --xmailer::
 --no-xmailer::
diff --git a/git-send-email.perl b/git-send-email.perl
index 8ec70e58ed..1736a09d21 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1739,9 +1739,8 @@ sub process_file {
 	}
 	if (defined $target_xfer_encoding) {
 		$xfer_encoding = '8bit' if not defined $xfer_encoding;
-		$message = apply_transfer_encoding(
+		($message, $xfer_encoding) = apply_transfer_encoding(
 			$message, $xfer_encoding, $target_xfer_encoding);
-		$xfer_encoding = $target_xfer_encoding;
 	}
 	if (defined $xfer_encoding) {
 		push @xh, "Content-Transfer-Encoding: $xfer_encoding";
@@ -1852,13 +1851,16 @@ sub apply_transfer_encoding {
 	$message = MIME::Base64::decode($message)
 		if ($from eq 'base64');
 
+	$to = ($message =~ /.{999,}/) ? 'quoted-printable' : '8bit'
+		if $to eq 'auto';
+
 	die __("cannot send message as 7bit")
 		if ($to eq '7bit' and $message =~ /[^[:ascii:]]/);
-	return $message
+	return ($message, $to)
 		if ($to eq '7bit' or $to eq '8bit');
-	return MIME::QuotedPrint::encode($message, "\n", 0)
+	return (MIME::QuotedPrint::encode($message, "\n", 0), $to)
 		if ($to eq 'quoted-printable');
-	return MIME::Base64::encode($message, "\n")
+	return (MIME::Base64::encode($message, "\n"), $to)
 		if ($to eq 'base64');
 	die __("invalid transfer encoding");
 }
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e80eacbb1b..a35cba6a4c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -456,6 +456,29 @@ test_expect_success $PREREQ 'allow long lines with --no-validate' '
 		2>errors
 '
 
+test_expect_success $PREREQ 'short lines with auto encoding are 8bit' '
+	clean_fake_sendmail &&
+	git send-email \
+		--from="A <author@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--transfer-encoding=auto \
+		$patches &&
+	grep "Content-Transfer-Encoding: 8bit" msgtxt1
+'
+
+test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable' '
+	clean_fake_sendmail &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--transfer-encoding=auto \
+		--no-validate \
+		longline.patch &&
+	grep "Content-Transfer-Encoding: quoted-printable" msgtxt1
+'
+
 test_expect_success $PREREQ 'Invalid In-Reply-To' '
 	clean_fake_sendmail &&
 	git send-email \

[PATCH v2 2/4] send-email: accept long lines with suitable transfer encoding

[brian m. carlson]

With --validate (which is the default), we warn about lines exceeding
998 characters due to the limits specified in RFC 5322.  However, if
we're using a suitable transfer encoding (quoted-printable or base64),
we're guaranteed not to have lines exceeding 76 characters, so there's
no need to fail in this case.  The auto transfer encoding handles this
specific case, so accept it as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt |  7 +++++--
 git-send-email.perl              | 18 +++++++++++-------
 t/t9001-send-email.sh            | 13 +++++++++++++
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 385c7de9e2..0e648075bb 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -401,8 +401,11 @@ have been specified, in which case default to 'compose'.
 +
 --
 		*	Invoke the sendemail-validate hook if present (see linkgit:githooks[5]).
-		*	Warn of patches that contain lines longer than 998 characters; this
-			is due to SMTP limits as described by http://www.ietf.org/rfc/rfc2821.txt.
+		*	Warn of patches that contain lines longer than
+			998 characters unless a suitable transfer encoding
+			('auto', 'base64', or 'quoted-printable') is used;
+			this is due to SMTP limits as described by
+			http://www.ietf.org/rfc/rfc2821.txt.
 --
 +
 Default is the value of `sendemail.validate`; if this is not set,
diff --git a/git-send-email.perl b/git-send-email.perl
index 1736a09d21..e6bcc55827 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -645,7 +645,7 @@ sub is_format_patch_arg {
 if ($validate) {
 	foreach my $f (@files) {
 		unless (-p $f) {
-			my $error = validate_patch($f);
+			my $error = validate_patch($f, $target_xfer_encoding);
 			$error and die sprintf(__("fatal: %s: %s\nwarning: no patches were sent\n"),
 						  $f, $error);
 		}
@@ -1879,7 +1879,7 @@ sub unique_email_list {
 }
 
 sub validate_patch {
-	my $fn = shift;
+	my ($fn, $xfer_encoding) = @_;
 
 	if ($repo) {
 		my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'),
@@ -1899,11 +1899,15 @@ sub validate_patch {
 		return $hook_error if $hook_error;
 	}
 
-	open(my $fh, '<', $fn)
-		or die sprintf(__("unable to open %s: %s\n"), $fn, $!);
-	while (my $line = <$fh>) {
-		if (length($line) > 998) {
-			return sprintf(__("%s: patch contains a line longer than 998 characters"), $.);
+	# Any long lines will be automatically fixed if we use a suitable transfer
+	# encoding.
+	unless ($xfer_encoding =~ /^(?:auto|quoted-printable|base64)$/) {
+		open(my $fh, '<', $fn)
+			or die sprintf(__("unable to open %s: %s\n"), $fn, $!);
+		while (my $line = <$fh>) {
+			if (length($line) > 998) {
+				return sprintf(__("%s: patch contains a line longer than 998 characters"), $.);
+			}
 		}
 	}
 	return;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index a35cba6a4c..1b474cca28 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -479,6 +479,19 @@ test_expect_success $PREREQ 'long lines with auto encoding are quoted-printable'
 	grep "Content-Transfer-Encoding: quoted-printable" msgtxt1
 '
 
+for enc in auto quoted-printable base64
+do
+	test_expect_success $PREREQ "--validate passes with encoding $enc" '
+		git send-email \
+			--from="Example <nobody@example.com>" \
+			--to=nobody@example.com \
+			--smtp-server="$(pwd)/fake.sendmail" \
+			--transfer-encoding=$enc \
+			--validate \
+			$patches longline.patch
+	'
+done
+
 test_expect_success $PREREQ 'Invalid In-Reply-To' '
 	clean_fake_sendmail &&
 	git send-email \

[PATCH v2 3/4] send-email: automatically determine transfer-encoding

[brian m. carlson]

git send-email, when invoked without a --transfer-encoding option, sends
8bit data without a MIME version or a transfer encoding.  This has
several downsides.

First, unless the transfer encoding is specified, it defaults to 7bit,
meaning that non-ASCII data isn't allowed.  Second, if lines longer than
998 bytes are used, we will send an message that is invalid according to
RFC 5322.  The --validate option, which is the default, catches this
issue, but it isn't clear to many people how to resolve this.

To solve these issues, default the transfer encoding to "auto", so that
we explicitly specify 8bit encoding when lines don't exceed 998 bytes
and quoted-printable otherwise.  This means that we now always emit
Content-Transfer-Encoding and MIME-Version headers, so remove the
conditionals from this portion of the code.

It is unlikely that the unconditional inclusion of these two headers
will affect the deliverability of messages in anything but a positive
way, since MIME is already widespread and well understood by most email
programs.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt |  3 +--
 git-send-email.perl              | 18 ++++++------------
 t/t9001-send-email.sh            | 21 +++++++++++++++++++++
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 0e648075bb..2f32dbf16d 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -147,8 +147,7 @@ Note that no attempts whatsoever are made to validate the encoding.
 	otherwise.
 +
 Default is the value of the `sendemail.transferEncoding` configuration
-value; if that is unspecified, git will use 8bit and not add a
-Content-Transfer-Encoding header.
+value; if that is unspecified, default to `auto`.
 
 --xmailer::
 --no-xmailer::
diff --git a/git-send-email.perl b/git-send-email.perl
index e6bcc55827..f4c07908d2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -231,7 +231,7 @@ sub do_edit {
 my (@suppress_cc);
 my ($auto_8bit_encoding);
 my ($compose_encoding);
-my ($target_xfer_encoding);
+my $target_xfer_encoding = 'auto';
 
 my ($debug_net_smtp) = 0;		# Net::SMTP, see send_message()
 
@@ -1737,17 +1737,11 @@ sub process_file {
 			}
 		}
 	}
-	if (defined $target_xfer_encoding) {
-		$xfer_encoding = '8bit' if not defined $xfer_encoding;
-		($message, $xfer_encoding) = apply_transfer_encoding(
-			$message, $xfer_encoding, $target_xfer_encoding);
-	}
-	if (defined $xfer_encoding) {
-		push @xh, "Content-Transfer-Encoding: $xfer_encoding";
-	}
-	if (defined $xfer_encoding or $has_content_type) {
-		unshift @xh, 'MIME-Version: 1.0' unless $has_mime_version;
-	}
+	$xfer_encoding = '8bit' if not defined $xfer_encoding;
+	($message, $xfer_encoding) = apply_transfer_encoding(
+		$message, $xfer_encoding, $target_xfer_encoding);
+	push @xh, "Content-Transfer-Encoding: $xfer_encoding";
+	unshift @xh, 'MIME-Version: 1.0' unless $has_mime_version;
 
 	$needs_confirm = (
 		$confirm eq "always" or
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1b474cca28..1da282c415 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -225,6 +225,8 @@ X-Mailer: X-MAILER-STRING
 In-Reply-To: <unique-message-id@example.com>
 References: <unique-message-id@example.com>
 Reply-To: Reply <reply@example.com>
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -415,6 +417,7 @@ test_expect_success $PREREQ 'reject long lines' '
 		--from="Example <nobody@example.com>" \
 		--to=nobody@example.com \
 		--smtp-server="$(pwd)/fake.sendmail" \
+		--transfer-encoding=8bit \
 		$patches longline.patch \
 		2>errors &&
 	grep longline.patch errors
@@ -609,6 +612,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -653,6 +658,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -688,6 +695,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -714,6 +723,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -748,6 +759,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -779,6 +792,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -810,6 +825,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -845,6 +862,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF
@@ -873,6 +892,8 @@ Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
 
 Result: OK
 EOF

[PATCH v2 4/4] docs: correct RFC specifying email line length

[brian m. carlson]

The git send-email documentation specifies RFC 2821 (the SMTP RFC) as
providing line length limits, but the specification that restricts line
length to 998 octets is RFC 2822 (the email message format RFC).  Since
RFC 2822 has been obsoleted by RFC 5322, update the text to refer to RFC
5322 instead of RFC 2821.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-send-email.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 2f32dbf16d..465a4ecbed 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -404,7 +404,7 @@ have been specified, in which case default to 'compose'.
 			998 characters unless a suitable transfer encoding
 			('auto', 'base64', or 'quoted-printable') is used;
 			this is due to SMTP limits as described by
-			http://www.ietf.org/rfc/rfc2821.txt.
+			http://www.ietf.org/rfc/rfc5322.txt.
 --
 +
 Default is the value of `sendemail.validate`; if this is not set,

Re: [PATCH v2 0/4] Automatic transfer encoding for patches

[Drew DeVault]

LGTM, thanks for the v2.