From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 17 Dec 2013 20:03:31 +0000 (-0800)
Subject: Merge branch 'tb/clone-ssh-with-colon-for-port'
X-Git-Tag: v1.9-rc0~58
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/1945e8ac85c9f2b01d6e431b267fdb259bd50e1f?ds=inline;hp=-c

Merge branch 'tb/clone-ssh-with-colon-for-port'

Be more careful when parsing remote repository URL given in the
scp-style host:path notation.

* tb/clone-ssh-with-colon-for-port:
git_connect(): use common return point
connect.c: refactor url parsing
git_connect(): refactor the port handling for ssh
git fetch: support host:/~repo
t5500: add test cases for diag-url
git fetch-pack: add --diag-url
git_connect: factor out discovery of the protocol and its parts
git_connect: remove artificial limit of a remote command
t5601: add tests for ssh
t5601: remove clear_ssh, refactor setup_ssh_wrapper
---

1945e8ac85c9f2b01d6e431b267fdb259bd50e1f
diff --combined builtin/fetch-pack.c
index 10fcdc2ff2,758b5acd55..8b8978a252
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@@ -7,7 -7,7 +7,7 @@@
  static const char fetch_pack_usage[] =
  "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] "
  "[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
- "[--no-progress] [-v] [<host>:]<directory> [<refs>...]";
+ "[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
  
  static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc,
  				 const char *name, int namelen)
@@@ -48,11 -48,11 +48,11 @@@ int cmd_fetch_pack(int argc, const cha
  	for (i = 1; i < argc && *argv[i] == '-'; i++) {
  		const char *arg = argv[i];
  
 -		if (!prefixcmp(arg, "--upload-pack=")) {
 +		if (starts_with(arg, "--upload-pack=")) {
  			args.uploadpack = arg + 14;
  			continue;
  		}
 -		if (!prefixcmp(arg, "--exec=")) {
 +		if (starts_with(arg, "--exec=")) {
  			args.uploadpack = arg + 7;
  			continue;
  		}
@@@ -81,11 -81,15 +81,15 @@@
  			args.stdin_refs = 1;
  			continue;
  		}
+ 		if (!strcmp("--diag-url", arg)) {
+ 			args.diag_url = 1;
+ 			continue;
+ 		}
  		if (!strcmp("-v", arg)) {
  			args.verbose = 1;
  			continue;
  		}
 -		if (!prefixcmp(arg, "--depth=")) {
 +		if (starts_with(arg, "--depth=")) {
  			args.depth = strtol(arg + 8, NULL, 0);
  			continue;
  		}
@@@ -146,10 -150,14 +150,14 @@@
  		fd[0] = 0;
  		fd[1] = 1;
  	} else {
+ 		int flags = args.verbose ? CONNECT_VERBOSE : 0;
+ 		if (args.diag_url)
+ 			flags |= CONNECT_DIAG_URL;
  		conn = git_connect(fd, dest, args.uploadpack,
- 				   args.verbose ? CONNECT_VERBOSE : 0);
+ 				   flags);
+ 		if (!conn)
+ 			return args.diag_url ? 0 : 1;
  	}
- 
  	get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL);
  
  	ref = fetch_pack(&args, fd, conn, ref, dest,
diff --combined connect.c
index d51d106633,8a013a723b..c763eeda86
--- a/connect.c
+++ b/connect.c
@@@ -145,7 -145,7 +145,7 @@@ struct ref **get_remote_heads(int in, c
  		if (!len)
  			break;
  
 -		if (len > 4 && !prefixcmp(buffer, "ERR "))
 +		if (len > 4 && starts_with(buffer, "ERR "))
  			die("remote error: %s", buffer + 4);
  
  		if (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] != ' ')
@@@ -232,10 -232,34 +232,34 @@@ int server_supports(const char *feature
  
  enum protocol {
  	PROTO_LOCAL = 1,
+ 	PROTO_FILE,
  	PROTO_SSH,
  	PROTO_GIT
  };
  
+ int url_is_local_not_ssh(const char *url)
+ {
+ 	const char *colon = strchr(url, ':');
+ 	const char *slash = strchr(url, '/');
+ 	return !colon || (slash && slash < colon) ||
+ 		has_dos_drive_prefix(url);
+ }
+ 
+ static const char *prot_name(enum protocol protocol)
+ {
+ 	switch (protocol) {
+ 		case PROTO_LOCAL:
+ 		case PROTO_FILE:
+ 			return "file";
+ 		case PROTO_SSH:
+ 			return "ssh";
+ 		case PROTO_GIT:
+ 			return "git";
+ 		default:
+ 			return "unkown protocol";
+ 	}
+ }
+ 
  static enum protocol get_protocol(const char *name)
  {
  	if (!strcmp(name, "ssh"))
@@@ -247,7 -271,7 +271,7 @@@
  	if (!strcmp(name, "ssh+git"))
  		return PROTO_SSH;
  	if (!strcmp(name, "file"))
- 		return PROTO_LOCAL;
+ 		return PROTO_FILE;
  	die("I don't handle protocol '%s'", name);
  }
  
@@@ -527,55 -551,31 +551,31 @@@ static struct child_process *git_proxy_
  	return proxy;
  }
  
- #define MAX_CMD_LEN 1024
- 
- static char *get_port(char *host)
+ static const char *get_port_numeric(const char *p)
  {
  	char *end;
- 	char *p = strchr(host, ':');
- 
  	if (p) {
  		long port = strtol(p + 1, &end, 10);
  		if (end != p + 1 && *end == '\0' && 0 <= port && port < 65536) {
- 			*p = '\0';
- 			return p+1;
+ 			return p;
  		}
  	}
  
  	return NULL;
  }
  
- static struct child_process no_fork;
- 
  /*
-  * This returns a dummy child_process if the transport protocol does not
-  * need fork(2), or a struct child_process object if it does.  Once done,
-  * finish the connection with finish_connect() with the value returned from
-  * this function (it is safe to call finish_connect() with NULL to support
-  * the former case).
-  *
-  * If it returns, the connect is successful; it just dies on errors (this
-  * will hopefully be changed in a libification effort, to return NULL when
-  * the connection failed).
+  * Extract protocol and relevant parts from the specified connection URL.
+  * The caller must free() the returned strings.
   */
- struct child_process *git_connect(int fd[2], const char *url_orig,
- 				  const char *prog, int flags)
+ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
+ 				       char **ret_path)
  {
  	char *url;
  	char *host, *path;
  	char *end;
- 	int c;
- 	struct child_process *conn = &no_fork;
+ 	int separator = '/';
  	enum protocol protocol = PROTO_LOCAL;
- 	int free_path = 0;
- 	char *port = NULL;
- 	const char **arg;
- 	struct strbuf cmd;
- 
- 	/* Without this we cannot rely on waitpid() to tell
- 	 * what happened to our children.
- 	 */
- 	signal(SIGCHLD, SIG_DFL);
  
  	if (is_url(url_orig))
  		url = url_decode(url_orig);
@@@ -587,40 -587,33 +587,33 @@@
  		*host = '\0';
  		protocol = get_protocol(url);
  		host += 3;
- 		c = '/';
  	} else {
  		host = url;
- 		c = ':';
+ 		if (!url_is_local_not_ssh(url)) {
+ 			protocol = PROTO_SSH;
+ 			separator = ':';
+ 		}
  	}
  
  	/*
- 	 * Don't do destructive transforms with git:// as that
- 	 * protocol code does '[]' unwrapping of its own.
+ 	 * Don't do destructive transforms as protocol code does
+ 	 * '[]' unwrapping in get_host_and_port()
  	 */
  	if (host[0] == '[') {
  		end = strchr(host + 1, ']');
  		if (end) {
- 			if (protocol != PROTO_GIT) {
- 				*end = 0;
- 				host++;
- 			}
  			end++;
  		} else
  			end = host;
  	} else
  		end = host;
  
- 	path = strchr(end, c);
- 	if (path && !has_dos_drive_prefix(end)) {
- 		if (c == ':') {
- 			if (host != url || path < strchrnul(host, '/')) {
- 				protocol = PROTO_SSH;
- 				*path++ = '\0';
- 			} else /* '/' in the host part, assume local path */
- 				path = end;
- 		}
- 	} else
+ 	if (protocol == PROTO_LOCAL)
  		path = end;
+ 	else if (protocol == PROTO_FILE && has_dos_drive_prefix(end))
+ 		path = end; /* "file://$(pwd)" may be "file://C:/projects/repo" */
+ 	else
+ 		path = strchr(end, separator);
  
  	if (!path || !*path)
  		die("No path specified. See 'man git-pull' for valid url syntax");
@@@ -629,33 -622,67 +622,67 @@@
  	 * null-terminate hostname and point path to ~ for URL's like this:
  	 *    ssh://host.xz/~user/repo
  	 */
- 	if (protocol != PROTO_LOCAL && host != url) {
- 		char *ptr = path;
+ 
+ 	end = path; /* Need to \0 terminate host here */
+ 	if (separator == ':')
+ 		path++; /* path starts after ':' */
+ 	if (protocol == PROTO_GIT || protocol == PROTO_SSH) {
  		if (path[1] == '~')
  			path++;
- 		else {
- 			path = xstrdup(ptr);
- 			free_path = 1;
- 		}
- 
- 		*ptr = '\0';
  	}
  
- 	/*
- 	 * Add support for ssh port: ssh://host.xy:<port>/...
+ 	path = xstrdup(path);
+ 	*end = '\0';
+ 
+ 	*ret_host = xstrdup(host);
+ 	*ret_path = path;
+ 	free(url);
+ 	return protocol;
+ }
+ 
+ static struct child_process no_fork;
+ 
+ /*
+  * This returns a dummy child_process if the transport protocol does not
+  * need fork(2), or a struct child_process object if it does.  Once done,
+  * finish the connection with finish_connect() with the value returned from
+  * this function (it is safe to call finish_connect() with NULL to support
+  * the former case).
+  *
+  * If it returns, the connect is successful; it just dies on errors (this
+  * will hopefully be changed in a libification effort, to return NULL when
+  * the connection failed).
+  */
+ struct child_process *git_connect(int fd[2], const char *url,
+ 				  const char *prog, int flags)
+ {
+ 	char *hostandport, *path;
+ 	struct child_process *conn = &no_fork;
+ 	enum protocol protocol;
+ 	const char **arg;
+ 	struct strbuf cmd = STRBUF_INIT;
+ 
+ 	/* Without this we cannot rely on waitpid() to tell
+ 	 * what happened to our children.
  	 */
- 	if (protocol == PROTO_SSH && host != url)
- 		port = get_port(end);
+ 	signal(SIGCHLD, SIG_DFL);
  
- 	if (protocol == PROTO_GIT) {
+ 	protocol = parse_connect_url(url, &hostandport, &path);
+ 	if (flags & CONNECT_DIAG_URL) {
+ 		printf("Diag: url=%s\n", url ? url : "NULL");
+ 		printf("Diag: protocol=%s\n", prot_name(protocol));
+ 		printf("Diag: hostandport=%s\n", hostandport ? hostandport : "NULL");
+ 		printf("Diag: path=%s\n", path ? path : "NULL");
+ 		conn = NULL;
+ 	} else if (protocol == PROTO_GIT) {
  		/* These underlying connection commands die() if they
  		 * cannot connect.
  		 */
- 		char *target_host = xstrdup(host);
- 		if (git_use_proxy(host))
- 			conn = git_proxy_connect(fd, host);
+ 		char *target_host = xstrdup(hostandport);
+ 		if (git_use_proxy(hostandport))
+ 			conn = git_proxy_connect(fd, hostandport);
  		else
- 			git_tcp_connect(fd, host, flags);
+ 			git_tcp_connect(fd, hostandport, flags);
  		/*
  		 * Separate original protocol components prog and path
  		 * from extended host header with a NUL byte.
@@@ -668,55 -695,51 +695,51 @@@
  			     prog, path, 0,
  			     target_host, 0);
  		free(target_host);
- 		free(url);
- 		if (free_path)
- 			free(path);
- 		return conn;
- 	}
- 
- 	conn = xcalloc(1, sizeof(*conn));
- 
- 	strbuf_init(&cmd, MAX_CMD_LEN);
- 	strbuf_addstr(&cmd, prog);
- 	strbuf_addch(&cmd, ' ');
- 	sq_quote_buf(&cmd, path);
- 	if (cmd.len >= MAX_CMD_LEN)
- 		die("command line too long");
- 
- 	conn->in = conn->out = -1;
- 	conn->argv = arg = xcalloc(7, sizeof(*arg));
- 	if (protocol == PROTO_SSH) {
- 		const char *ssh = getenv("GIT_SSH");
- 		int putty = ssh && strcasestr(ssh, "plink");
- 		if (!ssh) ssh = "ssh";
- 
- 		*arg++ = ssh;
- 		if (putty && !strcasestr(ssh, "tortoiseplink"))
- 			*arg++ = "-batch";
- 		if (port) {
- 			/* P is for PuTTY, p is for OpenSSH */
- 			*arg++ = putty ? "-P" : "-p";
- 			*arg++ = port;
+ 	} else {
+ 		conn = xcalloc(1, sizeof(*conn));
+ 
+ 		strbuf_addstr(&cmd, prog);
+ 		strbuf_addch(&cmd, ' ');
+ 		sq_quote_buf(&cmd, path);
+ 
+ 		conn->in = conn->out = -1;
+ 		conn->argv = arg = xcalloc(7, sizeof(*arg));
+ 		if (protocol == PROTO_SSH) {
+ 			const char *ssh = getenv("GIT_SSH");
+ 			int putty = ssh && strcasestr(ssh, "plink");
+ 			char *ssh_host = hostandport;
+ 			const char *port = NULL;
+ 			get_host_and_port(&ssh_host, &port);
+ 			port = get_port_numeric(port);
+ 
+ 			if (!ssh) ssh = "ssh";
+ 
+ 			*arg++ = ssh;
+ 			if (putty && !strcasestr(ssh, "tortoiseplink"))
+ 				*arg++ = "-batch";
+ 			if (port) {
+ 				/* P is for PuTTY, p is for OpenSSH */
+ 				*arg++ = putty ? "-P" : "-p";
+ 				*arg++ = port;
+ 			}
+ 			*arg++ = ssh_host;
+ 		}	else {
+ 			/* remove repo-local variables from the environment */
+ 			conn->env = local_repo_env;
+ 			conn->use_shell = 1;
  		}
- 		*arg++ = host;
- 	}
- 	else {
- 		/* remove repo-local variables from the environment */
- 		conn->env = local_repo_env;
- 		conn->use_shell = 1;
- 	}
- 	*arg++ = cmd.buf;
- 	*arg = NULL;
+ 		*arg++ = cmd.buf;
+ 		*arg = NULL;
  
- 	if (start_command(conn))
- 		die("unable to fork");
+ 		if (start_command(conn))
+ 			die("unable to fork");
  
- 	fd[0] = conn->out; /* read from child's stdout */
- 	fd[1] = conn->in;  /* write to child's stdin */
- 	strbuf_release(&cmd);
- 	free(url);
- 	if (free_path)
- 		free(path);
+ 		fd[0] = conn->out; /* read from child's stdout */
+ 		fd[1] = conn->in;  /* write to child's stdin */
+ 		strbuf_release(&cmd);
+ 	}
+ 	free(hostandport);
+ 	free(path);
  	return conn;
  }
  
diff --combined transport.c
index 8023956696,5485e2ab1e..824c5b93f9
--- a/transport.c
+++ b/transport.c
@@@ -169,13 -169,13 +169,13 @@@ static void set_upstreams(struct transp
  		remotename = ref->name;
  		tmp = resolve_ref_unsafe(localname, sha, 1, &flag);
  		if (tmp && flag & REF_ISSYMREF &&
 -			!prefixcmp(tmp, "refs/heads/"))
 +			starts_with(tmp, "refs/heads/"))
  			localname = tmp;
  
  		/* Both source and destination must be local branches. */
 -		if (!localname || prefixcmp(localname, "refs/heads/"))
 +		if (!localname || !starts_with(localname, "refs/heads/"))
  			continue;
 -		if (!remotename || prefixcmp(remotename, "refs/heads/"))
 +		if (!remotename || !starts_with(remotename, "refs/heads/"))
  			continue;
  
  		if (!pretend)
@@@ -191,7 -191,7 +191,7 @@@
  
  static const char *rsync_url(const char *url)
  {
 -	return prefixcmp(url, "rsync://") ? skip_prefix(url, "rsync:") : url;
 +	return !starts_with(url, "rsync://") ? skip_prefix(url, "rsync:") : url;
  }
  
  static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)
@@@ -296,8 -296,8 +296,8 @@@ static int write_one_ref(const char *na
  	FILE *f;
  
  	/* when called via for_each_ref(), flags is non-zero */
 -	if (flags && prefixcmp(name, "refs/heads/") &&
 -			prefixcmp(name, "refs/tags/"))
 +	if (flags && !starts_with(name, "refs/heads/") &&
 +			!starts_with(name, "refs/tags/"))
  		return 0;
  
  	strbuf_addstr(buf, name);
@@@ -652,7 -652,7 +652,7 @@@ static void print_ok_ref_status(struct 
  		print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
  	else if (is_null_sha1(ref->old_sha1))
  		print_ref_status('*',
 -			(!prefixcmp(ref->name, "refs/tags/") ? "[new tag]" :
 +			(starts_with(ref->name, "refs/tags/") ? "[new tag]" :
  			"[new branch]"),
  			ref, ref->peer_ref, NULL, porcelain);
  	else {
@@@ -885,14 -885,6 +885,6 @@@ void transport_take_over(struct transpo
  	transport->cannot_reuse = 1;
  }
  
- static int is_local(const char *url)
- {
- 	const char *colon = strchr(url, ':');
- 	const char *slash = strchr(url, '/');
- 	return !colon || (slash && slash < colon) ||
- 		has_dos_drive_prefix(url);
- }
- 
  static int is_file(const char *url)
  {
  	struct stat buf;
@@@ -930,18 -922,18 +922,18 @@@ struct transport *transport_get(struct 
  
  		while (is_urlschemechar(p == url, *p))
  			p++;
 -		if (!prefixcmp(p, "::"))
 +		if (starts_with(p, "::"))
  			helper = xstrndup(url, p - url);
  	}
  
  	if (helper) {
  		transport_helper_init(ret, helper);
 -	} else if (!prefixcmp(url, "rsync:")) {
 +	} else if (starts_with(url, "rsync:")) {
  		ret->get_refs_list = get_refs_via_rsync;
  		ret->fetch = fetch_objs_via_rsync;
  		ret->push = rsync_transport_push;
  		ret->smart_options = NULL;
- 	} else if (is_local(url) && is_file(url) && is_bundle(url, 1)) {
+ 	} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {
  		struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
  		ret->data = data;
  		ret->get_refs_list = get_refs_from_bundle;
@@@ -949,11 -941,11 +941,11 @@@
  		ret->disconnect = close_bundle;
  		ret->smart_options = NULL;
  	} else if (!is_url(url)
 -		|| !prefixcmp(url, "file://")
 -		|| !prefixcmp(url, "git://")
 -		|| !prefixcmp(url, "ssh://")
 -		|| !prefixcmp(url, "git+ssh://")
 -		|| !prefixcmp(url, "ssh+git://")) {
 +		|| starts_with(url, "file://")
 +		|| starts_with(url, "git://")
 +		|| starts_with(url, "ssh://")
 +		|| starts_with(url, "git+ssh://")
 +		|| starts_with(url, "ssh+git://")) {
  		/* These are builtin smart transports. */
  		struct git_transport_data *data = xcalloc(1, sizeof(*data));
  		ret->data = data;
@@@ -1297,7 -1289,7 +1289,7 @@@ char *transport_anonymize_url(const cha
  	size_t anon_len, prefix_len = 0;
  
  	anon_part = strchr(url, '@');
- 	if (is_local(url) || !anon_part)
+ 	if (url_is_local_not_ssh(url) || !anon_part)
  		goto literal_copy;
  
  	anon_len = strlen(++anon_part);