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

From

Junio C Hamano <gitster@pobox.com>

Date

Sep 15, 2014, 20:15 UTC

Message-ID

<xmqq1trc63o8.fsf@gitster.dls.corp.google.com>

In-Reply-To

<xmqqha087lwv.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> Why cache.h when this is still only between mail{info,split}.c both
> of which do not really deal with any "Git" data?
>
> For mailsplit, we are trying to detect mbox boundary various MUAs
> would use in their output, and is_from_line() may be appropriate,
> but I am not sure if the same logic is appropriate for mailinfo.
> What are we trying to protect us against?  Those who paste a single
> e-mail output from format-patch in full?  Do people paste a single
> e-mail received to their mailbox from somebody else and do we need
> to protect against that, skipping the ">From " thing, while risking
> to skip "From me at 10:30 30 minutes..."?
>
> If we only want to skip ">?From" in pasted format-patch output, we
> would want a rule in mailinfo that is tighter than is_from_line() in
> mailsplit.

 Makefile            |  1 +
 builtin/mailinfo.c  |  3 ++-
 builtin/mailsplit.c |  1 +
 cache.h             |  6 ------
 mbox.c              | 15 +++++++++++++++
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index dc5d2af..c0491a3 100644
--- a/Makefile
+++ b/Makefile
@@ -686,6 +686,7 @@ LIB_H += list-objects.h
 LIB_H += ll-merge.h
 LIB_H += log-tree.h
 LIB_H += mailmap.h
+LIB_H += mbox.h
 LIB_H += merge-blobs.h
 LIB_H += merge-recursive.h
 LIB_H += mergesort.h
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index a434d39..ccccd69 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -6,6 +6,7 @@
 #include "builtin.h"
 #include "utf8.h"
 #include "strbuf.h"
+#include "mbox.h"
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
@@ -329,7 +330,7 @@ static int check_header(const struct strbuf *line,
 
 	/* for inbody stuff */
 	if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
-		ret = is_from_line(line->buf + 1, line->len - 1);
+		ret = is_format_patch_separator(line->buf + 1, line->len - 1);
 		goto check_header_out;
 	}
 	if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 775588e..d8da1e4 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -8,6 +8,7 @@
 #include "builtin.h"
 #include "string-list.h"
 #include "strbuf.h"
+#include "mbox.h"
 
 static const char git_mailsplit_usage[] =
 "git mailsplit [-d<prec>] [-f<n>] [-b] [--keep-cr] -o<directory> [(<mbox>|<Maildir>)...]";
diff --git a/cache.h b/cache.h
index eae58e7..fcb511d 100644
--- a/cache.h
+++ b/cache.h
@@ -1502,10 +1502,4 @@ void stat_validity_update(struct stat_validity *sv, int fd);
 
 int versioncmp(const char *s1, const char *s2);
 
-/*
- * Returns true if the line appears to be an mbox "From" line starting a new
- * message.
- */
-int is_from_line(const char *line, int len);
-
 #endif /* CACHE_H */
diff --git a/mbox.c b/mbox.c
index 75f3150..2ab2f85 100644
--- a/mbox.c
+++ b/mbox.c
@@ -30,3 +30,18 @@ int is_from_line(const char *line, int len)
 	/* Ok, close enough */
 	return 1;
 }
+
+#define SAMPLE "From e6807f3efca28b30decfecb1732a56c7db1137ee Mon Sep 17 00:00:00 2001\n"
+int is_format_patch_separator(const char *line, int len)
+{
+	const char *cp;
+
+	if (len != strlen(SAMPLE))
+		return 0;
+	if (!skip_prefix(line, "From ", &cp))
+		return 0;
+	if (strspn(cp, "0123456789abcdef") != 40)
+		return 0;
+	cp += 40;
+	return !memcmp(SAMPLE + (cp - line), cp, strlen(SAMPLE) - (cp - line));
+}