From: Jonathan Tan Date: Sat, 17 Sep 2016 00:22:25 GMT Subject: Re: [RFC/PATCH 3/3] mailinfo: handle in-body header continuations Message-ID: In-Reply-To: On 09/16/2016 04:04 PM, Junio C Hamano wrote: > Jonathan Tan writes: >> I'm concerned about what happens if check_header fails - we would then >> have some lines which need to be treated as log messages. (At least, >> they are currently treated that way.) > > I actually think we should refactor check_header() further so that > in-body header processing does not even see things that shouldn't be > changed. The current check_header() should be used only for real > mail headers, and then a reduced version of check_header() that is > called for in-body will _ONLY_ handle the header lines that are > handled by the first "search for the interesting parts" loop. > > And of course we would update your "does it look like rfc2822?" to > match what are handled by the "interesting parts" loop. That I > think would match the current behaviour much better, I would think. There would be a bit of code duplication in that this "does it look like rfc2822" function would also need to account for duplicate headers (e.g. 2 "Subject:" lines in the in-body headers) because check_header would reject the 2nd one, but that is minor. (Alternatively, we could just allow duplicate headers in the in-body headers.) > The ">From " and "[PATCH]" cases in check_header() should not even > be there. We should handle them inside handle_commit_msg(), as > these two cases should never appear in the real header part of a > message. > And if we clean it up like that, I do not think we would ever need > to worry about "ah, it looked like a header but it is not after > all". And not having to worry about it is a good thing and should > be one of the primary goals in this conversion, I whould think. Yes, this makes sense. I'll go ahead and make a patch set implementing this (unless anyone has any objections).