git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH] disallow refs containing successive slashes
Date: Sat, 10 Oct 2009 19:49:48 +0200	[thread overview]
Message-ID: <4AD0C93C.6050306@web.de> (raw)

When creating branches using names starting with '/' or containing a '//',
a leading slash would just vanish and successive slashes were 'compressed'
into just one slash.

This behaviour comes from the refs being stored as files in subdirectories
where the filesystem compresses each '//' in a pathname to a '/' . So
disallowing refs to contain '//' fixes these problems and prohibits
branch names to start with a '/' or to contain '//'.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

I became aware of this issue while looking into problems occuring
when a user created a branch starting with a '/' in git gui (e.g.
"/foo"). Strange things happen, while git gui shows the current
branch as "/foo" under the hood a branch "foo" (without the slash)
had been created. But then you can't delete "/foo" from git gui,
because a branch of that name doesn't exist.

IMHO branch names should not be allowed to start with a '/' or
contain a '//' (branches with trailing slashes are not allowed since
commit 03feddd6). So this implies that refs should never contain a
'//' in the first place.

This behaviour has been changed the last time by commit 03feddd.
The version before that commit didn't allow '//' to be part of a
ref (but still allowed a trailing '/'). The newer version explicitly
states "/* tolerate duplicated slashes */" in the added code. But i
couldn't find any hints on the list or on the interweb *why* these
were allowed. So this patch might break something i can't see right
now, even though the test suite runs fine ...

Thoughts? Objections?


 Documentation/git-check-ref-format.txt |    2 ++
 refs.c                                 |    7 +++++--
 t/t1600-check-ref-format.sh            |   19 +++++++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100755 t/t1600-check-ref-format.sh

diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 0b7982e..180ffbb 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -38,6 +38,8 @@ imposes the following rules on how references are named:

 . They cannot end with a slash `/` nor a dot `.`.

+. They cannot contain two successive slashes `//`.
+
 . They cannot end with the sequence `.lock`.

 . They cannot contain a sequence `@{`.
diff --git a/refs.c b/refs.c
index 808f56b..cedd768 100644
--- a/refs.c
+++ b/refs.c
@@ -687,6 +687,7 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
  * - it has double dots "..", or
  * - it has ASCII control character, "~", "^", ":" or SP, anywhere, or
  * - it ends with a "/".
+ * - it contains '//'
  * - it ends with ".lock"
  * - it contains a "\" (backslash)
  */
@@ -712,8 +713,10 @@ int check_ref_format(const char *ref)

 	level = 0;
 	while (1) {
-		while ((ch = *cp++) == '/')
-			; /* tolerate duplicated slashes */
+		if ((ch = *cp++) == '/')
+			/* Don't tolerate successive slashes */
+			return CHECK_REF_FORMAT_ERROR;
+
 		if (!ch)
 			/* should not end with slashes */
 			return CHECK_REF_FORMAT_ERROR;
diff --git a/t/t1600-check-ref-format.sh b/t/t1600-check-ref-format.sh
new file mode 100755
index 0000000..05fb9f9
--- /dev/null
+++ b/t/t1600-check-ref-format.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+test_description='git check-ref-format'
+
+. ./test-lib.sh
+
+test_expect_success 'refs/heads/master is ok' '
+	git check-ref-format refs/heads/master
+'
+
+test_expect_success 'no successive slashes allowed' '
+	test_must_fail git check-ref-format a//b
+'
+
+test_expect_success 'no trailing slashes allowed' '
+	test_must_fail git check-ref-format a/b/
+'
+
+test_done
-- 
1.6.5.rc2.205.g1185a

             reply	other threads:[~2009-10-10 18:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-10 17:49 Jens Lehmann [this message]
2009-10-10 21:50 ` [PATCH] disallow refs containing successive slashes Junio C Hamano
2009-10-11 10:42   ` Jens Lehmann
2009-10-11 18:52     ` Junio C Hamano
2009-10-12  0:31       ` Jonathan Nieder
2009-10-12  2:47       ` Nanako Shiraishi
2009-10-12  5:25   ` [PATCH/RFC 0/4] plumbing to help fix git-gui Jonathan Nieder
2009-10-12  5:27     ` [PATCH 1/4] Add tests for git check-ref-format Jonathan Nieder
2009-10-12  5:28     ` [PATCH 2/4] Documentation: describe check-ref-format --branch Jonathan Nieder
2009-10-12  5:31     ` [PATCH/RFC 3/4] git check-ref-format --print Jonathan Nieder
2009-10-12 14:39       ` Shawn O. Pearce
2009-10-12 21:06         ` Junio C Hamano
2009-10-12 23:26           ` Shawn O. Pearce
2009-10-12 23:36       ` Junio C Hamano
2009-10-13  4:49         ` Jonathan Nieder
2009-10-12  5:33     ` [PATCH/RFC 4/4] check-ref-format: simplify --print implementation Jonathan Nieder
2009-10-12  5:45     ` [PATCH/RFC 0/4] plumbing to help fix git-gui Jonathan Nieder

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=4AD0C93C.6050306@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).