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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 27, 2017, 07:06 UTC
Message-ID: <xmqqefqs4o41.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170927061722.7ubswfgykxiawzh3@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 15 quoted lines

> diff --git a/path.c b/path.c
> index b533ec938d..3e4d7505ef 100644
> --- a/path.c
> +++ b/path.c
> @@ -662,6 +662,10 @@ int validate_headref(const char *path)
>  	len = read_in_full(fd, buffer, sizeof(buffer)-1);
>  	close(fd);
>  
> +	if (len < 0)
> +		return -1;
> +	buffer[len] = '\0';
> +
>  	/*
>  	 * Is it a symbolic ref?
>  	 */

A few tangents I noticed:

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

 - 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.

Neither should be touched by this patch, of course.

Thanks.