From 83b058752707a6ba4af51ebc98c47913bc7d2d25 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Torsten=20B=C3=B6gershausen?= Date: Thu, 28 Nov 2013 20:49:54 +0100 Subject: [PATCH] git_connect(): refactor the port handling for ssh MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Use get_host_and_port() even for ssh. Remove the variable port git_connect(), and simplify parse_connect_url() Use only one return point in git_connect(), doing the free() and return conn. t5601 had 2 corner test cases which now pass. Signed-off-by: Torsten Bögershausen Signed-off-by: Junio C Hamano --- connect.c | 47 +++++++++++++---------------------------------- t/t5500-fetch-pack.sh | 9 +++------ t/t5601-clone.sh | 10 +--------- 3 files changed, 17 insertions(+), 49 deletions(-) diff --git a/connect.c b/connect.c index 7e5f608a2f..78745306cb 100644 --- a/connect.c +++ b/connect.c @@ -541,16 +541,13 @@ static struct child_process *git_proxy_connect(int fd[2], char *host) return proxy; } -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; } } @@ -562,7 +559,7 @@ static char *get_port(char *host) * The caller must free() the returned strings. */ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, - char **ret_port, char **ret_path) + char **ret_path) { char *url; char *host, *path; @@ -570,7 +567,6 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, int separator; enum protocol protocol = PROTO_LOCAL; int free_path = 0; - char *port = NULL; if (is_url(url_orig)) url = url_decode(url_orig); @@ -589,16 +585,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, } /* - * 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; @@ -636,17 +628,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host, *ptr = '\0'; } - /* - * Add support for ssh port: ssh://host.xy:/... - */ - if (protocol == PROTO_SSH && separator == '/') - port = get_port(end); - *ret_host = xstrdup(host); - if (port) - *ret_port = xstrdup(port); - else - *ret_port = NULL; if (free_path) *ret_path = path; else @@ -674,7 +656,6 @@ struct child_process *git_connect(int fd[2], const char *url, char *host, *path; struct child_process *conn = &no_fork; enum protocol protocol; - char *port; const char **arg; struct strbuf cmd = STRBUF_INIT; @@ -683,18 +664,13 @@ struct child_process *git_connect(int fd[2], const char *url, */ signal(SIGCHLD, SIG_DFL); - protocol = parse_connect_url(url, &host, &port, &path); + protocol = parse_connect_url(url, &host, &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", host ? host : "NULL"); - if (port) - printf(":%s\n", port); - else - printf("\n"); + printf("Diag: hostandport=%s\n", host ? host : "NULL"); printf("Diag: path=%s\n", path ? path : "NULL"); free(host); - free(port); free(path); return NULL; } @@ -721,7 +697,6 @@ struct child_process *git_connect(int fd[2], const char *url, target_host, 0); free(target_host); free(host); - free(port); free(path); return conn; } @@ -737,6 +712,11 @@ struct child_process *git_connect(int fd[2], const char *url, if (protocol == PROTO_SSH) { const char *ssh = getenv("GIT_SSH"); int putty = ssh && strcasestr(ssh, "plink"); + char *ssh_host = host; /* keep host for the free() below */ + const char *port = NULL; + get_host_and_port(&ssh_host, &port); + port = get_port_numeric(port); + if (!ssh) ssh = "ssh"; *arg++ = ssh; @@ -747,7 +727,7 @@ struct child_process *git_connect(int fd[2], const char *url, *arg++ = putty ? "-P" : "-p"; *arg++ = port; } - *arg++ = host; + *arg++ = ssh_host; } else { /* remove repo-local variables from the environment */ @@ -764,7 +744,6 @@ struct child_process *git_connect(int fd[2], const char *url, fd[1] = conn->in; /* write to child's stdin */ strbuf_release(&cmd); free(host); - free(port); free(path); return conn; } diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index df5baf8d6f..b4866ea3a3 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -561,20 +561,18 @@ do do case "$p" in *ssh*) - hh=$(echo $h | tr -d "[]") pp=ssh ;; *) - hh=$h pp=$p ;; esac test_expect_success "fetch-pack --diag-url $p://$h/$r" ' - check_prot_host_path $p://$h/$r $pp "$hh" "/$r" + check_prot_host_path $p://$h/$r $pp "$h" "/$r" ' # "/~" -> "~" conversion test_expect_success "fetch-pack --diag-url $p://$h/~$r" ' - check_prot_host_path $p://$h/~$r $pp "$hh" "~$r" + check_prot_host_path $p://$h/~$r $pp "$h" "~$r" ' done done @@ -604,13 +602,12 @@ do p=ssh for h in host [::1] do - hh=$(echo $h | tr -d "[]") test_expect_success "fetch-pack --diag-url $h:$r" ' check_prot_path $h:$r $p "$r" ' # Do "/~" -> "~" conversion test_expect_success "fetch-pack --diag-url $h:/~$r" ' - check_prot_host_path $h:/~$r $p "$hh" "~$r" + check_prot_host_path $h:/~$r $p "$h" "~$r" ' done done diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 4db0c0b01e..53a1de9efd 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -364,15 +364,7 @@ do done # Corner cases -# failing -for url in [foo]bar/baz:qux [foo/bar]:baz -do - test_expect_failure "clone $url is not ssh" ' - test_clone_url $url none - ' -done - -for url in foo/bar:baz +for url in foo/bar:baz [foo]bar/baz:qux [foo/bar]:baz do test_expect_success "clone $url is not ssh" ' test_clone_url $url none -- 2.11.4.GIT