Re: [BUG] format-patch does not wrap From-field after author name

[Jeff King <peff@peff.net>]

On Thu, Apr 14, 2011 at 10:12:44AM -0700, Junio C Hamano wrote:

> Erik Faye-Lund <kusmabite@gmail.com> writes:
> 
> > With certain combinations of author name and email lengths, git
> > format-patch does not correctly wrap the From line to be below 78
> > characters, violating rfc2047.
> 
> Didn't we have this fixed quite a while ago with the series that ends at
> c22e7de (format-patch: rfc2047-encode newlines in headers, 2011-02-23)?
> 
> What version of git do you use?

No, that doesn't fix it. The problem is a short name and a long email
address. We rfc2047-encode and wrap the name, but then tack the email
onto the end without caring about line length.

According to rfc2047, we are specifically forbidden to use an
encoded-word in an addr-spec. So it would not make sense to assemble the
header first and then feed the whole thing to add_rfc2047.

Fortunately, this is an easy thing to wrap, since we can't actually
break the addr-spec across lines. We can just check if the line is long,
and if so, wrap the whole address to the next line, which is the best we
can do (and presumably nobody has an address portion over 78
characters). That patch looks something like:

diff --git a/pretty.c b/pretty.c
index e1d8a8f..63a7c2f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -287,6 +287,7 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 	if (fmt == CMIT_FMT_EMAIL) {
 		char *name_tail = strchr(line, '<');
 		int display_name_length;
+		int final_line;
 		if (!name_tail)
 			return;
 		while (line < name_tail && isspace(name_tail[-1]))
@@ -294,6 +295,11 @@ void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 		display_name_length = name_tail - line;
 		strbuf_addstr(sb, "From: ");
 		add_rfc2047(sb, line, display_name_length, encoding);
+		for (final_line = 0; final_line < sb->len; final_line++)
+			if (sb->buf[sb->len - final_line - 1] == '\n')
+				break;
+		if (namelen - display_name_length + final_line > 78)
+			strbuf_addch(sb, '\n');
 		strbuf_add(sb, name_tail, namelen - display_name_length);
 		strbuf_addch(sb, '\n');
 	} else {

Note that it relies on the commit header having a space before the "<",
which forms the continuation whitespace for the header. This is probably
reasonable, but we could double-check if we want to handle malformed
commit headers better.

Or we could just ignore it. AFAICS, this doesn't actually violate
rfc2047, nor rfc5322. The 78-character limit is simply a SHOULD, and
we have up to 998 for MUST. For a single-address header[1], this seems
kind of unlikely to me.

-Peff

[1] For multi-address headers like "format-patch --cc=foo --cc=bar", it
looks like we already break them across lines. No idea if "send-email"
is sensible in this area or not.