git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: git@vger.kernel.org
Cc: johan@herland.net, gitster@pobox.com, jrnieder@gmail.com
Subject: [PATCHv2 04/10] remote: Reject remote names containing '/'
Date: Sat, 11 May 2013 18:21:14 +0200	[thread overview]
Message-ID: <1368289280-30337-5-git-send-email-johan@herland.net> (raw)
In-Reply-To: <1368289280-30337-1-git-send-email-johan@herland.net>

Although we definitely support and encourage use of multi-level branch
names, we have never conciously tried to give support for multi-level
remote names. Currently, they are allowed, but there is no evidence that
they are commonly used.

Now, they do provide a source of problems when trying to expand the
"$nick/$name" shorthand notation (where $nick matches a remote name)
into a full refname. Consider the shorthand "foo/bar/baz": Does this
parse as $nick = foo, $name = bar/baz, or $nick = foo/bar, $name = baz?

Since we need to be unambiguous about these things, we hereby declare
that a remote name shall never contain a '/' character, and that the
only correct way to parse "foo/bar/baz" is $nick = foo, $name = bar/baz.

This patch teaches 'git remote' to reject remote names with slashes,
and adds tests verifying this.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin/remote.c  |  4 ++--
 t/t5505-remote.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 5e54d36..6e7369d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -195,7 +195,7 @@ static int add(int argc, const char **argv)
 		die(_("remote %s already exists."), name);
 
 	strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
-	if (!valid_fetch_refspec(buf2.buf))
+	if (!valid_fetch_refspec(buf2.buf) || strchr(name, '/'))
 		die(_("'%s' is not a valid remote name"), name);
 
 	strbuf_addf(&buf, "remote.%s.url", name);
@@ -646,7 +646,7 @@ static int mv(int argc, const char **argv)
 		die(_("remote %s already exists."), rename.new);
 
 	strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
-	if (!valid_fetch_refspec(buf.buf))
+	if (!valid_fetch_refspec(buf.buf) || strchr(rename.new, '/'))
 		die(_("'%s' is not a valid remote name"), rename.new);
 
 	strbuf_reset(&buf);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index dd10ff0..f7098fe 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -621,6 +621,12 @@ test_expect_success 'reject adding remote with an invalid name' '
 
 '
 
+test_expect_success 'reject adding remote with slash in name' '
+
+	test_must_fail git remote add multi/level .
+
+'
+
 # The first three test if the tracking branches are properly renamed,
 # the last two ones check if the config is updated.
 
@@ -668,6 +674,12 @@ test_expect_success 'rename a remote with name prefix of other remote' '
 
 '
 
+test_expect_success 'rename a remote with slash in name' '
+
+	test_must_fail git remote rename upstream up/stream
+
+'
+
 cat > remotes_origin << EOF
 URL: $(pwd)/one
 Push: refs/heads/master:refs/heads/upstream
-- 
1.8.1.3.704.g33f7d4f

  parent reply	other threads:[~2013-05-11 16:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-11 16:21 [PATCHv2 00/10] Prepare for alternative remote-tracking branch location Johan Herland
2013-05-11 16:21 ` [PATCHv2 01/10] t7900: Start testing usability of namespaced remote refs Johan Herland
2013-05-11 16:21 ` [PATCHv2 02/10] t7900: Demonstrate failure to expand "$peer/$branch" according to refspecs Johan Herland
2013-05-11 16:21 ` [PATCHv2 03/10] refs.c: Refactor code for mapping between shorthand names and full refnames Johan Herland
2013-05-13  4:56   ` Junio C Hamano
2013-05-13  6:31     ` Johan Herland
2013-05-13  6:45       ` Junio C Hamano
2013-05-13 20:34         ` Junio C Hamano
2013-05-14 14:24           ` Johan Herland
2013-05-14 17:50             ` Junio C Hamano
2013-05-15  6:45             ` Michael Haggerty
2013-05-15  7:39               ` Johan Herland
2013-05-15 13:53                 ` Johan Herland
2013-05-15 15:14                   ` Junio C Hamano
2013-05-15 19:49                   ` Eric Wong
2013-05-11 16:21 ` Johan Herland [this message]
2013-05-13  4:48   ` [PATCHv2 04/10] remote: Reject remote names containing '/' Eric Sunshine
2013-05-13  6:32     ` Johan Herland
2013-05-13  4:54   ` Junio C Hamano
2013-05-13  6:53     ` Johan Herland
2013-05-16  9:48       ` Ramkumar Ramachandra
2013-05-16 11:17         ` Johan Herland
2013-05-16 12:14           ` Ramkumar Ramachandra
2013-05-11 16:21 ` [PATCHv2 05/10] refs.c: Add support for expanding/shortening refs in refs/peers/* Johan Herland
2013-05-11 16:21 ` [PATCHv2 06/10] t7900: Test git branch -r/-a output w/remote-tracking branches " Johan Herland
2013-05-11 16:21 ` [PATCHv2 07/10] t3203: Add testcase for fix in 1603ade81352a526ccb206f41ff81ecbc855df2d Johan Herland
2013-05-11 16:21 ` [PATCHv2 08/10] builtin/branch.c: Refactor ref_item.name and .dest into strbufs Johan Herland
2013-05-11 16:21 ` [PATCHv2 09/10] builtin/branch.c: Refactor "remotes/" prepending to remote-tracking branches Johan Herland
2013-05-11 16:21 ` [PATCHv2 10/10] branch: Fix display of remote branches in refs/peers/* Johan Herland
2013-05-13  5:19   ` Eric Sunshine
2013-05-13  6:55     ` Johan Herland

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=1368289280-30337-5-git-send-email-johan@herland.net \
    --to=johan@herland.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).