Re: [PATCH 2/2] fsck: detect and warn a commit with embedded NUL

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 14, 2016, 18:37 UTC
Message-ID: <xmqqpotsf34j.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160414182544.GC22068@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> On Thu, Apr 14, 2016 at 02:21:03PM -0400, Jeff King wrote:
>
>> So yet another alternative would be to include this check in
>> verify_headers(). It would parse to the end of the headers as now, and
>> then from there additionally look for a NUL in the body.
>
> Hmm. Looking at verify_headers(), I think it has another bug.
>
> It wants to "return report(...)" as soon as it sees a problem, and stops
> looking for other ones. But the new model for reporting errors is that
> report() might ignore a problem if configured to do so.

Yup, it should be doing the "do not return early unless report says it is questionable", at least. Accumulating the error with err |= report() is an improvement, but even without that, doing the usual

	case '\0':
        	err = report(...);
                return err if err;

would be a real bugfix.