[RFC/PATCH] mailinfo: do not treat ">From" lines as in-body headers

[-cc driver-devel list, as this is getting into git patches]

On Sun, Sep 14, 2014 at 12:09:08AM +0300, Dan Carpenter wrote:

Thanks both of you for following up. I did confirm that git-send-email does not add such quoting. From your findings above, I'd agree that it's the list-archive software munging it, and they are buggy IMHO (they should de-quote on display).

Here's an RFC patch to help the "git am" side handle this case better.

-- >8 --
Subject: mailinfo: do not treat ">From" lines as in-body headers

Since commit 81c5cf7 (mailinfo: skip bogus UNIX From line inside body, 2006-05-21), we have treated lines like ">From" in the body as headers. This makes "git am" work for people who erroneously paste the whole mbox entry:

  From 12345abcd...
  From: them
  Subject: [PATCH] whatever

into their email body. However, it also causes false positives for people who really do have ">From" in the first paragraph of their commit messages. In this case, we'll drop the line completely, breaking the commit message.

We could try to make our checking more robust by doing one or both of:

  - making sure the line looks like a git "From " line
    (40-hex sha1, date, etc).

  - seeing if the following lines are actually rfc2822
    headers

However, it's probably not worth the complexity. There are a few reasons to think that this code is not actually triggered very often. One, the patch was written in 2006, when git was still relatively new, and people frequently made mistakes in sending patches; these days we not see this error much. And two, we check only the quoted ">From" form, and not the regular "From". So whether it kicks in at all depends entirely on how the mbox is saved by the user's MUA. And in the intervening 8 years, nobody has complained about the "From" case.

With this patch, we will simply treat such ">From" lines as normal body lines (and stop in-body header parsing). Note that we do _not_ unquote them into "From". Whether from-quoting is in effect depends on the exact mbox format being used, which depends on the MUA writing the file. We cannot know for sure whether to unquote or not, so we leave the line alone.

Signed-off-by: Jeff King <peff@peff.net>
---
I admit my arguments that it is not in use are a little flaky, and this
may just be me being lazy. Trying to match arbitrary "From" lines is
very hard and heuristic-filled, but if we are only trying to match
git-generated mbox lines, that's much easier. It would not hurt too much
to go that route.

I also tend to think we should unquote ">From" into "From". As discussed above, we do know whether the author meant the literal former, or meant the latter and it quoted into the former. But I'd guess that a literal ">From" is quite rare, so we'd probably serve more people by de-quoting. That is really a separate issue for another patch, though.

 builtin/mailinfo.c         |  4 ----
 t/t5100-mailinfo.sh        |  9 +++++++++
 t/t5100/quoted-from.expect |  3 +++
 t/t5100/quoted-from.in     | 10 ++++++++++
 4 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 t/t5100/quoted-from.expect
 create mode 100644 t/t5100/quoted-from.in