Re: Crashes while trying to show tag objects with bad timestamps

From: Jeff King <peff@peff.net>
Date: Feb 22, 2013, 23:04 UTC
Message-ID: <20130222230418.GC21579@sigill.intra.peff.net>
In-Reply-To: <7vy5egark3.fsf@alter.siamese.dyndns.org>

On Fri, Feb 22, 2013 at 02:53:48PM -0800, Junio C Hamano wrote:

> > I guess we should probably issue a warning, too. Also disappointingly,
> > git-fsck does not seem to detect this breakage at all.
> 
> Yes for the warning,

Unfortunately, a good warning is harder than I had hoped. At the point where we notice the problem, pp_user_info, we have very little context. We can say only something like:

  warning: malformed date in ident 'Jeff King <peff@peff.net> BOGUS'

but we cannot say in which object, or even that it was a "tagger" line (and in some cases we do not even have an object, as in make_cover_letter).

> and no for disappointing.  IIRC, in the very early implementations
> allowed tag object without dates.
> 
> I _think_ we can start tightening fsck, though.

Then I think it would make sense to allow the very specific no-date tag, but not allow arbitrary crud. I wonder if there's an example in the kernel or in git.git.

I also took a look at parsing routine of "cat-file -p". It's totally hand-rolled, separate from what "git show" does, and is not build on the pretty-print code at all. I wonder, though, if it actually makes sense to munge the date there. The commit-object pretty-printer for cat-file just shows the object intact. It seems weirdly inconsistent that we would munge tags just to rewrite the date. If you want a real pretty-printer, you should be using porcelain like "show".

It would be a regression, of course, for people relying on "cat-file -p" to have consistent output. But I am very tempted to call it a bug, and tempted to call "cat-file -p" inside a script a bad thing (you cannot, after all, tell what object type you have; you should figure out the type you expect and then use "cat-file <type> <obj>" to make sure you get the right one).

-Peff