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

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

[Erik Faye-Lund]

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.

This happens because pp_user_info in pretty.c use add_rfc2047 (which
wraps) for the name, but strbuf_add (which does not wrap) for the
email part.

Here's a test that illustrate the problem:

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9c66367..a4b8b59 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely
long headers (rfc2047)' '
 	test_cmp expect subject
 '

+M8="foo_bar_"
+M64=$M8$M8$M8$M8$M8$M8$M8$M8
+cat >expect <<'EOF'
+From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar
+ <foobar@foo.bar>
+EOF
+test_expect_success 'format-patch wraps non-quotable headers' '
+	rm -rf patches/ &&
+	echo content >>file &&
+	git add file &&
+	git commit -mfoo --author "$M64 <foobar@foo.bar>" &&
+	git format-patch --stdout -1 >patch &&
+	sed -n "/^From: /p; /^ /p; /^$/q" <patch >from &&
+	test_cmp expect from
+'
 test_done

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

[Junio C Hamano]

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?

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

[Jeff King]

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.

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

[Jeff King]

On Thu, Apr 14, 2011 at 07:01:41PM +0200, Erik Faye-Lund wrote:

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 9c66367..a4b8b59 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely
> long headers (rfc2047)' '

Speaking of wrapping, your MUA seems to have mangled the patch.

> +M8="foo_bar_"
> +M64=$M8$M8$M8$M8$M8$M8$M8$M8
> +cat >expect <<'EOF'
> +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar
> + <foobar@foo.bar>
> +EOF

Your expect data is missing the trailing "_". You could probably just
do:

  cat >expect <<EOF
  From: $M64
   <foobar@foo.bar>
  EOF

which is even simpler.

-Peff

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

[Erik Faye-Lund]

On Thu, Apr 14, 2011 at 7:52 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Apr 14, 2011 at 07:01:41PM +0200, Erik Faye-Lund wrote:
>
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index 9c66367..a4b8b59 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely
>> long headers (rfc2047)' '
>
> Speaking of wrapping, your MUA seems to have mangled the patch.
>
>> +M8="foo_bar_"
>> +M64=$M8$M8$M8$M8$M8$M8$M8$M8
>> +cat >expect <<'EOF'
>> +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar
>> + <foobar@foo.bar>
>> +EOF
>
> Your expect data is missing the trailing "_". You could probably just
> do:
>
>  cat >expect <<EOF
>  From: $M64
>   <foobar@foo.bar>
>  EOF
>
> which is even simpler.
>
> -Peff
>

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

[Erik Faye-Lund]

On Thu, Apr 14, 2011 at 7:52 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Apr 14, 2011 at 07:01:41PM +0200, Erik Faye-Lund wrote:
>
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index 9c66367..a4b8b59 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely
>> long headers (rfc2047)' '
>
> Speaking of wrapping, your MUA seems to have mangled the patch.
>

Hehe, yes. How ironic :)

What can I say, silly GMail? :P

>> +M8="foo_bar_"
>> +M64=$M8$M8$M8$M8$M8$M8$M8$M8
>> +cat >expect <<'EOF'
>> +From: foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar_foo_bar
>> + <foobar@foo.bar>
>> +EOF
>
> Your expect data is missing the trailing "_". You could probably just
> do:
>
>  cat >expect <<EOF
>  From: $M64
>   <foobar@foo.bar>
>  EOF
>
> which is even simpler.
>

Yes. Much better, thanks.

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

[Erik Faye-Lund]

On Thu, Apr 14, 2011 at 7:50 PM, Jeff King <peff@peff.net> wrote:
> 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?

I believe it was straight from todays master, but I'm not still at that machine.

> 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 {
>

Yes, this is the reasonable fix. Basically copying the wrapping logic
from add_rfc2047...

> 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.

I think that's a reasonable assumption; this field comes straight from
the commit. It should already be well-formed, no?

> 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.

True. But since the fix is as simple as it is, perhaps it's worth it
just for the clean conscience?

Don't get me wrong, this was just something I stumbled over. I don't
have any real-world case where it breaks.

> [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it
> looks like we already break them across lines.

Yes, but this is even worse: these fields don't get encoded at all!

> No idea if "send-email"
> is sensible in this area or not.

...but luckily send-email is sensible. The first thing it does is to
unfold each header line. Addresses are rfc2047-decoded and recoded (if
needed, utf-8 is assumed as the encoding if none is found), and To/Cc
headers are laid out one address per line.

This still produce invalid To/Cc headers if send-email isn't used, though.

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

[Jeff King]

On Thu, Apr 14, 2011 at 11:19:09PM +0200, Erik Faye-Lund wrote:

> > 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.
> 
> I think that's a reasonable assumption; this field comes straight from
> the commit. It should already be well-formed, no?

Probably; it was just something I noted while writing the patch, and I
like to be paranoid about assumptions. :)

> > 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.
> 
> True. But since the fix is as simple as it is, perhaps it's worth it
> just for the clean conscience?

Fair enough. Patch to follow.

> > [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it
> > looks like we already break them across lines.
> 
> Yes, but this is even worse: these fields don't get encoded at all!

Ugh, you're right. That is a totally separate issue, and one I really
don't want to get into. Because it means we have to _parse_ those
headers and understand which part is a name and which is an address.

People who use "--cc" or format.headers will have to deal with that
themselves. I consider both to be somewhat useless, since you can
post-process the mbox after format-patch is run (or in your MUA).
Whereas quoting and encoding fields in format-patch is necessary to give
unambiguous input to the MUA (be it send-email or whatever).

-Peff

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

[Jeff King]

On Thu, Apr 14, 2011 at 05:42:30PM -0400, Jeff King wrote:

> > > 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.
> > 
> > True. But since the fix is as simple as it is, perhaps it's worth it
> > just for the clean conscience?
> 
> Fair enough. Patch to follow.

Here it is.

-- >8 --
Subject: [PATCH] format-patch: wrap email addresses after long names

We already wrap names in "from" headers, which tend to be
the long part of an address. But it's also possible for a
long name to not be wrapped, but to make us want to wrap the
email address. For example (imagine for the sake of
readability we want to wrap at 50 characters instead of 78):

  From: this is my really long git name <foo@example.com>

The name does not overflow the line, but the name and email
together do. So we would rather see:

  From: this is my really long git name
    <git@example.com>

Because we wrap the name separately during add_rfc2047, we
neglected this case. Instead, we should see how long the
final line of the wrapped name ended up, and decide whether
or not to wrap based on that. We can't break the address
into multiple parts, so we either leave it with the name, or
put it by itself on a line.

Test by Erik Faye-Lund.

Signed-off-by: Jeff King <peff@peff.net>
---
 pretty.c                |    9 +++++++++
 t/t4014-format-patch.sh |   15 +++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/pretty.c b/pretty.c
index e1d8a8f..ba95de9 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,14 @@ 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');
+			if (!isspace(name_tail[0]))
+				strbuf_addch(sb, ' ');
+		}
 		strbuf_add(sb, name_tail, namelen - display_name_length);
 		strbuf_addch(sb, '\n');
 	} else {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index c3cdb52..7d06716 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -793,4 +793,19 @@ test_expect_success 'format-patch wraps extremely long headers (rfc2047)' '
 	test_cmp expect subject
 '
 
+M8="foo_bar_"
+M64=$M8$M8$M8$M8$M8$M8$M8$M8
+cat >expect <<EOF
+From: $M64
+ <foobar@foo.bar>
+EOF
+test_expect_success 'format-patch wraps non-quotable headers' '
+	rm -rf patches/ &&
+	echo content >>file &&
+	git add file &&
+	git commit -mfoo --author "$M64 <foobar@foo.bar>" &&
+	git format-patch --stdout -1 >patch &&
+	sed -n "/^From: /p; /^ /p; /^$/q" <patch >from &&
+	test_cmp expect from
+'
 test_done
-- 
1.7.5.rc1.3.ga78bf

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

[Erik Faye-Lund]

On Thu, Apr 14, 2011 at 11:42 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Apr 14, 2011 at 11:19:09PM +0200, Erik Faye-Lund wrote:
>> > 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.
>>
>> True. But since the fix is as simple as it is, perhaps it's worth it
>> just for the clean conscience?
>
> Fair enough. Patch to follow.
>

Thinking about it a bit more, I'm getting a bit more unsure:
- The 78-limit is about user-interfaces, not protocol robustness.
- Since send-email unwraps the line and does not re-wrap it, even if
we have a name like this it's likely that the work gets undone right
away.
- So that means that send-email should probably also be fixed. But now
I'm wondering if we've crossed the point where this will just lead to
less obvious code for very little gain.

So I think I might have over-though this.

>> > [1] For multi-address headers like "format-patch --cc=foo --cc=bar", it
>> > looks like we already break them across lines.
>>
>> Yes, but this is even worse: these fields don't get encoded at all!
>
> Ugh, you're right. That is a totally separate issue, and one I really
> don't want to get into.

Indeed. I have an itch around this area (I've been playing with
porting send-email to C), so I might look at it at some point soon.

> Because it means we have to _parse_ those
> headers and understand which part is a name and which is an address.
>

That part is surprisingly easy: If it contains a '<', then it's on the form
"Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's
UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most
sense to me)

> People who use "--cc" or format.headers will have to deal with that
> themselves. I consider both to be somewhat useless, since you can
> post-process the mbox after format-patch is run (or in your MUA).
> Whereas quoting and encoding fields in format-patch is necessary to give
> unambiguous input to the MUA (be it send-email or whatever).
>

I agree. I'm actually a tad surprised we support it in the first place.

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

[Jeff King]

On Fri, Apr 15, 2011 at 12:21:24AM +0200, Erik Faye-Lund wrote:

> >> True. But since the fix is as simple as it is, perhaps it's worth it
> >> just for the clean conscience?
> >
> > Fair enough. Patch to follow.
> >
> 
> Thinking about it a bit more, I'm getting a bit more unsure:
> - The 78-limit is about user-interfaces, not protocol robustness.

True. In theory we should also be limiting to avoid the 998-character
hard protocol limit, but that is getting ridiculously unlikely.

> - Since send-email unwraps the line and does not re-wrap it, even if
> we have a name like this it's likely that the work gets undone right
> away.

Not everybody uses send-email. So you are also helping MUAs which
consume the output of format-patch.

That being said, I doubt that this will make a difference to anybody.
The real reason that we put wrapping into add_rfc2047 was for subjects,
which _do_ get long.

> - So that means that send-email should probably also be fixed. But now
> I'm wondering if we've crossed the point where this will just lead to
> less obvious code for very little gain.

It is ugly code.

I'm just as happy if we drop it.

> > Because it means we have to _parse_ those
> > headers and understand which part is a name and which is an address.
> 
> That part is surprisingly easy: If it contains a '<', then it's on the form
> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's
> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most
> sense to me)

What about:

  "Foo \"The Bar\" Baz" <foo@example.com>

or

  Foo "The Bar" Baz <foo@example.com>

or

  Foo (The Bar) Baz <foo@example.com>

I.e., are we taking rfc822-style addresses, or are we taking something
that looks vaguely like an email address, and just treating everything
left of "<" as literal?

-Peff

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

[Erik Faye-Lund]

On Fri, Apr 15, 2011 at 12:29 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Apr 15, 2011 at 12:21:24AM +0200, Erik Faye-Lund wrote:
>
>> >> True. But since the fix is as simple as it is, perhaps it's worth it
>> >> just for the clean conscience?
>> >
>> > Fair enough. Patch to follow.
>> >
>>
>> Thinking about it a bit more, I'm getting a bit more unsure:
>> - The 78-limit is about user-interfaces, not protocol robustness.
>
> True. In theory we should also be limiting to avoid the 998-character
> hard protocol limit, but that is getting ridiculously unlikely.

I think that's over in my definition of "insanity land", yeah :)

>> - Since send-email unwraps the line and does not re-wrap it, even if
>> we have a name like this it's likely that the work gets undone right
>> away.
>
> Not everybody uses send-email. So you are also helping MUAs which
> consume the output of format-patch.

Good point.

> That being said, I doubt that this will make a difference to anybody.
> The real reason that we put wrapping into add_rfc2047 was for subjects,
> which _do_ get long.

Absolutely.

>> - So that means that send-email should probably also be fixed. But now
>> I'm wondering if we've crossed the point where this will just lead to
>> less obvious code for very little gain.
>
> It is ugly code.
>
> I'm just as happy if we drop it.

OK, then I'll try to forget about this issue for now. Sorry for troubling you.

>> > Because it means we have to _parse_ those
>> > headers and understand which part is a name and which is an address.
>>
>> That part is surprisingly easy: If it contains a '<', then it's on the form
>> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's
>> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most
>> sense to me)
>
> What about:
>
>  "Foo \"The Bar\" Baz" <foo@example.com>
>
> or
>
>  Foo "The Bar" Baz <foo@example.com>
>
> or
>
>  Foo (The Bar) Baz <foo@example.com>
>
> I.e., are we taking rfc822-style addresses, or are we taking something
> that looks vaguely like an email address, and just treating everything
> left of "<" as literal?

I was just thinking of interpreting everything left of '<' literally
and encode it (if needed). Currently, we interpret the entire string
literally, encoding the name would an improvement.

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

[Jeff King]

On Fri, Apr 15, 2011 at 12:43:08AM +0200, Erik Faye-Lund wrote:

> >> That part is surprisingly easy: If it contains a '<', then it's on the form
> >> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's
> >> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most
> >> sense to me)
> >
> > What about:
> >
> >  "Foo \"The Bar\" Baz" <foo@example.com>
> >
> > or
> >
> >  Foo "The Bar" Baz <foo@example.com>
> >
> > or
> >
> >  Foo (The Bar) Baz <foo@example.com>
> >
> > I.e., are we taking rfc822-style addresses, or are we taking something
> > that looks vaguely like an email address, and just treating everything
> > left of "<" as literal?
> 
> I was just thinking of interpreting everything left of '<' literally
> and encode it (if needed). Currently, we interpret the entire string
> literally, encoding the name would an improvement.

Won't that be a regression for people who already know that we take
things literally and are manually quoting and/or rfc2047-encoding the
contents?

-Peff

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

[Erik Faye-Lund]

On Fri, Apr 15, 2011 at 5:30 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Apr 15, 2011 at 12:43:08AM +0200, Erik Faye-Lund wrote:
>
>> >> That part is surprisingly easy: If it contains a '<', then it's on the form
>> >> "Foo Bar Baz <foo@bar.baz>". If not, it's "foo@bar.baz" (assuming it's
>> >> UTF-8 encoded rfc5322 mailbox'es we assume, which would make the most
>> >> sense to me)
>> >
>> > What about:
>> >
>> >  "Foo \"The Bar\" Baz" <foo@example.com>
>> >
>> > or
>> >
>> >  Foo "The Bar" Baz <foo@example.com>
>> >
>> > or
>> >
>> >  Foo (The Bar) Baz <foo@example.com>
>> >
>> > I.e., are we taking rfc822-style addresses, or are we taking something
>> > that looks vaguely like an email address, and just treating everything
>> > left of "<" as literal?
>>
>> I was just thinking of interpreting everything left of '<' literally
>> and encode it (if needed). Currently, we interpret the entire string
>> literally, encoding the name would an improvement.
>
> Won't that be a regression for people who already know that we take
> things literally and are manually quoting and/or rfc2047-encoding the
> contents?

Yes. But won't that always be the case when someone depends on buggy behavior?

Besides, send-email takes interprets it's --to and --cc arguments as
well as sendemail.to and sendemail.cc config options literally (i.e
quoting if needed without any attempts on unquoting first). IMO having
two closely related programs with similar options that behave
different in border-cases is pretty ugly. ESPECIALLY when one of them
has a habit of forwarding unknown options to the other, like
send-email does...

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

[Jeff King]

On Fri, Apr 15, 2011 at 10:32:27AM +0200, Erik Faye-Lund wrote:

> >> I was just thinking of interpreting everything left of '<' literally
> >> and encode it (if needed). Currently, we interpret the entire string
> >> literally, encoding the name would an improvement.
> >
> > Won't that be a regression for people who already know that we take
> > things literally and are manually quoting and/or rfc2047-encoding the
> > contents?
> 
> Yes. But won't that always be the case when someone depends on buggy behavior?

I guess I don't see the current behavior as necessarily buggy, just
sub-optimal. I can imagine people have worked around it by embedding
rfc2047-encoded content manually.

But I admit I don't really care that much, and I don't know what common
use is. Grepping the list archives didn't turn up anything useful.

> Besides, send-email takes interprets it's --to and --cc arguments as
> well as sendemail.to and sendemail.cc config options literally (i.e
> quoting if needed without any attempts on unquoting first). IMO having
> two closely related programs with similar options that behave
> different in border-cases is pretty ugly. ESPECIALLY when one of them
> has a habit of forwarding unknown options to the other, like
> send-email does...

Yeah, it would be nice to resolve that inconsistency.

-Peff