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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/9/2022 1:24 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> 
>> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>>
>> 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 <dyroneteng@xxxxxxxxx>
>> Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>> ---
>>  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.

In my testing, this has not been a problem, but that does not mean
that it is safe to do. I'll drop this patch in v3.

Thanks,
-Stolee



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux