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

From: Junio C Hamano <[email-masked]>
Date: Thu, 23 Feb 2023 11:49:26 -0800

Jeff King <[email-masked]> 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.

>> +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.

> The rest of the parsing looks good to me.

Thanks.