[BUG] Filenames with single colon being treated as remote repository

All,

It was brought to my attention today that git has some weird behaviour
when colons (:) are used in directory names.

In my distros packaging system, for git repositories we clone a bare
repo and then clone that bare repo locally as a temporary build
directory (no, we can't use cp, it's a bare repository). Say we have a
directory, /tmp/foo:bar/baz, that is a git repository. If I want to get
a clone of that repository locally, using all of the local
optimizations, then I need to run::

    $ git clone /tmp/foo:bar/baz /tmp/new-baz

but running this gives me this output::

    Cloning into 'new-baz'...
    ssh: Could not resolve hostname /tmp/foo: Success
    fatal: Could not read from remote repository.

    Please make sure you have the correct access rights
    and the repository exists.
    
which it should not be doing. It is possible to use a file:// url to
clone it, but then the --local option is ignored and no optimizations
are made. After asking on #git, I was directed to the transport.c file,
but I don't know what in that is failing. We ran some tests on the
is_local function and it seems to work correctly.

Any ideas on how to debug this further?

Thank you,
-- 
William Giokas | KaiSforza
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF

Hi,

William Giokas wrote:

    $ git clone /tmp/foo:bar/baz /tmp/new-baz

but running this gives me this output::

    Cloning into 'new-baz'...
    ssh: Could not resolve hostname /tmp/foo: Success
    fatal: Could not read from remote repository.

Here's a toy patch.  I haven't thought carefully about whether it's a
good idea, but maybe it can be useful for thinking about that.

Still needs documentation and tests.

My main worry is that the proposed rule for when an argument is
treated as a local path is hard to explain.  There's some precedent in
handling of bundles, though.  What do you think?

Thanks,
Jonathan

diff --git i/transport.c w/transport.c
index e6f9346c..61eba842 100644
--- i/transport.c
+++ w/transport.c

@@ -903,6 +903,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 {
 	const char *helper;
 	struct transport *ret = xcalloc(1, sizeof(*ret));
+	struct stat st;
 
 	ret->progress = isatty(2);
 

@@ -942,6 +943,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		ret->disconnect = close_bundle;
 		ret->smart_options = NULL;
 	} else if (!is_url(url)
+		|| (is_local(url) && !stat(url, &st))
 		|| !prefixcmp(url, "file://")
 		|| !prefixcmp(url, "git://")
 		|| !prefixcmp(url, "ssh://")

On Sat, Apr 20, 2013 at 11:05:39PM -0700, Jonathan Nieder wrote:

quoted

    Cloning into 'new-baz'...
    ssh: Could not resolve hostname /tmp/foo: Success
    fatal: Could not read from remote repository.

Here's a toy patch.  I haven't thought carefully about whether it's a
good idea, but maybe it can be useful for thinking about that.

Still needs documentation and tests.

My main worry is that the proposed rule for when an argument is
treated as a local path is hard to explain.  There's some precedent in
handling of bundles, though.  What do you think?

I think the rule could be something like:

  1. If it looks like a URL ("^scheme://"), it is.

  2. Otherwise, if it is a path in the filesystem, it is.

  3. Otherwise, if it has a colon, it's host:path

  4. Otherwise, barf.

where the interesting bit is the ordering of 2 and 3.  It seems like
"git clone" follows the order above with get_repo_path. But we do not
seem to follow it in git_connect, where we prefer 3 over 2.

I don't think that is enough. Something like /path/to/foo:bar would
trigger !is_url already, but then git_connect fails.

Try:

  $ git init --bare foo:bar
  $ git clone foo:bar
  ssh: Could not resolve hostname /home/peff/foo: Name or service not known
  fatal: Could not read from remote repository.
  ...

Clone recognizes it as a path and turns it into an absolute path. It
then feeds it to the transport code, which triggers !is_url and knows to
use the git transport. But then git_connect prefers ssh over the
filesystem.

If you do a straight fetch, though, the transport code might see the
relative path (if you use one):

  $ git init
  $ git init --bare sub:repo
  $ git fetch sub:repo
  ssh: Could not resolve hostname sub: Name or service not known

but that still triggers the is_url above (which demands the "://").

I am not sure whether your patch covers any cases I am missing, but I
think you would need an analogous change to git_connect for these common
cases.

-Peff

Jeff King wrote:

I don't think that is enough. Something like /path/to/foo:bar would
trigger !is_url already, but then git_connect fails.

Doh.  Here's another try, still untested.

diff --git i/connect.c w/connect.c
index 49e56ba3..fe13942f 100644
--- i/connect.c
+++ w/connect.c

@@ -504,6 +504,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	int c;
 	struct child_process *conn = &no_fork;
 	enum protocol protocol = PROTO_LOCAL;
+	struct stat st;
 	int free_path = 0;
 	char *port = NULL;
 	const char **arg;

@@ -548,7 +549,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 		end = host;
 
 	path = strchr(end, c);
-	if (path && !has_dos_drive_prefix(end)) {
+	if (path && !has_dos_drive_prefix(end) &&
+	    (c != ':' || stat(path, &st))) {
 		if (c == ':') {
 			protocol = PROTO_SSH;
 			*path++ = '\0';