From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 15 Sep 2016 21:11:15 +0000 (-0700)
Subject: Merge branch 'jk/fix-remote-curl-url-wo-proto'
X-Git-Tag: v2.11.0-rc0~149
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/c13f458d869fefb603c63ae7bf65fe3d595a8a83?ds=inline;hp=-c

Merge branch 'jk/fix-remote-curl-url-wo-proto'

"git fetch http::/site/path" did not die correctly and segfaulted
instead.

* jk/fix-remote-curl-url-wo-proto:
remote-curl: handle URLs without protocol
---

c13f458d869fefb603c63ae7bf65fe3d595a8a83
diff --combined http.c
index cd40b012f8,057f250b9c..edce47ca75
--- a/http.c
+++ b/http.c
@@@ -11,7 -11,6 +11,7 @@@
  #include "gettext.h"
  #include "transport.h"
  
 +static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
  #if LIBCURL_VERSION_NUM >= 0x070a08
  long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
  #else
@@@ -115,7 -114,6 +115,7 @@@ static unsigned long http_auth_methods 
  
  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;
  
@@@ -325,19 -323,6 +325,19 @@@ static int http_options(const char *var
  #endif
  	}
  
 +	if (!strcmp("http.extraheader", var)) {
 +		if (!value) {
 +			return config_error_nonbool(var);
 +		} else if (!*value) {
 +			curl_slist_free_all(extra_http_headers);
 +			extra_http_headers = NULL;
 +		} else {
 +			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);
  }
@@@ -461,7 -446,8 +461,7 @@@ static int sockopt_callback(void *clien
  
  	rc = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&ka, len);
  	if (rc < 0)
 -		warning("unable to set SO_KEEPALIVE on socket %s",
 -			strerror(errno));
 +		warning_errno("unable to set SO_KEEPALIVE on socket");
  
  	return 0; /* CURL_SOCKOPT_OK only exists since curl 7.21.5 */
  }
@@@ -478,125 -464,6 +478,125 @@@ static void set_curl_keepalive(CURL *c
  }
  #endif
  
 +static void redact_sensitive_header(struct strbuf *header)
 +{
 +	const char *sensitive_header;
 +
 +	if (skip_prefix(header->buf, "Authorization:", &sensitive_header) ||
 +	    skip_prefix(header->buf, "Proxy-Authorization:", &sensitive_header)) {
 +		/* The first token is the type, which is OK to log */
 +		while (isspace(*sensitive_header))
 +			sensitive_header++;
 +		while (*sensitive_header && !isspace(*sensitive_header))
 +			sensitive_header++;
 +		/* Everything else is opaque and possibly sensitive */
 +		strbuf_setlen(header,  sensitive_header - header->buf);
 +		strbuf_addstr(header, " <redacted>");
 +	}
 +}
 +
 +static void curl_dump_header(const char *text, unsigned char *ptr, size_t size, int hide_sensitive_header)
 +{
 +	struct strbuf out = STRBUF_INIT;
 +	struct strbuf **headers, **header;
 +
 +	strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
 +		text, (long)size, (long)size);
 +	trace_strbuf(&trace_curl, &out);
 +	strbuf_reset(&out);
 +	strbuf_add(&out, ptr, size);
 +	headers = strbuf_split_max(&out, '\n', 0);
 +
 +	for (header = headers; *header; header++) {
 +		if (hide_sensitive_header)
 +			redact_sensitive_header(*header);
 +		strbuf_insert((*header), 0, text, strlen(text));
 +		strbuf_insert((*header), strlen(text), ": ", 2);
 +		strbuf_rtrim((*header));
 +		strbuf_addch((*header), '\n');
 +		trace_strbuf(&trace_curl, (*header));
 +	}
 +	strbuf_list_free(headers);
 +	strbuf_release(&out);
 +}
 +
 +static void curl_dump_data(const char *text, unsigned char *ptr, size_t size)
 +{
 +	size_t i;
 +	struct strbuf out = STRBUF_INIT;
 +	unsigned int width = 60;
 +
 +	strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
 +		text, (long)size, (long)size);
 +	trace_strbuf(&trace_curl, &out);
 +
 +	for (i = 0; i < size; i += width) {
 +		size_t w;
 +
 +		strbuf_reset(&out);
 +		strbuf_addf(&out, "%s: ", text);
 +		for (w = 0; (w < width) && (i + w < size); w++) {
 +			unsigned char ch = ptr[i + w];
 +
 +			strbuf_addch(&out,
 +				       (ch >= 0x20) && (ch < 0x80)
 +				       ? ch : '.');
 +		}
 +		strbuf_addch(&out, '\n');
 +		trace_strbuf(&trace_curl, &out);
 +	}
 +	strbuf_release(&out);
 +}
 +
 +static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp)
 +{
 +	const char *text;
 +	enum { NO_FILTER = 0, DO_FILTER = 1 };
 +
 +	switch (type) {
 +	case CURLINFO_TEXT:
 +		trace_printf_key(&trace_curl, "== Info: %s", data);
 +	default:		/* we ignore unknown types by default */
 +		return 0;
 +
 +	case CURLINFO_HEADER_OUT:
 +		text = "=> Send header";
 +		curl_dump_header(text, (unsigned char *)data, size, DO_FILTER);
 +		break;
 +	case CURLINFO_DATA_OUT:
 +		text = "=> Send data";
 +		curl_dump_data(text, (unsigned char *)data, size);
 +		break;
 +	case CURLINFO_SSL_DATA_OUT:
 +		text = "=> Send SSL data";
 +		curl_dump_data(text, (unsigned char *)data, size);
 +		break;
 +	case CURLINFO_HEADER_IN:
 +		text = "<= Recv header";
 +		curl_dump_header(text, (unsigned char *)data, size, NO_FILTER);
 +		break;
 +	case CURLINFO_DATA_IN:
 +		text = "<= Recv data";
 +		curl_dump_data(text, (unsigned char *)data, size);
 +		break;
 +	case CURLINFO_SSL_DATA_IN:
 +		text = "<= Recv SSL data";
 +		curl_dump_data(text, (unsigned char *)data, size);
 +		break;
 +	}
 +	return 0;
 +}
 +
 +void setup_curl_trace(CURL *handle)
 +{
 +	if (!trace_want(&trace_curl))
 +		return;
 +	curl_easy_setopt(handle, CURLOPT_VERBOSE, 1L);
 +	curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
 +	curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
 +}
 +
 +
  static CURL *get_curl_handle(void)
  {
  	CURL *result = curl_easy_init();
@@@ -695,9 -562,9 +695,9 @@@
  		warning("protocol restrictions not applied to curl redirects because\n"
  			"your curl version is too old (>= 7.19.4)");
  #endif
 -
  	if (getenv("GIT_CURL_VERBOSE"))
 -		curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
 +		curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
 +	setup_curl_trace(result);
  
  	curl_easy_setopt(result, CURLOPT_USERAGENT,
  		user_agent ? user_agent : git_user_agent());
@@@ -723,7 -590,7 +723,7 @@@
  	 * precedence here, as in CURL.
  	 */
  	if (!curl_http_proxy) {
- 		if (!strcmp(http_auth.protocol, "https")) {
+ 		if (http_auth.protocol && !strcmp(http_auth.protocol, "https")) {
  			var_override(&curl_http_proxy, getenv("HTTPS_PROXY"));
  			var_override(&curl_http_proxy, getenv("https_proxy"));
  		} else {
@@@ -811,10 -678,8 +811,10 @@@ void http_init(struct remote *remote, c
  	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_copy_default_headers(),
 +		"Pragma: no-cache");
 +	no_pragma_header = curl_slist_append(http_copy_default_headers(),
 +		"Pragma:");
  
  #ifdef USE_CURL_MULTI
  	{
@@@ -900,9 -765,6 +900,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;
  
@@@ -1225,7 -1087,7 +1225,7 @@@ void append_remote_object_url(struct st
  
  	strbuf_addf(buf, "objects/%.*s/", 2, hex);
  	if (!only_two_digit_prefix)
 -		strbuf_addf(buf, "%s", hex+2);
 +		strbuf_addstr(buf, hex + 2);
  }
  
  char *get_remote_object_url(const char *url, const char *hex,
@@@ -1301,16 -1163,6 +1301,16 @@@ int run_one_slot(struct active_request_
  	return handle_curl_result(results);
  }
  
 +struct curl_slist *http_copy_default_headers(void)
 +{
 +	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;
@@@ -1528,7 -1380,7 +1528,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_copy_default_headers();
  	struct strbuf buf = STRBUF_INIT;
  	const char *accept_language;
  	int ret;
@@@ -1975,19 -1827,8 +1975,19 @@@ static size_t fwrite_sha1_file(char *pt
  	unsigned char expn[4096];
  	size_t size = eltsize * nmemb;
  	int posn = 0;
 -	struct http_object_request *freq =
 -		(struct http_object_request *)data;
 +	struct http_object_request *freq = data;
 +	struct active_request_slot *slot = freq->slot;
 +
 +	if (slot) {
 +		CURLcode c = curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE,
 +						&slot->http_code);
 +		if (c != CURLE_OK)
 +			die("BUG: curl_easy_getinfo for HTTP code failed: %s",
 +				curl_easy_strerror(c));
 +		if (slot->http_code >= 400)
 +			return size;
 +	}
 +
  	do {
  		ssize_t retval = xwrite(freq->localfile,
  					(char *) ptr + posn, size - posn);
@@@ -2053,7 -1894,8 +2053,7 @@@ struct http_object_request *new_http_ob
  	}
  
  	if (freq->localfile < 0) {
 -		error("Couldn't create temporary file %s: %s",
 -		      freq->tmpfile, strerror(errno));
 +		error_errno("Couldn't create temporary file %s", freq->tmpfile);
  		goto abort;
  	}
  
@@@ -2098,8 -1940,8 +2098,8 @@@
  			prev_posn = 0;
  			lseek(freq->localfile, 0, SEEK_SET);
  			if (ftruncate(freq->localfile, 0) < 0) {
 -				error("Couldn't truncate temporary file %s: %s",
 -					  freq->tmpfile, strerror(errno));
 +				error_errno("Couldn't truncate temporary file %s",
 +					    freq->tmpfile);
  				goto abort;
  			}
  		}
@@@ -2108,7 -1950,6 +2108,7 @@@
  	freq->slot = get_active_slot();
  
  	curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);
 +	curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
  	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
  	curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
  	curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
diff --combined t/t5550-http-fetch-dumb.sh
index dc9b87d6b4,9249140f99..7641417b4a
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@@ -91,55 -91,6 +91,55 @@@ test_expect_success 'configured usernam
  	expect_askpass pass user@host
  '
  
 +test_expect_success 'set up repo with http submodules' '
 +	git init super &&
 +	set_askpass user@host pass@host &&
 +	(
 +		cd super &&
 +		git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
 +		git commit -m "add submodule"
 +	)
 +'
 +
 +test_expect_success 'cmdline credential config passes to submodule via clone' '
 +	set_askpass wrong pass@host &&
 +	test_must_fail git clone --recursive super super-clone &&
 +	rm -rf super-clone &&
 +
 +	set_askpass wrong pass@host &&
 +	git -c "credential.$HTTPD_URL.username=user@host" \
 +		clone --recursive super super-clone &&
 +	expect_askpass pass user@host
 +'
 +
 +test_expect_success 'cmdline credential config passes submodule via fetch' '
 +	set_askpass wrong pass@host &&
 +	test_must_fail git -C super-clone fetch --recurse-submodules &&
 +
 +	set_askpass wrong pass@host &&
 +	git -C super-clone \
 +	    -c "credential.$HTTPD_URL.username=user@host" \
 +	    fetch --recurse-submodules &&
 +	expect_askpass pass user@host
 +'
 +
 +test_expect_success 'cmdline credential config passes submodule update' '
 +	# advance the submodule HEAD so that a fetch is required
 +	git commit --allow-empty -m foo &&
 +	git push "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git" HEAD &&
 +	sha1=$(git rev-parse HEAD) &&
 +	git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&
 +
 +	set_askpass wrong pass@host &&
 +	test_must_fail git -C super-clone submodule update &&
 +
 +	set_askpass wrong pass@host &&
 +	git -C super-clone \
 +	    -c "credential.$HTTPD_URL.username=user@host" \
 +	    submodule update &&
 +	expect_askpass pass user@host
 +'
 +
  test_expect_success 'fetch changes via http' '
  	echo content >>file &&
  	git commit -a -m two &&
@@@ -263,15 -214,15 +263,15 @@@ check_language () 
  		>expect
  		;;
  	?*)
 -		echo "Accept-Language: $1" >expect
 +		echo "=> Send header: Accept-Language: $1" >expect
  		;;
  	esac &&
 -	GIT_CURL_VERBOSE=1 \
 +	GIT_TRACE_CURL=true \
  	LANGUAGE=$2 \
  	git ls-remote "$HTTPD_URL/dumb/repo.git" >output 2>&1 &&
  	tr -d '\015' <output |
  	sort -u |
 -	sed -ne '/^Accept-Language:/ p' >actual &&
 +	sed -ne '/^=> Send header: Accept-Language:/ p' >actual &&
  	test_cmp expect actual
  }
  
@@@ -295,9 -246,17 +295,17 @@@ ja;q=0.95, zh;q=0.94, sv;q=0.93, pt;q=0
  '
  
  test_expect_success 'git client does not send an empty Accept-Language' '
 -	GIT_CURL_VERBOSE=1 LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr &&
 -	! grep "^Accept-Language:" stderr
 +	GIT_TRACE_CURL=true LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr &&
 +	! grep "^=> Send header: Accept-Language:" stderr
  '
  
+ test_expect_success 'remote-http complains cleanly about malformed urls' '
+ 	# do not actually issue "list" or other commands, as we do not
+ 	# want to rely on what curl would actually do with such a broken
+ 	# URL. This is just about making sure we do not segfault during
+ 	# initialization.
+ 	test_must_fail git remote-http http::/example.com/repo.git
+ '
+ 
  stop_httpd
  test_done