Re: [PATCH 1/6] fetch-pack: buffer object-format with other args

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 9, 2021, 04:49 UTC
Message-ID: <xmqqy2dsnjuf.fsf@gitster.g>
In-Reply-To: <86e4ce0e8d9665b1c5a2b30173be4afffe0a0abd.1617929278.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 9 quoted lines

> In send_fetch_request(), "object-format" is written directly to the file
> descriptor, as opposed to the other arguments, which are buffered.
> Buffer "object-format" as well. "object-format" must be buffered; in
> particular, it must appear after "command=fetch" in the request.
>
> This divergence was introduced in 4b831208bb ("fetch-pack: parse and
> advertise the object-format capability", 2020-05-27), perhaps as an
> oversight (the surrounding code at the point of this commit has already
> been using a request buffer.)

Good find. Did this actually resulted in a data corruption and that was how you discovered it?

Show 19 quoted lines

>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  fetch-pack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 6e68276aa3..2318ebe680 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1251,7 +1251,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
>  		if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
>  			die(_("mismatched algorithms: client %s; server %s"),
>  			    the_hash_algo->name, hash_name);
> -		packet_write_fmt(fd_out, "object-format=%s", the_hash_algo->name);
> +		packet_buf_write(&req_buf, "object-format=%s", the_hash_algo->name);
>  	} else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1) {
>  		die(_("the server does not support algorithm '%s'"),
>  		    the_hash_algo->name);