On 9/13/2022 3:25 PM, Matthew John Cheetham via GitGitGadget wrote:
> + /**
> + * A `strvec` of WWW-Authenticate header values. Each string
> + * is the value of a WWW-Authenticate header in an HTTP response,
> + * in the order they were received in the response.
> + */
> + struct strvec wwwauth_headers;
I like this careful documentation.
> + unsigned header_is_last_match:1;
But then this member is unclear how it is attached. It could use its
own "for internal use" comment if we don't want to describe it in full
detail here.
> +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;
> + const char *z = NULL;
> +
> + /*
> + * 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>
> + */
> +
> + strbuf_add(&buf, ptr, size);
> +
> + /* Strip the CRLF that should be present at the end of each field */
Is it really a CRLF? Or just an LF?
> + strbuf_trim_trailing_newline(&buf);
Thankfully, this will trim an LF _or_ CR/LF pair, so either way would be fine.
> + /* Start of a new WWW-Authenticate header */
> + if (skip_iprefix(buf.buf, "www-authenticate:", &val)) {
> + while (isspace(*val)) val++;
Break the "val++;" to its own line:
while (isspace(*val))
val++;
While we are here, do we need to be careful about the end of the string at
this point? Is it possible that the server will send all spaces up until the
maximum header size (as mentioned in the message)?
> +
> + strvec_push(values, val);
> + 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 && isspace(*buf.buf)) {
> + const char **v = values->v + values->nr - 1;
I suppose we expect leading spaces as critical to this header, right?
> + char *append = xstrfmt("%s%.*s", *v, (int)(size - 1), ptr + 1);
We might have better luck using a strbuf, initializing it with the expected
size and using strbuf_add() to append the strings. Maybe I'm just prematurely
optimizing, though.
> +
> + free((void*)*v);
> + *v = append;
> +
> + 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 (skip_iprefix(buf.buf, "http/", &z))
> + strvec_clear(values);
> +
> +exit:
> + strbuf_release(&buf);
> + return size;
> +}
> +
> size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf)
> {
> return nmemb;
> @@ -1829,6 +1904,8 @@ static int http_request(const char *url,
> fwrite_buffer);
> }
>
> + curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_wwwauth);
Nice integration point!
Thanks,
-Stolee
