Re: [PATCH v6 11/12] http: read HTTP WWW-Authenticate response headers

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 18, 2023, 11:42 UTC
Message-ID: <230118.86y1q0vzhh.gmgdl@evledraar.gmail.com>
In-Reply-To: <bc1ac8d3eb3ac6e1161f6b6b67343874c10cd14d.1674012618.git.gitgitgadget@gmail.com>

On Wed, Jan 18 2023, Matthew John Cheetham via GitGitGadget wrote:

> From: Matthew John Cheetham <mjcheetham@outlook.com>

Show 17 quoted lines

> +	strbuf_add(&buf, ptr, size);
> +
> +	/* Strip the CRLF that should be present at the end of each field */
> +	strbuf_trim_trailing_newline(&buf);
> +
> +	/* Start of a new WWW-Authenticate header */
> +	if (skip_iprefix(buf.buf, "www-authenticate:", &val)) {
> +		while (isspace(*val))
> +			val++;
> +
> +		strvec_push(values, val);
> +		http_auth.header_is_last_match = 1;
> +		goto exit;
> [...]
> +	if (http_auth.header_is_last_match && isspace(*buf.buf)) {
> +		/* Trim leading whitespace from this continuation hdr line. */
> +		strbuf_ltrim(&buf);

The mixture of this isspace() loop and then strbuf_ltrim() seems odd, why not stick with the strbuf API?

I.e. after skip_iprefix() strbuf_splice() the start of the string away, then use strbuf_ltrim() in the first "if" branch here?

Likewise this is open-coding the "isspace" in strbuf_ltrim() for the second "if". Maybe run the strbuf_ltrim() unconditionally, save away the length before, and then:

	if (http_auth.header_is_last_match && prev_len != buf.len) { ...