[PATCH] refname format cleanup

DORMANTno replies

5 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] refname format cleanup

From: Michael Schubert <hidden>
Date: 2016-06-15 22:54:15 

Previous discussion:

 http://thread.gmane.org/gmane.comp.version-control.git/200129/focus=200146

I'm not sure if I've drawn the right conclusions from the previous
thread, so please let me know in case that's the wrong way to go..

 * refs: disallow ref components starting with hyphen
 * symbolic-ref: check format of given refname

 builtin/symbolic-ref.c  |  4 +++-
 builtin/tag.c           |  3 ---
 refs.c                  |  2 ++
 sha1_name.c             |  2 --
 t/t1401-symbolic-ref.sh | 10 ++++++++++
 5 files changed, 15 insertions(+), 6 deletions(-)

[PATCH 1/2] refs: disallow ref components starting with hyphen

From: Michael Schubert <hidden>
Date: 2016-06-15 22:54:15 

Currently, we allow refname components to start with a hyphen. There's
no good reason to do so and it troubles the parseopt infrastructure.
Explicitly refuse refname components starting with a hyphen inside
check_refname_component().

Revert 63486240, which is obsolete now.

Signed-off-by: Michael Schubert <redacted>
---
 builtin/tag.c | 3 ---
 refs.c        | 2 ++
 sha1_name.c   | 2 --
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 7b1be85..c99fb42 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -403,9 +403,6 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 
 static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
 {
-	if (name[0] == '-')
-		return -1;
-
 	strbuf_reset(sb);
 	strbuf_addf(sb, "refs/tags/%s", name);
 
diff --git a/refs.c b/refs.c
index da74a2b..5714681 100644
--- a/refs.c
+++ b/refs.c
@@ -62,6 +62,8 @@ static int check_refname_component(const char *refname, int flags)
 		if (refname[1] == '\0')
 			return -1; /* Component equals ".". */
 	}
+	if (refname[0] == '-')
+		return -1; /* Component starts with '-'. */
 	if (cp - refname >= 5 && !memcmp(cp - 5, ".lock", 5))
 		return -1; /* Refname ends with ".lock". */
 	return cp - refname;
diff --git a/sha1_name.c b/sha1_name.c
index 5d81ea0..132d369 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -892,8 +892,6 @@ int strbuf_branchname(struct strbuf *sb, const char *name)
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 {
 	strbuf_branchname(sb, name);
-	if (name[0] == '-')
-		return -1;
 	strbuf_splice(sb, 0, 0, "refs/heads/", 11);
 	return check_refname_format(sb->buf, 0);
 }
-- 
1.7.11.2.196.ga22866b

[PATCH 2/2] symbolic-ref: check format of given refname

From: Michael Schubert <hidden>
Date: 2016-06-15 22:54:15 

Currently, it's possible to update HEAD with a nonsense reference since
no strict validation ist performed. Example:

	$ git symbolic-ref HEAD 'refs/heads/master
    >
    >
    > '

Fix this by checking the given reference with check_refname_format().

Signed-off-by: Michael Schubert <redacted>
---
 builtin/symbolic-ref.c  |  4 +++-
 t/t1401-symbolic-ref.sh | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 801d62e..a529541 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -44,13 +44,15 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options,
 			     git_symbolic_ref_usage, 0);
-	if (msg &&!*msg)
+	if (msg && !*msg)
 		die("Refusing to perform update with empty message");
 	switch (argc) {
 	case 1:
 		check_symref(argv[0], quiet);
 		break;
 	case 2:
+		if (check_refname_format(argv[1], 0))
+			die("No valid reference format: '%s'", argv[1]);
 		if (!strcmp(argv[0], "HEAD") &&
 		    prefixcmp(argv[1], "refs/"))
 			die("Refusing to point HEAD outside of refs/");
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index 2c96551..b1cd508 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -27,6 +27,16 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
 '
 reset_to_sane
 
+test_expect_success 'symbolic-ref refuses ref with leading dot' '
+	test_must_fail git symbolic-ref HEAD refs/heads/.foo
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref refuses ref with leading dash' '
+	test_must_fail git symbolic-ref HEAD refs/heads/-foo
+'
+reset_to_sane
+
 test_expect_success 'symbolic-ref refuses bare sha1' '
 	echo content >file && git add file && git commit -m one &&
 	test_must_fail git symbolic-ref HEAD `git rev-parse HEAD`
-- 
1.7.11.2.196.ga22866b

Re: [PATCH 1/2] refs: disallow ref components starting with hyphen

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:54:15 

On 07/16/2012 02:13 PM, Michael Schubert wrote:
Currently, we allow refname components to start with a hyphen. There's
no good reason to do so and it troubles the parseopt infrastructure.
Explicitly refuse refname components starting with a hyphen inside
check_refname_component().
Your change to refs.c looks correct.  However, you should also update 
the documentation of the refname rules at the top of refs.c and also in

     Documentation/git-check-ref-format.txt

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 2/2] symbolic-ref: check format of given refname

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:54:15 

On 07/16/2012 02:13 PM, Michael Schubert wrote:
quoted hunk
Currently, it's possible to update HEAD with a nonsense reference since
no strict validation ist performed. Example:

	$ git symbolic-ref HEAD 'refs/heads/master
     >
     >
     > '

Fix this by checking the given reference with check_refname_format().

Signed-off-by: Michael Schubert <redacted>
---
  builtin/symbolic-ref.c  |  4 +++-
  t/t1401-symbolic-ref.sh | 10 ++++++++++
  2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 801d62e..a529541 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -44,13 +44,15 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
  	git_config(git_default_config, NULL);
  	argc = parse_options(argc, argv, prefix, options,
  			     git_symbolic_ref_usage, 0);
-	if (msg &&!*msg)
+	if (msg && !*msg)
  		die("Refusing to perform update with empty message");
  	switch (argc) {
  	case 1:
  		check_symref(argv[0], quiet);
  		break;
  	case 2:
+		if (check_refname_format(argv[1], 0))
+			die("No valid reference format: '%s'", argv[1]);
The error message is awkward.  I suggest something like

     "Reference name has invalid format: '%s'"

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help