Subject: Re: [PATCH v4 1/3] clone: do not include authentication data in guessed dir

From: Junio C Hamano <[email-masked]>
Date: Wed, 05 Aug 2015 10:43:06 -0700

Patrick Steinhardt <[email-masked]> writes:

> If the URI contains authentication data and the URI's path
> component is empty we fail to guess a sensible directory name.
> E.g. cloning a repository 'ssh://user:[email-masked]/' we
> guess a directory name [email-masked]' where we would want
> the hostname only, e.g. 'example.com'.
>
> ...
> +	ptr = start;
> +	while (ptr < end && !is_dir_sep(*ptr) && *ptr != '@')
> +		ptr++;

Hmm....

> +	if (*ptr == '@')
> +		start = ptr + 1;
> +
> +	 * Find last component. To remain backwards compatible we
> +	 * also regard colons as path separators, such that
> +	 * cloning a repository 'foo:bar.git' would result in a
> +	 * directory 'bar' being guessed.
>  	 */

I think this is a reasonable thing to do (besides, I think some
people cannot have colon in their filenames, so keeping this aspect
the same as before would avoid unintended regressions).

> -	start = end;
> -	while (repo < start && !is_dir_sep(start[-1]) && start[-1] != ':')
> -		start--;
> +	ptr = end;
> +	while (start < ptr && !is_dir_sep(ptr[-1]) && ptr[-1] != ':')
> +		ptr--;
> +	start = ptr;
>  
>  	/*
>  	 * Strip .{bundle,git}.
> diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
> index 46725b9..3a454f9 100755
> --- a/t/t5603-clone-dirname.sh
> +++ b/t/t5603-clone-dirname.sh
> @@ -64,6 +64,7 @@ test_clone_dir ssh://host/foo/.git/ foo
>  # omitting the path should default to the hostname
>  test_clone_dir ssh://host/ host
>  test_clone_dir ssh://host:1234/ host fail
> -test_clone_dir ssh://user@host/ host fail
> +test_clone_dir ssh://user@host/ host
> +test_clone_dir ssh://user:password@host/ host

Perhaps add

"test_clone_dir ssh://user:passw@rd@host/ host"

here?  How is this expected to be parsed?

>  test_done