Subject: Re: [PATCH v2 1/9] bundle-uri: short-circuit capability parsing

From: Junio C Hamano <[email-masked]>
Date: Fri, 09 Sep 2022 10:24:29 -0700

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> When parsing the capability lines from the 'git remote-https' process,
> we can stop reading the lines once we notice the 'get' capability.
>
> Reported-by: Teng Long <[email-masked]>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  bundle-uri.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/bundle-uri.c b/bundle-uri.c
> index 4a8cc74ed05..7173ed065e9 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c
> @@ -56,8 +56,10 @@ static int download_https_uri_to_file(const char *file, const char *uri)
>  	while (!strbuf_getline(&line, child_out)) {
>  		if (!line.len)
>  			break;
> -		if (!strcmp(line.buf, "get"))
> +		if (!strcmp(line.buf, "get")) {
>  			found_get = 1;
> +			break;
> +		}
>  	}

Hmph, is this safe to do?  Who is feeding child_out?  Aren't they
get upset if we do not slurp what they write to us?  Are we
expecting to read more from them after this part?  Aren't we get
upset if we leave some other stuff when we read from child_out after
we saw "get"?  If we respond to child_in without reading all from
them, do we not get into a deadlock?

Perhaps these are all silly questions, but the description above
does not quite answer them.

>  	strbuf_release(&line);