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

2012-07-16 Thread Michael Haggerty

On 07/16/2012 02:13 PM, Michael Schubert wrote:

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 msc...@elegosoft.com
---
  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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-07-16 Thread Junio C Hamano
Michael Schubert msc...@elegosoft.com writes:

 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 msc...@elegosoft.com
 ---
  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/);

The existing context lines above may give a clue why this patch is
not such a good idea.  We only limit HEAD to point under refs/ but
allow advanced users and scripts creative uses of other kinds of
symrefs.  Shouldn't the patch apply the new restriction only to HEAD
as well?

By the way, should git symbolic-ref _ HEAD work?

 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`
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html