On Fri, Mar 31, 2017 at 05:52:00PM -0700, Linus Torvalds wrote: > Ok, did a bisect, and this bisects to commit 6b4b013f1884 ("mailinfo: > handle in-body header continuations"). > > The continuation logic is oddly complex, and I can't follow the logic. > But it is completely broken in how it thinks empty lines are somehow > "continuations". I think the continuation logic is OK. The problem is that the newline is never fed to check_inbody_header() at all. So the next line its state machine sees is the indented line, which looks like a continuation. It seems to me that ignoring that newline is a bug, and causes other problems. For instance, if you have a blank line and then another header-looking thing (not a continuation) after, it is respected. For instance, running mailinfo on: From ...mbox header... From: Email Author <author@xxxxxxxxxxx> Subject: email subject Date: whatever From: in-body author <author@xxxxxxxxxxx> Subject: in-body subject And the rest of the message. will use "in-body subject" as the subject, though I'd have thought we should stop parsing in-body headers as soon as we see the first in-body blank line (the one after the in-body "From"). The blank line gets eaten by the check at the beginning of handle_commit_msg(), right _before_ we pass it off to the check_inbody_header() function. It seems like that should be more like: diff --git a/mailinfo.c b/mailinfo.c index a489d9d0f..6d89781eb 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -757,8 +757,10 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line) assert(!mi->filter_stage); if (mi->header_stage) { - if (!line->len || (line->len == 1 && line->buf[0] == '\n')) + if (!line->len || (line->len == 1 && line->buf[0] == '\n')) { + mi->header_stage = 0; return 0; + } } if (mi->use_inbody_headers && mi->header_stage) { But that breaks a bunch of tests. It looks like the loop in handle_body always starts by feeding the initial header-separator (the real headers, not the in-body ones) to the various parsers. So that first newline makes us trigger "no more in-body headers" before we even start parsing any lines. Oops. So doing this: @@ -960,7 +962,7 @@ static void handle_body(struct mailinfo *mi, struct strbuf *line) goto handle_body_out; } - do { + while (!strbuf_getwholeline(line, mi->input, '\n')) { /* process any boundary lines */ if (*(mi->content_top) && is_multipart_boundary(mi, line)) { /* flush any leftover */ @@ -1013,7 +1015,7 @@ static void handle_body(struct mailinfo *mi, struct strbuf *line) if (mi->input_error) break; - } while (!strbuf_getwholeline(line, mi->input, '\n')); + } flush_inbody_header_accum(mi); on top fixes that. But that still breaks a test that has blank lines at the beginning of the message, before the in-body header. So probably the state-machine needs an extra state: sucking up pre-inbody-header newlines. -Peff