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

From: Victoria Dye <[email-masked]>
Date: Wed, 8 Feb 2023 13:05:52 -0800

Matthew John Cheetham via GitGitGadget wrote:
> diff --git a/git-compat-util.h b/git-compat-util.h
> index a76d0526f79..f11c44517d7 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -1266,6 +1266,28 @@ static inline int skip_iprefix(const char *str, const char *prefix,
>  	return 0;
>  }
>  
> +/*
> + * Like skip_prefix_mem, but compare case-insensitively. Note that the
> + * comparison is done via tolower(), so it is strictly ASCII (no multi-byte
> + * characters or locale-specific conversions).
> + */
> +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;
> +	}
> +
> +	return 0;
> +}
> +
>  static inline int strtoul_ui(char const *s, int base, unsigned int *result)
>  {
>  	unsigned long ul;
> diff --git a/http.c b/http.c
> index 8a5ba3f4776..7a56a3db5f7 100644
> --- a/http.c
> +++ b/http.c
> @@ -183,6 +183,124 @@ size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
>  	return nmemb;
>  }
>  
> +/*
> + * A folded header continuation line starts with at least one single whitespace
> + * character. It is not a continuation line if the line is *just* a newline.
> + * The RFC for HTTP states that CRLF is the header field line ending, but some
> + * servers may use LF only; we accept both.
> + */
> +static inline int is_hdr_continuation(const char *ptr, const size_t size)
> +{
> +	/* totally empty line or normal header */
> +	if (!size || !isspace(*ptr))
> +		return 0;
> +
> +	/* empty line with LF line ending */
> +	if (size == 1 && ptr[0] == '\n')
> +		return 0;
> +
> +	/* empty line with CRLF line ending */
> +	if (size == 2 && ptr[0] == '\r' && ptr[1] == '\n')
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p)
> +{
> +	size_t size = eltsize * nmemb;
> +	struct strvec *values = &http_auth.wwwauth_headers;
> +	struct strbuf buf = STRBUF_INIT;
> +	const char *val;
> +	size_t val_len;
> +
> +	/*
> +	 * Header lines may not come NULL-terminated from libcurl so we must
> +	 * limit all scans to the maximum length of the header line, or leverage
> +	 * strbufs for all operations.
> +	 *
> +	 * In addition, it is possible that header values can be split over
> +	 * multiple lines as per RFC 2616 (even though this has since been
> +	 * deprecated in RFC 7230). A continuation header field value is
> +	 * identified as starting with a space or horizontal tab.
> +	 *
> +	 * The formal definition of a header field as given in RFC 2616 is:
> +	 *
> +	 *   message-header = field-name ":" [ field-value ]
> +	 *   field-name     = token
> +	 *   field-value    = *( field-content | LWS )
> +	 *   field-content  = <the OCTETs making up the field-value
> +	 *                    and consisting of either *TEXT or combinations
> +	 *                    of token, separators, and quoted-string>
> +	 */
> +
> +	/* Start of a new WWW-Authenticate header */
> +	if (skip_iprefix_mem(ptr, size, "www-authenticate:", &val, &val_len)) {
> +		strbuf_add(&buf, val, val_len);
> +
> +		/*
> +		 * Strip the CRLF that should be present at the end of each
> +		 * field as well as any trailing or leading whitespace from the
> +		 * value.
> +		 */
> +		strbuf_trim(&buf);
> +
> +		strvec_push(values, buf.buf);
> +		http_auth.header_is_last_match = 1;
> +		goto exit;
> +	}
> +
> +	/*
> +	 * This line could be a continuation of the previously matched header
> +	 * field. If this is the case then we should append this value to the
> +	 * end of the previously consumed value.
> +	 */
> +	if (http_auth.header_is_last_match && is_hdr_continuation(ptr, size)) {
> +		/*
> +		 * Trim the CRLF and any leading or trailing from this line.
> +		 */
> +		strbuf_add(&buf, ptr, size);
> +		strbuf_trim(&buf);
> +
> +		/*
> +		 * At this point we should always have at least one existing
> +		 * value, even if it is empty. Do not bother appending the new
> +		 * value if this continuation header is itself empty.
> +		 */
> +		if (!values->nr) {
> +			BUG("should have at least one existing header value");
> +		} else if (buf.len) {
> +			char *prev = xstrdup(values->v[values->nr - 1]);
> +
> +			/* Join two non-empty values with a single space. */
> +			const char *const sp = *prev ? " " : "";
> +
> +			strvec_pop(values);
> +			strvec_pushf(values, "%s%s%s", prev, sp, buf.buf);
> +			free(prev);
> +		}
> +
> +		goto exit;
> +	}
> +
> +	/* This is the start of a new header we don't care about */
> +	http_auth.header_is_last_match = 0;
> +
> +	/*
> +	 * 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);

I found this updated version of 'fwrite_wwwauth()' (using
'skip_iprefix_mem()', 'is_hdr_continuation()', and 'strncasecmp()') a bit
easier to read than previous iterations - possibly because all the
prefix-skipping is done before adding to 'buf', so 'buf' represents *only*
the line's header value (possibly with leading/trailing whitespace, which is
trimmed). Plus, avoiding unnecessary allocations is always nice. 

> +
> +exit:
> +	strbuf_release(&buf);
> +	return size;
> +}