Re: [PATCH v10 2/3] http: read HTTP WWW-Authenticate response headers

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

 



On 2023-02-23 11:49, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
>> Alternatively, you could copy the tolower() loop from skip_iprefix().
>> Something like:
>>   
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 28456241b6..f671a0ec3f 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -1296,17 +1296,13 @@ static inline int skip_iprefix_mem(const char *buf, size_t len,
>>  				   const char *prefix,
>>  				   const char **out, size_t *outlen)
>>  {
>> -	size_t prefix_len = strlen(prefix);
>> -
>> -	if (len < prefix_len)
>> -		return 0;
>> -
>> -	if (!strncasecmp(buf, prefix, prefix_len)) {
>> -		*out = buf + prefix_len;
>> -		*outlen = len - prefix_len;
>> -		return 1;
>> -	}
>> -
>> +	do {
>> +		if (!*prefix) {
>> +			*out = buf;
>> +			*outlen = len;
>> +			return 1;
>> +		}
>> +	} while (len-- > 0 && tolower(*buf++) == tolower(*prefix++));
>>  	return 0;
>>  }
>>  
>>
>> looks right to me, though only lightly tested (via t5563). I'm happy
>> with either implementation.
> 
> Yeah, the alternative version looks clearer to me.

Will update - thanks!

>>> +static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p)
>>> [...]
>>> +	/*
>>> +	 * If this is a HTTP status line and not a header field, this signals
>>> +	 * a different HTTP response. libcurl writes all the output of all
>>> +	 * response headers of all responses, including redirects.
>>> +	 * We only care about the last HTTP request response's headers so clear
>>> +	 * the existing array.
>>> +	 */
>>> +	if (!strncasecmp(ptr, "http/", 5))
>>> +		strvec_clear(values);
>>
>> Since "ptr" isn't NUL terminated, using strncasecmp() may walk off the
>> end. I think you'd need to check that there are five bytes. You could
>> even use skip_iprefix_mem(), though of course we'd throw away the output
>> values. (For strings there is also istarts_with(), but I don't think we
>> have a "mem" equivalent).
> 
> Yuck, thank you very much for carefully reading.  I missed this one
> when I queued it.

Oops! Will update in a v11

>> The rest of the parsing looks good to me.
> 
> Thanks.



[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