Subject: Re: [PATCH 36/67] remote-ext: simplify git pkt-line generation

From: Junio C Hamano <[email-masked]>
Date: Wed, 16 Sep 2015 13:18:03 -0700

Jeff King <[email-masked]> writes:

>  static void send_git_request(int stdin_fd, const char *serv, const char *repo,
>  	const char *vhost)
>  {
> -	size_t bufferspace;
> -	size_t wpos = 0;
> -	char *buffer;
> +	struct strbuf buffer = STRBUF_INIT;
>  
> -	/*
> -	 * Request needs 12 bytes extra if there is vhost (xxxx \0host=\0) and
> -	 * 6 bytes extra (xxxx \0) if there is no vhost.
> -	 */
> +	/* Generate packet with a dummy size header */
> +	strbuf_addf(&buffer, "0000%s %s%c", serv, repo, 0);
>  	if (vhost)
> -		bufferspace = strlen(serv) + strlen(repo) + strlen(vhost) + 12;
> -	else
> -		bufferspace = strlen(serv) + strlen(repo) + 6;
> +		strbuf_addf(&buffer, "host=%s%c", vhost, 0);
>  
> -	if (bufferspace > 0xFFFF)

> +	/* Now go back and fill in the size */
> +	if (buffer.len > 0xFFFF)
>  		die("Request too large to send");
> +	xsnprintf(buffer.buf, buffer.alloc, "%04x", (unsigned)buffer.len);

So we now write "0000something something\0host=something" into the buffer
and then try to overwrite the first four bytes?  Does this xsnprintf()
stop after writing the four hexadecimal, or does it clobber the first
byte of the payload (i.e. copy of serv[0]) by a NUL termination?

>  
> +	if (write_in_full(stdin_fd, buffer.buf, buffer.len) < 0)
>  		die_errno("Failed to send request");
>  
> +	strbuf_release(&buffer);
>  }
>  
>  static int run_child(const char *arg, const char *service)