public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Patrick Steinhardt <ps@pks•im>
Cc: git@vger•kernel.org, sunshine@sunshineco•com, peff@peff•net,
	pclouds@gmail•com
Subject: Re: [PATCH v4 1/3] clone: do not include authentication data in guessed dir
Date: Wed, 05 Aug 2015 10:43:06 -0700	[thread overview]
Message-ID: <xmqqzj253a39.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1438769179-5943-2-git-send-email-ps@pks.im> (Patrick Steinhardt's message of "Wed, 5 Aug 2015 12:06:17 +0200")

Patrick Steinhardt <ps@pks•im> 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: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....

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

  reply	other threads:[~2015-08-05 17:43 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27 11:48 [PATCH] clone: fix repo name when cloning a server's root Patrick Steinhardt
2015-07-27 12:51 ` Duy Nguyen
2015-07-27 12:59   ` Patrick Steinhardt
2015-07-27 14:29   ` Junio C Hamano
2015-07-29 15:51 ` [PATCH v2 0/6] " Patrick Steinhardt
2015-07-29 15:51   ` [PATCH v2 1/6] tests: fix broken && chains in t1509-root-worktree Patrick Steinhardt
2015-07-29 15:51   ` [PATCH v2 2/6] tests: fix cleanup after tests " Patrick Steinhardt
2015-07-29 15:51   ` [PATCH v2 3/6] connect: expose parse_connect_url() Patrick Steinhardt
2015-07-29 15:51   ` [PATCH v2 4/6] connect: move error check to caller of parse_connect_url Patrick Steinhardt
2015-07-29 20:32     ` Eric Sunshine
2015-07-30 12:19       ` Patrick Steinhardt
2015-07-29 15:51   ` [PATCH v2 5/6] clone: fix hostname parsing when guessing dir Patrick Steinhardt
2015-07-29 17:42     ` Junio C Hamano
2015-07-30 12:18       ` Patrick Steinhardt
2015-07-30 16:30         ` Junio C Hamano
2015-07-30 16:53           ` Junio C Hamano
2015-08-03  8:34             ` Patrick Steinhardt
2015-08-03 16:37               ` Jeff King
2015-08-03 19:43                 ` Junio C Hamano
2015-07-29 15:51   ` [PATCH v2 6/6] clone: add tests for cloning with empty path Patrick Steinhardt
2015-07-30 18:18     ` Eric Sunshine
2015-07-31  0:58       ` Junio C Hamano
2015-07-31  8:45         ` Patrick Steinhardt
2015-08-04 11:29 ` [PATCH v3 0/6] fix repo name when cloning a server's root Patrick Steinhardt
2015-08-04 11:29   ` [PATCH v3 1/6] tests: fix broken && chains in t1509-root-worktree Patrick Steinhardt
2015-08-04 11:29   ` [PATCH v3 2/6] tests: fix cleanup after tests " Patrick Steinhardt
2015-08-04 11:29   ` [PATCH v3 3/6] clone: do not include authentication data in guessed dir Patrick Steinhardt
2015-08-04 11:29   ` [PATCH v3 4/6] clone: do not use port number as dir name Patrick Steinhardt
2015-08-04 11:29   ` [PATCH v3 5/6] clone: abort if no dir name could be guessed Patrick Steinhardt
2015-08-04 11:29   ` [PATCH v3 6/6] clone: add tests for cloning with empty path Patrick Steinhardt
2015-08-04 18:37     ` Eric Sunshine
2015-08-05 17:34   ` [PATCH v3 0/6] fix repo name when cloning a server's root Junio C Hamano
2015-08-05 21:19     ` Jeff King
2015-08-06  7:22       ` Torsten Bögershausen
2015-08-06  8:00         ` Junio C Hamano
2015-08-05 10:06 ` [PATCH v4 0/3] " Patrick Steinhardt
2015-08-05 10:06   ` [PATCH v4 1/3] clone: do not include authentication data in guessed dir Patrick Steinhardt
2015-08-05 17:43     ` Junio C Hamano [this message]
2015-08-05 19:36       ` Junio C Hamano
2015-08-05 19:41         ` Junio C Hamano
2015-08-06  9:47           ` Patrick Steinhardt
2015-08-07 20:45             ` Junio C Hamano
2015-08-08 17:37               ` Patrick Steinhardt
2015-08-05 10:06   ` [PATCH v4 2/3] clone: do not use port number as dir name Patrick Steinhardt
2015-08-05 10:06   ` [PATCH v4 3/3] clone: abort if no dir name could be guessed Patrick Steinhardt
2015-08-05 17:44     ` Junio C Hamano
2015-08-10 15:48 ` [PATCH v5 0/5] Improve guessing of repository names Patrick Steinhardt
2015-08-10 15:48   ` [PATCH v5 1/5] clone: add tests for output directory Patrick Steinhardt
2015-08-10 15:48   ` [PATCH v5 2/5] clone: use computed length in guess_dir_name Patrick Steinhardt
2015-08-10 15:48   ` [PATCH v5 3/5] clone: do not include authentication data in guessed dir Patrick Steinhardt
2015-08-10 15:48   ` [PATCH v5 4/5] clone: do not use port number as dir name Patrick Steinhardt
2015-08-10 15:48   ` [PATCH v5 5/5] clone: abort if no dir name could be guessed Patrick Steinhardt
2015-08-10 18:07   ` [PATCH v5 0/5] Improve guessing of repository names Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzj253a39.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=ps@pks$(echo .)im \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox