Merge branch 'jk/url-decode'

* jk/url-decode:
  decode file:// and ssh:// URLs
  make url-related functions reusable

Makefile

@@ -627,6 +627,7 @@ LIB_OBJS += tree-diff.o
 LIB_OBJS += tree.o
 LIB_OBJS += tree-walk.o
 LIB_OBJS += unpack-trees.o
+LIB_OBJS += url.o
 LIB_OBJS += usage.o
 LIB_OBJS += userdiff.o
 LIB_OBJS += utf8.o

connect.c

@@ -5,6 +5,7 @@
 #include "refs.h"
 #include "run-command.h"
 #include "remote.h"
+#include "url.h"
 
 static char *server_capabilities;
 
@@ -450,7 +451,7 @@ static struct child_process no_fork;
 struct child_process *git_connect(int fd[2], const char *url_orig,
 				  const char *prog, int flags)
 {
-	char *url = xstrdup(url_orig);
+	char *url;
 	char *host, *path;
 	char *end;
 	int c;
@@ -466,6 +467,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	 */
 	signal(SIGCHLD, SIG_DFL);
 
+	if (is_url(url_orig))
+		url = url_decode(url_orig);
+	else
+		url = xstrdup(url_orig);
+
 	host = strstr(url, "://");
 	if (host) {
 		*host = '\0';

http-backend.c

@@ -6,6 +6,7 @@
 #include "exec_cmd.h"
 #include "run-command.h"
 #include "string-list.h"
+#include "url.h"
 
 static const char content_type[] = "Content-Type";
 static const char content_length[] = "Content-Length";
@@ -25,60 +26,6 @@ static struct rpc_service rpc_service[] = {
 	{ "receive-pack", "receivepack", -1 },
 };
 
-static int decode_char(const char *q)
-{
-	int i;
-	unsigned char val = 0;
-	for (i = 0; i < 2; i++) {
-		unsigned char c = *q++;
-		val <<= 4;
-		if (c >= '0' && c <= '9')
-			val += c - '0';
-		else if (c >= 'a' && c <= 'f')
-			val += c - 'a' + 10;
-		else if (c >= 'A' && c <= 'F')
-			val += c - 'A' + 10;
-		else
-			return -1;
-	}
-	return val;
-}
-
-static char *decode_parameter(const char **query, int is_name)
-{
-	const char *q = *query;
-	struct strbuf out;
-
-	strbuf_init(&out, 16);
-	do {
-		unsigned char c = *q;
-
-		if (!c)
-			break;
-		if (c == '&' || (is_name && c == '=')) {
-			q++;
-			break;
-		}
-
-		if (c == '%') {
-			int val = decode_char(q + 1);
-			if (0 <= val) {
-				strbuf_addch(&out, val);
-				q += 3;
-				continue;
-			}
-		}
-
-		if (c == '+')
-			strbuf_addch(&out, ' ');
-		else
-			strbuf_addch(&out, c);
-		q++;
-	} while (1);
-	*query = q;
-	return strbuf_detach(&out, NULL);
-}
-
 static struct string_list *get_parameters(void)
 {
 	if (!query_params) {
@@ -86,8 +33,8 @@ static struct string_list *get_parameters(void)
 
 		query_params = xcalloc(1, sizeof(*query_params));
 		while (query && *query) {
-			char *name = decode_parameter(&query, 1);
+			char *name = url_decode_parameter_name(&query);
-			char *value = decode_parameter(&query, 0);
+			char *value = url_decode_parameter_value(&query);
 			struct string_list_item *i;
 
 			i = string_list_lookup(name, query_params);

t/t5601-clone.sh

@@ -176,4 +176,16 @@ test_expect_success 'clone respects global branch.autosetuprebase' '
 	)
 '
 
+test_expect_success 'respect url-encoding of file://' '
+	git init x+y &&
+	test_must_fail git clone "file://$PWD/x+y" xy-url &&
+	git clone "file://$PWD/x%2By" xy-url
+'
+
+test_expect_success 'do not respect url-encoding of non-url path' '
+	git init x+y &&
+	test_must_fail git clone x%2By xy-regular &&
+	git clone x+y xy-regular
+'
+
 test_done

transport.c

@@ -9,6 +9,7 @@
 #include "dir.h"
 #include "refs.h"
 #include "branch.h"
+#include "url.h"
 
 /* rsync support */
 
@@ -871,54 +872,6 @@ static int is_file(const char *url)
 	return S_ISREG(buf.st_mode);
 }
 
-static int isurlschemechar(int first_flag, int ch)
-{
-	/*
-	 * The set of valid URL schemes, as per STD66 (RFC3986) is
-	 * '[A-Za-z][A-Za-z0-9+.-]*'. But use sightly looser check
-	 * of '[A-Za-z0-9][A-Za-z0-9+.-]*' because earlier version
-	 * of check used '[A-Za-z0-9]+' so not to break any remote
-	 * helpers.
-	 */
-	int alphanumeric, special;
-	alphanumeric = ch > 0 && isalnum(ch);
-	special = ch == '+' || ch == '-' || ch == '.';
-	return alphanumeric || (!first_flag && special);
-}
-
-static int is_url(const char *url)
-{
-	const char *url2, *first_slash;
-
-	if (!url)
-		return 0;
-	url2 = url;
-	first_slash = strchr(url, '/');
-
-	/* Input with no slash at all or slash first can't be URL. */
-	if (!first_slash || first_slash == url)
-		return 0;
-	/* Character before must be : and next must be /. */
-	if (first_slash[-1] != ':' || first_slash[1] != '/')
-		return 0;
-	/* There must be something before the :// */
-	if (first_slash == url + 1)
-		return 0;
-	/*
-	 * Check all characters up to first slash - 1. Only alphanum
-	 * is allowed.
-	 */
-	url2 = url;
-	while (url2 < first_slash - 1) {
-		if (!isurlschemechar(url2 == url, (unsigned char)*url2))
-			return 0;
-		url2++;
-	}
-
-	/* Valid enough. */
-	return 1;
-}
-
 static int external_specification_len(const char *url)
 {
 	return strchr(url, ':') - url;
@@ -946,7 +899,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	if (url) {
 		const char *p = url;
 
-		while (isurlschemechar(p == url, *p))
+		while (is_urlschemechar(p == url, *p))
 			p++;
 		if (!prefixcmp(p, "::"))
 			helper = xstrndup(url, p - url);

url.c

@@ -0,0 +1,118 @@
+#include "cache.h"
+
+int is_urlschemechar(int first_flag, int ch)
+{
+	/*
+	 * The set of valid URL schemes, as per STD66 (RFC3986) is
+	 * '[A-Za-z][A-Za-z0-9+.-]*'. But use sightly looser check
+	 * of '[A-Za-z0-9][A-Za-z0-9+.-]*' because earlier version
+	 * of check used '[A-Za-z0-9]+' so not to break any remote
+	 * helpers.
+	 */
+	int alphanumeric, special;
+	alphanumeric = ch > 0 && isalnum(ch);
+	special = ch == '+' || ch == '-' || ch == '.';
+	return alphanumeric || (!first_flag && special);
+}
+
+int is_url(const char *url)
+{
+	const char *url2, *first_slash;
+
+	if (!url)
+		return 0;
+	url2 = url;
+	first_slash = strchr(url, '/');
+
+	/* Input with no slash at all or slash first can't be URL. */
+	if (!first_slash || first_slash == url)
+		return 0;
+	/* Character before must be : and next must be /. */
+	if (first_slash[-1] != ':' || first_slash[1] != '/')
+		return 0;
+	/* There must be something before the :// */
+	if (first_slash == url + 1)
+		return 0;
+	/*
+	 * Check all characters up to first slash - 1. Only alphanum
+	 * is allowed.
+	 */
+	url2 = url;
+	while (url2 < first_slash - 1) {
+		if (!is_urlschemechar(url2 == url, (unsigned char)*url2))
+			return 0;
+		url2++;
+	}
+
+	/* Valid enough. */
+	return 1;
+}
+
+static int url_decode_char(const char *q)
+{
+	int i;
+	unsigned char val = 0;
+	for (i = 0; i < 2; i++) {
+		unsigned char c = *q++;
+		val <<= 4;
+		if (c >= '0' && c <= '9')
+			val += c - '0';
+		else if (c >= 'a' && c <= 'f')
+			val += c - 'a' + 10;
+		else if (c >= 'A' && c <= 'F')
+			val += c - 'A' + 10;
+		else
+			return -1;
+	}
+	return val;
+}
+
+static char *url_decode_internal(const char **query, const char *stop_at)
+{
+	const char *q = *query;
+	struct strbuf out;
+
+	strbuf_init(&out, 16);
+	do {
+		unsigned char c = *q;
+
+		if (!c)
+			break;
+		if (stop_at && strchr(stop_at, c)) {
+			q++;
+			break;
+		}
+
+		if (c == '%') {
+			int val = url_decode_char(q + 1);
+			if (0 <= val) {
+				strbuf_addch(&out, val);
+				q += 3;
+				continue;
+			}
+		}
+
+		if (c == '+')
+			strbuf_addch(&out, ' ');
+		else
+			strbuf_addch(&out, c);
+		q++;
+	} while (1);
+	*query = q;
+	return strbuf_detach(&out, NULL);
+}
+
+char *url_decode(const char *url)
+{
+	return url_decode_internal(&url, NULL);
+}
+
+char *url_decode_parameter_name(const char **query)
+{
+	return url_decode_internal(query, "&=");
+}
+
+char *url_decode_parameter_value(const char **query)
+{
+	return url_decode_internal(query, "&");
+}

url.h

@@ -0,0 +1,10 @@
+#ifndef URL_H
+#define URL_H
+
+extern int is_url(const char *url);
+extern int is_urlschemechar(int first_flag, int ch);
+extern char *url_decode(const char *url);
+extern char *url_decode_parameter_name(const char **query);
+extern char *url_decode_parameter_value(const char **query);
+
+#endif /* URL_H */