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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 5, 2015, 17:43 UTC
Message-ID: <xmqqzj253a39.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1438769179-5943-2-git-send-email-ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 10 quoted lines

> 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:password@example.com/' we
> guess a directory name 'password@example.com' where we would want
> the hostname only, e.g. 'example.com'.
>
> ...
> +	ptr = start;
> +	while (ptr < end && !is_dir_sep(*ptr) && *ptr != '@')
> +		ptr++;

Hmm....

Show 8 quoted lines

> +	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).

Show 21 quoted lines

> -	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