Re: [PATCH v2 1/9] bundle-uri: short-circuit capability parsing
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Sep 19, 2022, 17:55 UTC
- Message-ID
- <c40247c6-d925-af1a-4ce2-6dcec7fb748d@github.com>
- In-Reply-To
- <xmqqleqslauq.fsf@gitster.g>
On 9/9/2022 1:24 PM, Junio C Hamano wrote:
Show 37 quoted lines
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>> 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@gmail.com>
>> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
>> ---
>> 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