Re: [PATCH] http: Support sending custom HTTP headers

[Jeff King <peff@peff.net>]

On Mon, Apr 25, 2016 at 03:13:08PM +0200, Johannes Schindelin wrote:

> diff --git a/http.c b/http.c
> index 4304b80..02d7147 100644
> --- a/http.c
> +++ b/http.c
> @@ -114,6 +114,7 @@ static unsigned long http_auth_methods = CURLAUTH_ANY;
>  
>  static struct curl_slist *pragma_header;
>  static struct curl_slist *no_pragma_header;
> +static struct curl_slist *extra_http_headers;
>  
>  static struct active_request_slot *active_queue_head;
>  
> @@ -323,6 +324,12 @@ static int http_options(const char *var, const char *value, void *cb)
>  #endif
>  	}
>  
> +	if (!strcmp("http.extraheader", var)) {
> +		extra_http_headers =
> +			curl_slist_append(extra_http_headers, value);
> +		return 0;
> +	}

I wondered if this would trigger for "http.*.extraheader", too. And it
should, as that is all handled in the caller of http_options. Good.

> @@ -678,8 +685,10 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
>  	if (remote)
>  		var_override(&http_proxy_authmethod, remote->http_proxy_authmethod);
>  
> -	pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
> -	no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
> +	pragma_header = curl_slist_append(http_get_default_headers(),
> +		"Pragma: no-cache");
> +	no_pragma_header = curl_slist_append(http_get_default_headers(),
> +		"Pragma:");

This looked wrong to me at first, because we are appending to the
default header list in each case. But the secret sauce is that calling
http_get_default_headers() actually creates a _new_ list that is a copy
of the default headers (and the caller can do what they will with it,
and must free it).

I think that's really the only sane way to do it because of curl's
interfaces. But maybe it is worth a comment either here, or along with
http_get_default_headers(), or both.

-Peff