On Wed, Nov 18, 2009 at 10:51:54AM -0500, Jeff King wrote:
> So the problem is slightly less severe; the body of your commit message
> has to _start_ with "From:". Still, it is awfully ugly to hit a parsing
> ambiguity like this when you are trying to do something as simple as
> rebase.
>
> Some solutions I can think of are:
>
> 1. Improve the header-finding heuristic to actually look for something
> more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
> head which other headers we handle in this position. Probably
> Date, too).
>
> 2. Give mailinfo a "--strict" mode to indicate that it is directly
> parsing the output of format-patch, and not some random email. Use
> --strict when invoking "git am" via "git rebase".
Solution (2) seemed like a lot of work, so here is the relatively small
solution (1). I think looking for <.*@.*> is too restrictive, as people
may be using:
From: bare@xxxxxxxxxxx
which should remain valid. I just look for an '@' instead.
Note that this validation also applies to actual headers. Should we turn
it off for them? As it is, it breaks t3400, which uses a bogus email
address. I suppose we should probably preserve such bogosities if they
are in the official headers.
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index c90cd31..6d69ef3 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -275,6 +275,17 @@ static inline int cmp_header(const struct strbuf *line, const char *hdr)
line->buf[len] == ':' && isspace(line->buf[len + 1]);
}
+static int validate_header(const char *header, const struct strbuf *data)
+{
+ if (!strcmp(header, "From"))
+ return !!strchr(data->buf, '@');
+ if (!strcmp(header, "Date")) {
+ char buf[50];
+ return parse_date(data->buf, buf, sizeof(buf)) >= 0;
+ }
+ return 1;
+}
+
static int check_header(const struct strbuf *line,
struct strbuf *hdr_data[], int overwrite)
{
@@ -289,8 +300,10 @@ static int check_header(const struct strbuf *line,
*/
strbuf_add(&sb, line->buf + len + 2, line->len - len - 2);
decode_header(&sb);
- handle_header(&hdr_data[i], &sb);
- ret = 1;
+ if (validate_header(header[i], &sb)) {
+ ret = 1;
+ handle_header(&hdr_data[i], &sb);
+ }
goto check_header_out;
}
}
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 0279d07..be06e0f 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
- test `cat last` = 14'
+ test `cat last` = 16'
check_mailinfo () {
mail=$1 opt=$2
diff --git a/t/t5100/info0015 b/t/t5100/info0015
new file mode 100644
index 0000000..c4d8d77
--- /dev/null
+++ b/t/t5100/info0015
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@xxxxxxxxxxx
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0016 b/t/t5100/info0016
new file mode 100644
index 0000000..f4857d4
--- /dev/null
+++ b/t/t5100/info0016
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@xxxxxxxxxxx
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
new file mode 100644
index 0000000..be5115b
--- /dev/null
+++ b/t/t5100/msg0015
@@ -0,0 +1,3 @@
+From: bogosity
+ - a list
+ - of stuff
diff --git a/t/t5100/msg0016 b/t/t5100/msg0016
new file mode 100644
index 0000000..1063f51
--- /dev/null
+++ b/t/t5100/msg0016
@@ -0,0 +1,4 @@
+Date: bogus
+
+and some content
+
diff --git a/t/t5100/patch0015 b/t/t5100/patch0015
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016 b/t/t5100/patch0016
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 13fa4ae..de10312 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -650,3 +650,36 @@ index b0b5d8f..461c47e 100644
convert_to_utf8(line, charset.buf);
--
1.6.4.1
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@xxxxxxxxxxx>
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: bogosity
+ - a list
+ - of stuff
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@xxxxxxxxxxx>
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+Date: bogus
+
+and some content
+
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html