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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2016, 18:43 UTC
Message-ID: <xmqq7fflleau.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <abe253758829795c285c2036196ebe7edd9bab34.1461589951.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> To make communication for `git fetch`, `git ls-remote` and friends extra
> secure, we introduce a way to send custom HTTP headers with all
> requests.

I think an ability to send custom headers may be a good addition and have no problem with it, but I tend to agree with Shawn that its log message that advertises it as if it has anything to do with security is probably a bad idea in both ways (i.e. it isn't very secure, and the usefulness of the feature is not limited to security).

Show 10 quoted lines

> This allows us, for example, to send an extra token that the server
> tests for. The server could use this token e.g. to ensure that only
> certain operations or refs are allowed, or allow the token to be used
> only once.
>
> This feature can be used like this:
>
> 	git -c http.extraheader='Secret: sssh!' fetch $URL $REF
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

> Published-As: https://github.com/dscho/git/releases/tag/extra-http-headers-v1

Move this after "---".

> ---

This obviously needs documentation updates and tests, no?

Show 163 quoted lines

>  http-push.c   | 10 +++++-----
>  http.c        | 28 +++++++++++++++++++++++++---
>  http.h        |  1 +
>  remote-curl.c |  4 ++--
>  4 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index bd60668..04eef17 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -211,7 +211,7 @@ static void curl_setup_http(CURL *curl, const char *url,
>  static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)
>  {
>  	struct strbuf buf = STRBUF_INIT;
> -	struct curl_slist *dav_headers = NULL;
> +	struct curl_slist *dav_headers = http_get_default_headers();
>  
>  	if (options & DAV_HEADER_IF) {
>  		strbuf_addf(&buf, "If: (<%s>)", lock->token);
> @@ -417,7 +417,7 @@ static void start_put(struct transfer_request *request)
>  static void start_move(struct transfer_request *request)
>  {
>  	struct active_request_slot *slot;
> -	struct curl_slist *dav_headers = NULL;
> +	struct curl_slist *dav_headers = http_get_default_headers();
>  
>  	slot = get_active_slot();
>  	slot->callback_func = process_response;
> @@ -845,7 +845,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)
>  	char *ep;
>  	char timeout_header[25];
>  	struct remote_lock *lock = NULL;
> -	struct curl_slist *dav_headers = NULL;
> +	struct curl_slist *dav_headers = http_get_default_headers();
>  	struct xml_ctx ctx;
>  	char *escaped;
>  
> @@ -1126,7 +1126,7 @@ static void remote_ls(const char *path, int flags,
>  	struct slot_results results;
>  	struct strbuf in_buffer = STRBUF_INIT;
>  	struct buffer out_buffer = { STRBUF_INIT, 0 };
> -	struct curl_slist *dav_headers = NULL;
> +	struct curl_slist *dav_headers = http_get_default_headers();
>  	struct xml_ctx ctx;
>  	struct remote_ls_ctx ls;
>  
> @@ -1204,7 +1204,7 @@ static int locking_available(void)
>  	struct slot_results results;
>  	struct strbuf in_buffer = STRBUF_INIT;
>  	struct buffer out_buffer = { STRBUF_INIT, 0 };
> -	struct curl_slist *dav_headers = NULL;
> +	struct curl_slist *dav_headers = http_get_default_headers();
>  	struct xml_ctx ctx;
>  	int lock_flags = 0;
>  	char *escaped;
> 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;
> +	}
> +
>  	/* Fall back on the default ones */
>  	return git_default_config(var, value, cb);
>  }
> @@ -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:");
>  
>  #ifdef USE_CURL_MULTI
>  	{
> @@ -765,6 +774,9 @@ void http_cleanup(void)
>  #endif
>  	curl_global_cleanup();
>  
> +	curl_slist_free_all(extra_http_headers);
> +	extra_http_headers = NULL;
> +
>  	curl_slist_free_all(pragma_header);
>  	pragma_header = NULL;
>  
> @@ -1163,6 +1175,16 @@ int run_one_slot(struct active_request_slot *slot,
>  	return handle_curl_result(results);
>  }
>  
> +struct curl_slist *http_get_default_headers()
> +{
> +	struct curl_slist *headers = NULL, *h;
> +
> +	for (h = extra_http_headers; h; h = h->next)
> +		headers = curl_slist_append(headers, h->data);
> +
> +	return headers;
> +}
> +
>  static CURLcode curlinfo_strbuf(CURL *curl, CURLINFO info, struct strbuf *buf)
>  {
>  	char *ptr;
> @@ -1380,7 +1402,7 @@ static int http_request(const char *url,
>  {
>  	struct active_request_slot *slot;
>  	struct slot_results results;
> -	struct curl_slist *headers = NULL;
> +	struct curl_slist *headers = http_get_default_headers();
>  	struct strbuf buf = STRBUF_INIT;
>  	const char *accept_language;
>  	int ret;
> diff --git a/http.h b/http.h
> index 4ef4bbd..b0927de 100644
> --- a/http.h
> +++ b/http.h
> @@ -106,6 +106,7 @@ extern void step_active_slots(void);
>  extern void http_init(struct remote *remote, const char *url,
>  		      int proactive_auth);
>  extern void http_cleanup(void);
> +extern struct curl_slist *http_get_default_headers();
>  
>  extern long int git_curl_ipresolve;
>  extern int active_requests;
> diff --git a/remote-curl.c b/remote-curl.c
> index 15e48e2..86ba787 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -474,7 +474,7 @@ static int run_slot(struct active_request_slot *slot,
>  static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
>  {
>  	struct active_request_slot *slot;
> -	struct curl_slist *headers = NULL;
> +	struct curl_slist *headers = http_get_default_headers();
>  	struct strbuf buf = STRBUF_INIT;
>  	int err;
>  
> @@ -503,7 +503,7 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
>  static int post_rpc(struct rpc_state *rpc)
>  {
>  	struct active_request_slot *slot;
> -	struct curl_slist *headers = NULL;
> +	struct curl_slist *headers = http_get_default_headers();
>  	int use_gzip = rpc->gzip_request;
>  	char *gzip_body = NULL;
>  	size_t gzip_size = 0;