Re: [PATCH 1/3] validate_headref: NUL-terminate HEAD buffer

On Wed, Sep 27, 2017 at 04:06:22PM +0900, Junio C Hamano wrote:

A few tangents I noticed:

 - the result of readlink should be checked with starts_with() in
   the modern codebase (#leftoverbits).

Yes, though it needs to NUL-terminate, too (readlink does not do so
automatically). Again, we seem to have left room for the NUL but didn't
bother to put it there.

 - buffer[256] would mean that we cannot have a branch whose name is
   more than a couple of hundred bytes long; as you said, we may be
   better off using strbuf_read to read the whole thing.

Heh, I almost talked about this in the cover letter, but didn't want to
go off on a tangent. But since you mention it...

I had the same concern, but actually truncation is not a problem here
(for a symlink or a symref). We are only seeing if the contents look
vaguely correct, so really we never parse past "refs/" in either case.

The real symref resolution happens in refs/files-backend.c these days,
and does use a strbuf.

-Peff