{"thread":{"id":"28345","subject":"[PATCH 0/6] Improved infrastructure for refname normalization","startedAt":"2011-09-09T11:46:12Z","lastAt":"2011-09-10T04:04:41Z","messageCount":13,"participants":["Michael Haggerty","A Large Angry SCM","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"175172","messageId":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":null,"subject":"[PATCH 0/6] Improved infrastructure for refname normalization","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:12Z","receivedAt":"2011-09-09T11:46:12Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"As a prerequisite to storing references caches hierarchically (itself\nneeded for performance reasons), here is a patch series to help us get\nrefname normalization under control.\n\nThe problem is that some UI accepts unnormalized reference names (like\n\"/foo/bar\" or \"foo///bar\" instead of \"foo/bar\") and passes them on to\nlibrary routines without normalizing them.  The library, on the other\nhand, assumes that the refnames are normalized.  Sometimes (mostly in\nthe case of loose references) unnormalized refnames happen to work,\nbut in other cases (like packed references or when looking up refnames\nin the cache) they silently fail.  Given that refnames are sometimes\ntreated as path names, there is a chance that some security-relevant\nbugs are lurking in this area, if not in git proper then in scripts\nthat interact with git.\n\nThis patch series adds the following tools for dealing with refnames\nand their normalization (without actually doing much to fix the\nproblem; see below):\n\n* Fix check_ref_format() to make it easier and reliable to specify\n  which types of refnames are allowed in a particular situation\n  (multi-level vs. one-level, with vs. without a refspec-like \"*\",\n  normalized or unnormalized).\n\n* Add a function normalize_refname() that can check and optionally\n  normalize refnames according to the one true set of rules.\n\n* Add options to \"git check-ref-format\" to give scripts access to\n  these facilities (and to allow them to be tested in the test suite).\n\n* Forbid \".lock\" at the end of any refname component, as directories\n  with such names can conflict with attempts to create lock files for\n  other refnames.\n\n\nThe patches just provide the tools for handling refnames more\nconsistently.  How do we actually fix the problem of inconsistent\nhandling of refname normalization?  First we need policy.  I suggest\nthe following:\n\nUnnormalized refnames should only be accepted at the UI level and\nshould be normalized before use.  This can be done using code like\n\n    char normalized_refname[strlen(arg) + 1];\n    if (normalize_refname(normalized_refname, sizeof(normalized_refname),\n                           arg, REFNAME_ALLOW_UNNORMALIZED|other_flags))\n        die(\"invalid refname '%s'\", arg);\n\n    /* From now on, use normalized_refname. */\n\nRefnames coming from other sources, such as from a remote repository,\nshould be checked that they are in the correct *normalized* format,\nlike so:\n\n    if (check_ref_format(refname, other_flags))\n        die(\"invalid refname '%s'\", refname);\n\nRefnames from the local repository (e.g., from the packed references\nfile) should also be checked that they are in the correct normalized\nformat, though this policy could be debated if there are performance\nconcerns.\n\nRefnames probably do not need to be checked at the entrance to the\nrefs.{c,h} library functions because callers are responsible for not\npassing invalid or unnormalized refnames in.  However, some assert()s\nwould probably be justified, especially during the transition while we\nare fixing broken callers.\n\nIf there is agreement about this policy, I would be happy to write it\nup (presumably in Documentation/technical/api-ref.txt, maybe also\nincorporating the content from\nDocumentation/technical/api-ref-iteration.txt).\n\nI do not yet have enough global overview of the code to know which\ncallers need fixing, but once these tools are in place the callers can\nbe fixed incrementally.\n\nMichael Haggerty (6):\n  Change bad_ref_char() to return a boolean value\n  git check-ref-format: add options --onelevel-ok and --refname-pattern\n  Change check_ref_format() to take a flags argument\n  Add a library function normalize_refname()\n  Do not allow \".lock\" at the end of any refname component\n  Add a REFNAME_ALLOW_UNNORMALIZED flag to check_ref_format()\n\n Documentation/git-check-ref-format.txt |   42 ++++++++--\n builtin/check-ref-format.c             |   62 ++++++++--------\n builtin/checkout.c                     |    2 +-\n builtin/fetch-pack.c                   |    2 +-\n builtin/receive-pack.c                 |    3 +-\n builtin/replace.c                      |    2 +-\n builtin/show-ref.c                     |    2 +-\n builtin/tag.c                          |    4 +-\n connect.c                              |    2 +-\n environment.c                          |    2 +-\n fast-import.c                          |    7 +--\n notes-merge.c                          |    5 +-\n pack-refs.c                            |    2 +-\n refs.c                                 |  133 +++++++++++++++++++-------------\n refs.h                                 |   30 ++++++-\n remote.c                               |   55 ++++---------\n sha1_name.c                            |    4 +-\n t/t1402-check-ref-format.sh            |   93 ++++++++++++++++++++--\n transport.c                            |   18 ++---\n walker.c                               |    2 +-\n 20 files changed, 294 insertions(+), 178 deletions(-)\n\n-- \n1.7.6.8.gd2879\n"},{"id":"175176","messageId":"1315568778-3592-2-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 1/6] Change bad_ref_char() to return a boolean value","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:13Z","receivedAt":"2011-09-09T11:46:13Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Previously most bad characters were indicated by returning 1, but \"*\"\nwas special-cased to return 2 instead of 1.  One caller examined the\nreturn value to see whether the special case occurred.\n\nBut it is easier (to document and understand) for bad_ref_char()\nsimply to return a boolean value, treating \"*\" like any other bad\ncharacter.  Special-case the handling of \"*\" (which only occurs in\nvery specific circumstances) at the caller.  The resulting calling\ncode thereby also becomes more transparent.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nThis is just a random refactoring.\n\n refs.c |   15 ++++++---------\n 1 files changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a615043..fd29d89 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -860,22 +860,21 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n  * - it contains a \"\\\" (backslash)\n  */\n \n+/* Return true iff ch is not allowed in reference names. */\n static inline int bad_ref_char(int ch)\n {\n \tif (((unsigned) ch) <= ' ' || ch == 0x7f ||\n \t    ch == '~' || ch == '^' || ch == ':' || ch == '\\\\')\n \t\treturn 1;\n \t/* 2.13 Pattern Matching Notation */\n-\tif (ch == '?' || ch == '[') /* Unsupported */\n+\tif (ch == '*' || ch == '?' || ch == '[') /* Unsupported */\n \t\treturn 1;\n-\tif (ch == '*') /* Supported at the end */\n-\t\treturn 2;\n \treturn 0;\n }\n \n int check_ref_format(const char *ref)\n {\n-\tint ch, level, bad_type, last;\n+\tint ch, level, last;\n \tint ret = CHECK_REF_FORMAT_OK;\n \tconst char *cp = ref;\n \n@@ -890,9 +889,8 @@ int check_ref_format(const char *ref)\n \t\t/* we are at the beginning of the path component */\n \t\tif (ch == '.')\n \t\t\treturn CHECK_REF_FORMAT_ERROR;\n-\t\tbad_type = bad_ref_char(ch);\n-\t\tif (bad_type) {\n-\t\t\tif (bad_type == 2 && (!*cp || *cp == '/') &&\n+\t\tif (bad_ref_char(ch)) {\n+\t\t\tif (ch == '*' && (!*cp || *cp == '/') &&\n \t\t\t    ret == CHECK_REF_FORMAT_OK)\n \t\t\t\tret = CHECK_REF_FORMAT_WILDCARD;\n \t\t\telse\n@@ -902,8 +900,7 @@ int check_ref_format(const char *ref)\n \t\tlast = ch;\n \t\t/* scan the rest of the path component */\n \t\twhile ((ch = *cp++) != 0) {\n-\t\t\tbad_type = bad_ref_char(ch);\n-\t\t\tif (bad_type)\n+\t\t\tif (bad_ref_char(ch))\n \t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n \t\t\tif (ch == '/')\n \t\t\t\tbreak;\n-- \n1.7.6.8.gd2879\n"},{"id":"175174","messageId":"1315568778-3592-3-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 2/6] git check-ref-format: add options --onelevel-ok and --refname-pattern","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:14Z","receivedAt":"2011-09-09T11:46:14Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Also add tests of the new options.  (Actually, one big reason to add\nthe new options is to make it easy to test check_ref_format(), though\nthe options should also be useful to other scripts.)\n\nInterpret the result of check_ref_format() based on which types of\nrefnames are allowed.  However, because check_ref_format() can only\nreturn a single value, one test case is still broken.  Specifically,\nthe case \"git check-ref-format --onelevel '*'\" incorrectly succeeds\nbecause check_ref_format() returns CHECK_REF_FORMAT_ONELEVEL for this\nrefname even though the refname is also CHECK_REF_FORMAT_WILDCARD.\nThe type of check that leads to this failure is used elsewhere in\n\"real\" code and could lead to bugs; it will be fixed over the next few\ncommits.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n\nThe failing test is expressed awkwardly in\nt/t1402-check-ref-format.sh, but since it will be fixed in the next\ncommit it doesn't seem worth investing any work in it.\n\n Documentation/git-check-ref-format.txt |   29 +++++++++--\n builtin/check-ref-format.c             |   56 +++++++++++++++++---\n t/t1402-check-ref-format.sh            |   87 +++++++++++++++++++++++++++++---\n 3 files changed, 151 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex c9fdf84..3ab22b9 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -8,8 +8,8 @@ git-check-ref-format - Ensures that a reference name is well formed\n SYNOPSIS\n --------\n [verse]\n-'git check-ref-format' <refname>\n-'git check-ref-format' --print <refname>\n+'git check-ref-format' [--print]\n+       [--[no-]allow-onelevel] [--refspec-pattern] <refname>\n 'git check-ref-format' --branch <branchname-shorthand>\n \n DESCRIPTION\n@@ -32,14 +32,18 @@ git imposes the following rules on how references are named:\n \n . They must contain at least one `/`. This enforces the presence of a\n   category like `heads/`, `tags/` etc. but the actual names are not\n-  restricted.\n+  restricted.  If the `--allow-onelevel` option is used, this rule\n+  is waived.\n \n . They cannot have two consecutive dots `..` anywhere.\n \n . They cannot have ASCII control characters (i.e. bytes whose\n   values are lower than \\040, or \\177 `DEL`), space, tilde `~`,\n-  caret `{caret}`, colon `:`, question-mark `?`, asterisk `*`,\n-  or open bracket `[` anywhere.\n+  caret `{caret}`, or colon `:` anywhere.\n+\n+. They cannot have question-mark `?`, asterisk `*`, or open bracket\n+  `[` anywhere.  See the `--refspec-pattern` option below for an\n+  exception to this rule.\n \n . They cannot end with a slash `/` nor a dot `.`.\n \n@@ -78,6 +82,21 @@ were on.  This option should be used by porcelains to accept this\n syntax anywhere a branch name is expected, so they can act as if you\n typed the branch name.\n \n+OPTIONS\n+-------\n+--allow-onelevel::\n+--no-allow-onelevel::\n+\tControls whether one-level refnames are accepted (i.e.,\n+\trefnames that do not contain multiple `/`-separated\n+\tcomponents).  The default is `--no-allow-onelevel`.\n+\n+--refspec-pattern::\n+\tInterpret <refname> as a reference name pattern for a refspec\n+\t(as used with remote repositories).  If this option is\n+\tenabled, <refname> is allowed to contain a single `*` in place\n+\tof a one full pathname component (e.g., `foo/*/bar` but not\n+\t`foo/bar*`).\n+\n EXAMPLES\n --------\n \ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex 0723cf2..6bb9377 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -8,7 +8,7 @@\n #include \"strbuf.h\"\n \n static const char builtin_check_ref_format_usage[] =\n-\"git check-ref-format [--print] <refname>\\n\"\n+\"git check-ref-format [--print] [options] <refname>\\n\"\n \"   or: git check-ref-format --branch <branchname-shorthand>\";\n \n /*\n@@ -45,27 +45,65 @@ static int check_ref_format_branch(const char *arg)\n \treturn 0;\n }\n \n-static int check_ref_format_print(const char *arg)\n+static void refname_format_print(const char *arg)\n {\n \tchar *refname = xmalloc(strlen(arg) + 1);\n \n-\tif (check_ref_format(arg))\n-\t\treturn 1;\n \tcollapse_slashes(refname, arg);\n \tprintf(\"%s\\n\", refname);\n-\treturn 0;\n }\n \n+#define REFNAME_ALLOW_ONELEVEL 1\n+#define REFNAME_REFSPEC_PATTERN 2\n+\n int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n {\n+\tint i;\n+\tint print = 0;\n+\tint flags = 0;\n+\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(builtin_check_ref_format_usage);\n \n \tif (argc == 3 && !strcmp(argv[1], \"--branch\"))\n \t\treturn check_ref_format_branch(argv[2]);\n-\tif (argc == 3 && !strcmp(argv[1], \"--print\"))\n-\t\treturn check_ref_format_print(argv[2]);\n-\tif (argc != 2)\n+\n+\tfor (i = 1; i < argc && argv[i][0] == '-'; ++i) {\n+\t\tif (!strcmp(argv[i], \"--print\"))\n+\t\t\tprint = 1;\n+\t\telse if (!strcmp(argv[i], \"--allow-onelevel\"))\n+\t\t\tflags |= REFNAME_ALLOW_ONELEVEL;\n+\t\telse if (!strcmp(argv[i], \"--no-allow-onelevel\"))\n+\t\t\tflags &= ~REFNAME_ALLOW_ONELEVEL;\n+\t\telse if (!strcmp(argv[i], \"--refspec-pattern\"))\n+\t\t\tflags |= REFNAME_REFSPEC_PATTERN;\n+\t\telse\n+\t\t\tusage(builtin_check_ref_format_usage);\n+\t}\n+\tif (! (i == argc - 1))\n \t\tusage(builtin_check_ref_format_usage);\n-\treturn !!check_ref_format(argv[1]);\n+\n+\tswitch (check_ref_format(argv[i])) {\n+\tcase CHECK_REF_FORMAT_OK:\n+\t\tbreak;\n+\tcase CHECK_REF_FORMAT_ERROR:\n+\t\treturn 1;\n+\tcase CHECK_REF_FORMAT_ONELEVEL:\n+\t\tif (!(flags & REFNAME_ALLOW_ONELEVEL))\n+\t\t\treturn 1;\n+\t\telse\n+\t\t\tbreak;\n+\tcase CHECK_REF_FORMAT_WILDCARD:\n+\t\tif (!(flags & REFNAME_REFSPEC_PATTERN))\n+\t\t\treturn 1;\n+\t\telse\n+\t\t\tbreak;\n+\tdefault:\n+\t\tdie(\"internal error: unexpected value from check_ref_format()\");\n+\t}\n+\n+\tif (print)\n+\t\trefname_format_print(argv[i]);\n+\n+\treturn 0;\n }\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex ed4275a..95dcd31 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -5,23 +5,35 @@ test_description='Test git check-ref-format'\n . ./test-lib.sh\n \n valid_ref() {\n-\ttest_expect_success \"ref name '$1' is valid\" \\\n-\t\t\"git check-ref-format '$1'\"\n+\tif test \"$#\" = 1\n+\tthen\n+\t\ttest_expect_success \"ref name '$1' is valid\" \\\n+\t\t\t\"git check-ref-format '$1'\"\n+\telse\n+\t\ttest_expect_success \"ref name '$1' is valid with options $2\" \\\n+\t\t\t\"git check-ref-format $2 '$1'\"\n+\tfi\n }\n invalid_ref() {\n-\ttest_expect_success \"ref name '$1' is not valid\" \\\n-\t\t\"test_must_fail git check-ref-format '$1'\"\n+\tif test \"$#\" = 1\n+\tthen\n+\t\ttest_expect_success \"ref name '$1' is invalid\" \\\n+\t\t\t\"test_must_fail git check-ref-format '$1'\"\n+\telse\n+\t\ttest_expect_success \"ref name '$1' is invalid with options $2\" \\\n+\t\t\t\"test_must_fail git check-ref-format $2 '$1'\"\n+\tfi\n }\n \n-valid_ref 'heads/foo'\n-invalid_ref 'foo'\n valid_ref 'foo/bar/baz'\n valid_ref 'refs///heads/foo'\n invalid_ref 'heads/foo/'\n valid_ref '/heads/foo'\n valid_ref '///heads/foo'\n-invalid_ref '/foo'\n invalid_ref './foo'\n+invalid_ref './foo/bar'\n+invalid_ref 'foo/./bar'\n+invalid_ref 'foo/bar/.'\n invalid_ref '.refs/foo'\n invalid_ref 'heads/foo..bar'\n invalid_ref 'heads/foo?bar'\n@@ -33,6 +45,67 @@ invalid_ref 'heads/foo\\bar'\n invalid_ref \"$(printf 'heads/foo\\t')\"\n invalid_ref \"$(printf 'heads/foo\\177')\"\n valid_ref \"$(printf 'heads/fu\\303\\237')\"\n+invalid_ref 'heads/*foo/bar' --refspec-pattern\n+invalid_ref 'heads/foo*/bar' --refspec-pattern\n+invalid_ref 'heads/f*o/bar' --refspec-pattern\n+\n+ref='foo'\n+invalid_ref \"$ref\"\n+valid_ref \"$ref\" --allow-onelevel\n+invalid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='foo/bar'\n+valid_ref \"$ref\"\n+valid_ref \"$ref\" --allow-onelevel\n+valid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='foo/*'\n+invalid_ref \"$ref\"\n+invalid_ref \"$ref\" --allow-onelevel\n+valid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='*/foo'\n+invalid_ref \"$ref\"\n+invalid_ref \"$ref\" --allow-onelevel\n+valid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='foo/*/bar'\n+invalid_ref \"$ref\"\n+invalid_ref \"$ref\" --allow-onelevel\n+valid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='*'\n+invalid_ref \"$ref\"\n+\n+#invalid_ref \"$ref\" --allow-onelevel\n+test_expect_failure \"ref name '$ref' is invalid with options --allow-onelevel\" \\\n+\t\"test_must_fail git check-ref-format --allow-onelevel '$ref'\"\n+\n+invalid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='foo/*/*'\n+invalid_ref \"$ref\" --refspec-pattern\n+invalid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='*/foo/*'\n+invalid_ref \"$ref\" --refspec-pattern\n+invalid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='*/*/foo'\n+invalid_ref \"$ref\" --refspec-pattern\n+invalid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n+\n+ref='/foo'\n+invalid_ref \"$ref\"\n+valid_ref \"$ref\" --allow-onelevel\n+invalid_ref \"$ref\" --refspec-pattern\n+valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n \n test_expect_success \"check-ref-format --branch @{-1}\" '\n \tT=$(git write-tree) &&\n-- \n1.7.6.8.gd2879\n"},{"id":"175175","messageId":"1315568778-3592-4-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 3/6] Change check_ref_format() to take a flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:15Z","receivedAt":"2011-09-09T11:46:15Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change check_ref_format() to take a flags argument that indicates what\nis acceptable in the reference name (analogous to --allow-onelevel and\n--refspec-pattern).  This is more convenient for callers and also\nfixes a failure in the test suite (and likely elsewhere in the code)\nby enabling \"onelevel\" and \"refspec-pattern\" to be allowed\nindependently of each other.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/check-ref-format.c  |   21 +----------------\n builtin/checkout.c          |    2 +-\n builtin/fetch-pack.c        |    2 +-\n builtin/receive-pack.c      |    2 +-\n builtin/replace.c           |    2 +-\n builtin/show-ref.c          |    2 +-\n builtin/tag.c               |    4 +-\n connect.c                   |    2 +-\n environment.c               |    2 +-\n fast-import.c               |    7 +-----\n notes-merge.c               |    5 ++-\n pack-refs.c                 |    2 +-\n refs.c                      |   42 +++++++++++++++------------------\n refs.h                      |   17 +++++++++----\n remote.c                    |   53 +++++++++++-------------------------------\n sha1_name.c                 |    4 +-\n t/t1402-check-ref-format.sh |    6 +----\n transport.c                 |   15 ++---------\n walker.c                    |    2 +-\n 19 files changed, 67 insertions(+), 125 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex 6bb9377..c639400 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -53,9 +53,6 @@ static void refname_format_print(const char *arg)\n \tprintf(\"%s\\n\", refname);\n }\n \n-#define REFNAME_ALLOW_ONELEVEL 1\n-#define REFNAME_REFSPEC_PATTERN 2\n-\n int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -83,24 +80,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \tif (! (i == argc - 1))\n \t\tusage(builtin_check_ref_format_usage);\n \n-\tswitch (check_ref_format(argv[i])) {\n-\tcase CHECK_REF_FORMAT_OK:\n-\t\tbreak;\n-\tcase CHECK_REF_FORMAT_ERROR:\n+\tif (check_ref_format(argv[i], flags))\n \t\treturn 1;\n-\tcase CHECK_REF_FORMAT_ONELEVEL:\n-\t\tif (!(flags & REFNAME_ALLOW_ONELEVEL))\n-\t\t\treturn 1;\n-\t\telse\n-\t\t\tbreak;\n-\tcase CHECK_REF_FORMAT_WILDCARD:\n-\t\tif (!(flags & REFNAME_REFSPEC_PATTERN))\n-\t\t\treturn 1;\n-\t\telse\n-\t\t\tbreak;\n-\tdefault:\n-\t\tdie(\"internal error: unexpected value from check_ref_format()\");\n-\t}\n \n \tif (print)\n \t\trefname_format_print(argv[i]);\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3bb6525..2882116 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \tnew->name = arg;\n \tsetup_branch_path(new);\n \n-\tif (check_ref_format(new->path) == CHECK_REF_FORMAT_OK &&\n+\tif (!check_ref_format(new->path, 0) &&\n \t    resolve_ref(new->path, branch_rev, 1, NULL))\n \t\thashcpy(rev, branch_rev);\n \telse\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 412bd32..411aa7d 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \tfor (ref = *refs; ref; ref = next) {\n \t\tnext = ref->next;\n \t\tif (!memcmp(ref->name, \"refs/\", 5) &&\n-\t\t    check_ref_format(ref->name + 5))\n+\t\t    check_ref_format(ref->name + 5, 0))\n \t\t\t; /* trash */\n \t\telse if (args.fetch_all &&\n \t\t\t (!args.depth || prefixcmp(ref->name, \"refs/tags/\") )) {\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ae164da..4e880ef 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)\n \tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n-\tif (prefixcmp(name, \"refs/\") || check_ref_format(name + 5)) {\n+\tif (prefixcmp(name, \"refs/\") || check_ref_format(name + 5, 0)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n \t\treturn \"funny refname\";\n \t}\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex fe3a647..15f0e5e 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,\n \t\t     \"refs/replace/%s\",\n \t\t     sha1_to_hex(object)) > sizeof(ref) - 1)\n \t\tdie(\"replace ref name too long: %.*s...\", 50, ref);\n-\tif (check_ref_format(ref))\n+\tif (check_ref_format(ref, 0))\n \t\tdie(\"'%s' is not a valid ref name.\", ref);\n \n \tif (!resolve_ref(ref, prev, 1, NULL))\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 45f0340..375a14b 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)\n \t\t\tif (strncmp(ref, match, matchlen))\n \t\t\t\tcontinue;\n \t\t}\n-\t\tif (check_ref_format(ref)) {\n+\t\tif (check_ref_format(ref, 0)) {\n \t\t\twarning(\"ref '%s' ignored\", ref);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 667515e..7aceaab 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -407,12 +407,12 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)\n static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n {\n \tif (name[0] == '-')\n-\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\treturn -1;\n \n \tstrbuf_reset(sb);\n \tstrbuf_addf(sb, \"refs/tags/%s\", name);\n \n-\treturn check_ref_format(sb->buf);\n+\treturn check_ref_format(sb->buf, 0);\n }\n \n int cmd_tag(int argc, const char **argv, const char *prefix)\ndiff --git a/connect.c b/connect.c\nindex ee1d4b4..292a9e2 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)\n \tlen -= 5;\n \n \t/* REF_NORMAL means that we don't want the magic fake tag refs */\n-\tif ((flags & REF_NORMAL) && check_ref_format(name) < 0)\n+\tif ((flags & REF_NORMAL) && check_ref_format(name, 0))\n \t\treturn 0;\n \n \t/* REF_HEADS means that we want regular branch heads */\ndiff --git a/environment.c b/environment.c\nindex e96edcf..8acbb87 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)\n \t\tif (strcmp((*c)->buf, \"/\") != 0)\n \t\t\tstrbuf_addf(&buf, \"refs/namespaces/%s\", (*c)->buf);\n \tstrbuf_list_free(components);\n-\tif (check_ref_format(buf.buf) != CHECK_REF_FORMAT_OK)\n+\tif (check_ref_format(buf.buf, 0))\n \t\tdie(\"bad git namespace path \\\"%s\\\"\", raw_namespace);\n \tstrbuf_addch(&buf, '/');\n \treturn strbuf_detach(&buf, NULL);\ndiff --git a/fast-import.c b/fast-import.c\nindex 742e7da..4d55ee6 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -722,13 +722,8 @@ static struct branch *new_branch(const char *name)\n \n \tif (b)\n \t\tdie(\"Invalid attempt to create duplicate branch: %s\", name);\n-\tswitch (check_ref_format(name)) {\n-\tcase 0: break; /* its valid */\n-\tcase CHECK_REF_FORMAT_ONELEVEL:\n-\t\tbreak; /* valid, but too few '/', allow anyway */\n-\tdefault:\n+\tif (check_ref_format(name, REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n-\t}\n \n \tb = pool_calloc(1, sizeof(struct branch));\n \tb->name = pool_strdup(name);\ndiff --git a/notes-merge.c b/notes-merge.c\nindex e1aaf43..bb8d7c8 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -570,7 +570,8 @@ int notes_merge(struct notes_merge_options *o,\n \t/* Dereference o->local_ref into local_sha1 */\n \tif (!resolve_ref(o->local_ref, local_sha1, 0, NULL))\n \t\tdie(\"Failed to resolve local notes ref '%s'\", o->local_ref);\n-\telse if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))\n+\telse if (!check_ref_format(o->local_ref, 0) &&\n+\t\tis_null_sha1(local_sha1))\n \t\tlocal = NULL; /* local_sha1 == null_sha1 indicates unborn ref */\n \telse if (!(local = lookup_commit_reference(local_sha1)))\n \t\tdie(\"Could not parse local commit %s (%s)\",\n@@ -583,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,\n \t\t * Failed to get remote_sha1. If o->remote_ref looks like an\n \t\t * unborn ref, perform the merge using an empty notes tree.\n \t\t */\n-\t\tif (!check_ref_format(o->remote_ref)) {\n+\t\tif (!check_ref_format(o->remote_ref, 0)) {\n \t\t\thashclr(remote_sha1);\n \t\t\tremote = NULL;\n \t\t} else {\ndiff --git a/pack-refs.c b/pack-refs.c\nindex 1290570..bc0032d 100644\n--- a/pack-refs.c\n+++ b/pack-refs.c\n@@ -72,7 +72,7 @@ static void try_remove_empty_parents(char *name)\n \tfor (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */\n \t\twhile (*p && *p != '/')\n \t\t\tp++;\n-\t\t/* tolerate duplicate slashes; see check_ref_format() */\n+\t\t/* tolerate duplicate slashes; see normalize_refname() */\n \t\twhile (*p == '/')\n \t\t\tp++;\n \t}\ndiff --git a/refs.c b/refs.c\nindex fd29d89..a206a4c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -872,10 +872,9 @@ static inline int bad_ref_char(int ch)\n \treturn 0;\n }\n \n-int check_ref_format(const char *ref)\n+int check_ref_format(const char *ref, int flags)\n {\n \tint ch, level, last;\n-\tint ret = CHECK_REF_FORMAT_OK;\n \tconst char *cp = ref;\n \n \tlevel = 0;\n@@ -884,41 +883,42 @@ int check_ref_format(const char *ref)\n \t\t\t; /* tolerate duplicated slashes */\n \t\tif (!ch)\n \t\t\t/* should not end with slashes */\n-\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\t\treturn -1;\n \n \t\t/* we are at the beginning of the path component */\n \t\tif (ch == '.')\n-\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\t\treturn -1;\n \t\tif (bad_ref_char(ch)) {\n-\t\t\tif (ch == '*' && (!*cp || *cp == '/') &&\n-\t\t\t    ret == CHECK_REF_FORMAT_OK)\n-\t\t\t\tret = CHECK_REF_FORMAT_WILDCARD;\n+\t\t\tif ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&\n+\t\t\t\t(!*cp || *cp == '/'))\n+\t\t\t\t/* Accept one wildcard as a full refname component. */\n+\t\t\t\tflags &= ~REFNAME_REFSPEC_PATTERN;\n \t\t\telse\n-\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\t\t\treturn -1;\n \t\t}\n \n \t\tlast = ch;\n \t\t/* scan the rest of the path component */\n \t\twhile ((ch = *cp++) != 0) {\n \t\t\tif (bad_ref_char(ch))\n-\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\t\t\treturn -1;\n \t\t\tif (ch == '/')\n \t\t\t\tbreak;\n \t\t\tif (last == '.' && ch == '.')\n-\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\t\t\treturn -1;\n \t\t\tif (last == '@' && ch == '{')\n-\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\t\t\treturn -1;\n \t\t\tlast = ch;\n \t\t}\n \t\tlevel++;\n \t\tif (!ch) {\n \t\t\tif (ref <= cp - 2 && cp[-2] == '.')\n-\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n-\t\t\tif (level < 2)\n-\t\t\t\treturn CHECK_REF_FORMAT_ONELEVEL;\n+\t\t\t\treturn -1;\n+\t\t\tif (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))\n+\t\t\t\treturn -1;\n \t\t\tif (has_extension(ref, \".lock\"))\n-\t\t\t\treturn CHECK_REF_FORMAT_ERROR;\n-\t\t\treturn ret;\n+\t\t\t\treturn -1;\n+\t\t\treturn 0;\n \t\t}\n \t}\n }\n@@ -1103,7 +1103,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char\n struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)\n {\n \tchar refpath[PATH_MAX];\n-\tif (check_ref_format(ref))\n+\tif (check_ref_format(ref, 0))\n \t\treturn NULL;\n \tstrcpy(refpath, mkpath(\"refs/%s\", ref));\n \treturn lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);\n@@ -1111,13 +1111,9 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)\n \n struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)\n {\n-\tswitch (check_ref_format(ref)) {\n-\tdefault:\n+\tif (check_ref_format(ref, REFNAME_ALLOW_ONELEVEL))\n \t\treturn NULL;\n-\tcase 0:\n-\tcase CHECK_REF_FORMAT_ONELEVEL:\n-\t\treturn lock_ref_sha1_basic(ref, old_sha1, flags, NULL);\n-\t}\n+\treturn lock_ref_sha1_basic(ref, old_sha1, flags, NULL);\n }\n \n static struct lock_file packlock;\ndiff --git a/refs.h b/refs.h\nindex dfb086e..b248ce6 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -97,11 +97,18 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi\n  */\n extern int for_each_reflog(each_ref_fn, void *);\n \n-#define CHECK_REF_FORMAT_OK 0\n-#define CHECK_REF_FORMAT_ERROR (-1)\n-#define CHECK_REF_FORMAT_ONELEVEL (-2)\n-#define CHECK_REF_FORMAT_WILDCARD (-3)\n-extern int check_ref_format(const char *target);\n+#define REFNAME_ALLOW_ONELEVEL 1\n+#define REFNAME_REFSPEC_PATTERN 2\n+\n+/*\n+ * Return 0 iff ref has the correct format for a refname according to\n+ * the rules described in Documentation/git-check-ref-format.txt.  If\n+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level\n+ * reference names.  If REFNAME_REFSPEC_PATTERN is set in flags, then\n+ * allow a \"*\" wildcard character in place of one of the name\n+ * components.\n+ */\n+extern int check_ref_format(const char *target, int flags);\n \n extern const char *prettify_refname(const char *refname);\n extern char *shorten_unambiguous_ref(const char *ref, int strict);\ndiff --git a/remote.c b/remote.c\nindex b8ecfa5..7059885 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -493,23 +493,6 @@ static void read_config(void)\n }\n \n /*\n- * We need to make sure the remote-tracking branches are well formed, but a\n- * wildcard refspec in \"struct refspec\" must have a trailing slash. We\n- * temporarily drop the trailing '/' while calling check_ref_format(),\n- * and put it back.  The caller knows that a CHECK_REF_FORMAT_ONELEVEL\n- * error return is Ok for a wildcard refspec.\n- */\n-static int verify_refname(char *name, int is_glob)\n-{\n-\tint result;\n-\n-\tresult = check_ref_format(name);\n-\tif (is_glob && result == CHECK_REF_FORMAT_WILDCARD)\n-\t\tresult = CHECK_REF_FORMAT_OK;\n-\treturn result;\n-}\n-\n-/*\n  * This function frees a refspec array.\n  * Warning: code paths should be checked to ensure that the src\n  *          and dst pointers are always freeable pointers as well\n@@ -532,13 +515,13 @@ static void free_refspecs(struct refspec *refspec, int nr_refspec)\n static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)\n {\n \tint i;\n-\tint st;\n \tstruct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);\n \n \tfor (i = 0; i < nr_refspec; i++) {\n \t\tsize_t llen;\n \t\tint is_glob;\n \t\tconst char *lhs, *rhs;\n+\t\tint flags;\n \n \t\tis_glob = 0;\n \n@@ -576,6 +559,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \n \t\trs[i].pattern = is_glob;\n \t\trs[i].src = xstrndup(lhs, llen);\n+\t\tflags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);\n \n \t\tif (fetch) {\n \t\t\t/*\n@@ -585,26 +569,20 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\t */\n \t\t\tif (!*rs[i].src)\n \t\t\t\t; /* empty is ok */\n-\t\t\telse {\n-\t\t\t\tst = verify_refname(rs[i].src, is_glob);\n-\t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n-\t\t\t\t\tgoto invalid;\n-\t\t\t}\n+\t\t\telse if (check_ref_format(rs[i].src, flags))\n+\t\t\t\tgoto invalid;\n \t\t\t/*\n \t\t\t * RHS\n \t\t\t * - missing is ok, and is same as empty.\n \t\t\t * - empty is ok; it means not to store.\n \t\t\t * - otherwise it must be a valid looking ref.\n \t\t\t */\n-\t\t\tif (!rs[i].dst) {\n+\t\t\tif (!rs[i].dst)\n \t\t\t\t; /* ok */\n-\t\t\t} else if (!*rs[i].dst) {\n+\t\t\telse if (!*rs[i].dst)\n \t\t\t\t; /* ok */\n-\t\t\t} else {\n-\t\t\t\tst = verify_refname(rs[i].dst, is_glob);\n-\t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n-\t\t\t\t\tgoto invalid;\n-\t\t\t}\n+\t\t\telse if (check_ref_format(rs[i].dst, flags))\n+\t\t\t\tgoto invalid;\n \t\t} else {\n \t\t\t/*\n \t\t\t * LHS\n@@ -616,8 +594,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\tif (!*rs[i].src)\n \t\t\t\t; /* empty is ok */\n \t\t\telse if (is_glob) {\n-\t\t\t\tst = verify_refname(rs[i].src, is_glob);\n-\t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n+\t\t\t\tif (check_ref_format(rs[i].src, flags))\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n \t\t\telse\n@@ -630,14 +607,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\t * - otherwise it must be a valid looking ref.\n \t\t\t */\n \t\t\tif (!rs[i].dst) {\n-\t\t\t\tst = verify_refname(rs[i].src, is_glob);\n-\t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n+\t\t\t\tif (check_ref_format(rs[i].src, flags))\n \t\t\t\t\tgoto invalid;\n \t\t\t} else if (!*rs[i].dst) {\n \t\t\t\tgoto invalid;\n \t\t\t} else {\n-\t\t\t\tst = verify_refname(rs[i].dst, is_glob);\n-\t\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL)\n+\t\t\t\tif (check_ref_format(rs[i].dst, flags))\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n \t\t}\n@@ -1427,8 +1402,8 @@ int get_fetch_map(const struct ref *remote_refs,\n \n \tfor (rmp = &ref_map; *rmp; ) {\n \t\tif ((*rmp)->peer_ref) {\n-\t\t\tint st = check_ref_format((*rmp)->peer_ref->name + 5);\n-\t\t\tif (st && st != CHECK_REF_FORMAT_ONELEVEL) {\n+\t\t\tif (check_ref_format((*rmp)->peer_ref->name + 5,\n+\t\t\t\tREFNAME_ALLOW_ONELEVEL)) {\n \t\t\t\tstruct ref *ignore = *rmp;\n \t\t\t\terror(\"* Ignoring funny ref '%s' locally\",\n \t\t\t\t      (*rmp)->peer_ref->name);\n@@ -1620,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla\n \tint len;\n \n \t/* we already know it starts with refs/ to get here */\n-\tif (check_ref_format(refname + 5))\n+\tif (check_ref_format(refname + 5, 0))\n \t\treturn 0;\n \n \tlen = strlen(refname) + 1;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex ff5992a..975ec3b 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -972,9 +972,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n {\n \tstrbuf_branchname(sb, name);\n \tif (name[0] == '-')\n-\t\treturn CHECK_REF_FORMAT_ERROR;\n+\t\treturn -1;\n \tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n-\treturn check_ref_format(sb->buf);\n+\treturn check_ref_format(sb->buf, 0);\n }\n \n /*\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 95dcd31..6ac5025 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -81,11 +81,7 @@ valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n \n ref='*'\n invalid_ref \"$ref\"\n-\n-#invalid_ref \"$ref\" --allow-onelevel\n-test_expect_failure \"ref name '$ref' is invalid with options --allow-onelevel\" \\\n-\t\"test_must_fail git check-ref-format --allow-onelevel '$ref'\"\n-\n+invalid_ref \"$ref\" --allow-onelevel\n invalid_ref \"$ref\" --refspec-pattern\n valid_ref \"$ref\" '--refspec-pattern --allow-onelevel'\n \ndiff --git a/transport.c b/transport.c\nindex fa279d5..225d9b8 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -754,18 +754,9 @@ void transport_verify_remote_names(int nr_heads, const char **heads)\n \t\t\tcontinue;\n \n \t\tremote = remote ? (remote + 1) : local;\n-\t\tswitch (check_ref_format(remote)) {\n-\t\tcase 0: /* ok */\n-\t\tcase CHECK_REF_FORMAT_ONELEVEL:\n-\t\t\t/* ok but a single level -- that is fine for\n-\t\t\t * a match pattern.\n-\t\t\t */\n-\t\tcase CHECK_REF_FORMAT_WILDCARD:\n-\t\t\t/* ok but ends with a pattern-match character */\n-\t\t\tcontinue;\n-\t\t}\n-\t\tdie(\"remote part of refspec is not a valid name in %s\",\n-\t\t    heads[i]);\n+\t\tif (check_ref_format(remote, REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))\n+\t\t\tdie(\"remote part of refspec is not a valid name in %s\",\n+\t\t\t\theads[i]);\n \t}\n }\n \ndiff --git a/walker.c b/walker.c\nindex dce7128..e5d8eb2 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *\n {\n \tif (!get_sha1_hex(target, sha1))\n \t\treturn 0;\n-\tif (!check_ref_format(target)) {\n+\tif (!check_ref_format(target, 0)) {\n \t\tstruct ref *ref = alloc_ref(target);\n \t\tif (!walker->fetch_ref(walker, ref)) {\n \t\t\thashcpy(sha1, ref->old_sha1);\n-- \n1.7.6.8.gd2879\n"},{"id":"175173","messageId":"1315568778-3592-5-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 4/6] Add a library function normalize_refname()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:16Z","receivedAt":"2011-09-09T11:46:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Implement check_ref_format() using the new function.  Also use it to\nreplace collapse_slashes() in check-ref-format.c.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n builtin/check-ref-format.c  |   41 ++++-------------\n refs.c                      |  109 +++++++++++++++++++++++++++----------------\n refs.h                      |   24 +++++++---\n t/t1402-check-ref-format.sh |    3 +\n 4 files changed, 98 insertions(+), 79 deletions(-)\n\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex c639400..4c202af 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -11,28 +11,6 @@ static const char builtin_check_ref_format_usage[] =\n \"git check-ref-format [--print] [options] <refname>\\n\"\n \"   or: git check-ref-format --branch <branchname-shorthand>\";\n \n-/*\n- * Remove leading slashes and replace each run of adjacent slashes in\n- * src with a single slash, and write the result to dst.\n- *\n- * This function is similar to normalize_path_copy(), but stripped down\n- * to meet check_ref_format's simpler needs.\n- */\n-static void collapse_slashes(char *dst, const char *src)\n-{\n-\tchar ch;\n-\tchar prev = '/';\n-\n-\twhile ((ch = *src++) != '\\0') {\n-\t\tif (prev == '/' && ch == prev)\n-\t\t\tcontinue;\n-\n-\t\t*dst++ = ch;\n-\t\tprev = ch;\n-\t}\n-\t*dst = '\\0';\n-}\n-\n static int check_ref_format_branch(const char *arg)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -45,12 +23,15 @@ static int check_ref_format_branch(const char *arg)\n \treturn 0;\n }\n \n-static void refname_format_print(const char *arg)\n+static int check_ref_format_print(const char *arg, int flags)\n {\n-\tchar *refname = xmalloc(strlen(arg) + 1);\n+\tint refnamelen = strlen(arg) + 1;\n+\tchar *refname = xmalloc(refnamelen);\n \n-\tcollapse_slashes(refname, arg);\n+\tif (normalize_refname(refname, refnamelen, arg, flags))\n+\t\treturn 1;\n \tprintf(\"%s\\n\", refname);\n+\treturn 0;\n }\n \n int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n@@ -79,12 +60,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t}\n \tif (! (i == argc - 1))\n \t\tusage(builtin_check_ref_format_usage);\n-\n-\tif (check_ref_format(argv[i], flags))\n-\t\treturn 1;\n-\n \tif (print)\n-\t\trefname_format_print(argv[i]);\n-\n-\treturn 0;\n+\t\treturn check_ref_format_print(argv[i], flags);\n+\telse\n+\t\treturn !!check_ref_format(argv[i], flags);\n }\ndiff --git a/refs.c b/refs.c\nindex a206a4c..372350e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -872,55 +872,84 @@ static inline int bad_ref_char(int ch)\n \treturn 0;\n }\n \n-int check_ref_format(const char *ref, int flags)\n+int normalize_refname(char *dst, int dstlen, const char *ref, int flags)\n {\n-\tint ch, level, last;\n-\tconst char *cp = ref;\n-\n-\tlevel = 0;\n-\twhile (1) {\n-\t\twhile ((ch = *cp++) == '/')\n-\t\t\t; /* tolerate duplicated slashes */\n-\t\tif (!ch)\n-\t\t\t/* should not end with slashes */\n-\t\t\treturn -1;\n+\tint ch, last, component_len, component_count = 0;\n+\tconst char *cp = ref, *component;\n \n-\t\t/* we are at the beginning of the path component */\n-\t\tif (ch == '.')\n-\t\t\treturn -1;\n-\t\tif (bad_ref_char(ch)) {\n-\t\t\tif ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&\n-\t\t\t\t(!*cp || *cp == '/'))\n-\t\t\t\t/* Accept one wildcard as a full refname component. */\n-\t\t\t\tflags &= ~REFNAME_REFSPEC_PATTERN;\n-\t\t\telse\n-\t\t\t\treturn -1;\n-\t\t}\n+\tch = *cp;\n+\tdo {\n+\t\twhile (ch == '/')\n+\t\t\tch = *++cp; /* tolerate leading and repeated slashes */\n \n-\t\tlast = ch;\n-\t\t/* scan the rest of the path component */\n-\t\twhile ((ch = *cp++) != 0) {\n-\t\t\tif (bad_ref_char(ch))\n-\t\t\t\treturn -1;\n-\t\t\tif (ch == '/')\n-\t\t\t\tbreak;\n+\t\t/*\n+\t\t * We are at the start of a path component.  Record\n+\t\t * its start for later reference.  If we are copying\n+\t\t * to dst, use the copy there, because we might be\n+\t\t * overwriting ref; otherwise, use the copy from the\n+\t\t * input string.\n+\t\t */\n+\t\tcomponent = dst ? dst : cp;\n+\t\tcomponent_len = 0;\n+\t\tlast = '\\0';\n+\t\twhile (1) {\n+\t\t\tif (ch != 0 && bad_ref_char(ch)) {\n+\t\t\t\tif ((flags & REFNAME_REFSPEC_PATTERN) &&\n+\t\t\t\t\tch == '*' &&\n+\t\t\t\t\tcomponent_len == 0 &&\n+\t\t\t\t\t(cp[1] == 0 || cp[1] == '/')) {\n+\t\t\t\t\t/* Accept one wildcard as a full refname component. */\n+\t\t\t\t\tflags &= ~REFNAME_REFSPEC_PATTERN;\n+\t\t\t\t} else {\n+\t\t\t\t\t/* Illegal character in refname */\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tif (last == '.' && ch == '.')\n+\t\t\t\t/* Refname must not contain \"..\". */\n \t\t\t\treturn -1;\n \t\t\tif (last == '@' && ch == '{')\n+\t\t\t\t/* Refname must not contain \"@{\". */\n \t\t\t\treturn -1;\n+\t\t\tif (dst) {\n+\t\t\t\tif (dstlen-- <= 0)\n+\t\t\t\t\t/* Output array was too small. */\n+\t\t\t\t\treturn -1;\n+\t\t\t\t*dst++ = ch;\n+\t\t\t}\n+\t\t\tif (ch == 0 || ch == '/')\n+\t\t\t\tbreak;\n+\t\t\t++component_len;\n \t\t\tlast = ch;\n+\t\t\tch = *++cp;\n \t\t}\n-\t\tlevel++;\n-\t\tif (!ch) {\n-\t\t\tif (ref <= cp - 2 && cp[-2] == '.')\n-\t\t\t\treturn -1;\n-\t\t\tif (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))\n-\t\t\t\treturn -1;\n-\t\t\tif (has_extension(ref, \".lock\"))\n-\t\t\t\treturn -1;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n+\n+\t\t/* We are at the end of a path component. */\n+\t\t++component_count;\n+\t\tif (component_len == 0)\n+\t\t\t/* Either ref was zero length or it ended with slash. */\n+\t\t\treturn -1;\n+\n+\t\tif (component[0] == '.')\n+\t\t\t/* Components must not start with '.'. */\n+\t\t\treturn -1;\n+\t} while (ch != 0);\n+\n+\tif (last == '.')\n+\t\t/* Refname must not end with '.'. */\n+\t\treturn -1;\n+\tif (component_len >= 5 && !memcmp(&component[component_len - 5], \".lock\", 5))\n+\t\t/* Refname must not end with \".lock\". */\n+\t\treturn -1;\n+\tif (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)\n+\t\t/* Refname must have at least two components. */\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+int check_ref_format(const char *ref, int flags)\n+{\n+\treturn normalize_refname(NULL, 0, ref, flags);\n }\n \n const char *prettify_refname(const char *name)\ndiff --git a/refs.h b/refs.h\nindex b248ce6..8a15f83 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -101,14 +101,24 @@ extern int for_each_reflog(each_ref_fn, void *);\n #define REFNAME_REFSPEC_PATTERN 2\n \n /*\n- * Return 0 iff ref has the correct format for a refname according to\n- * the rules described in Documentation/git-check-ref-format.txt.  If\n- * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level\n- * reference names.  If REFNAME_REFSPEC_PATTERN is set in flags, then\n- * allow a \"*\" wildcard character in place of one of the name\n- * components.\n+ * Check that ref is a valid refname according to the rules described\n+ * in Documentation/git-check-ref-format.txt and normalize it by\n+ * stripping out superfluous \"/\" characters.  If dst != NULL, write\n+ * the normalized refname to dst, which must be an allocated character\n+ * array with length dstlen (typically at least as long as ref).  dst\n+ * may point at the same memory as ref.  Return 0 iff the refname was\n+ * OK and fit into dst.  If REFNAME_ALLOW_ONELEVEL is set in flags,\n+ * then accept one-level reference names.  If REFNAME_REFSPEC_PATTERN\n+ * is set in flags, then allow a \"*\" wildcard characters in place of\n+ * one of the name components.\n  */\n-extern int check_ref_format(const char *target, int flags);\n+extern int normalize_refname(char *dst, int dstlen, const char *ref, int flags);\n+\n+/*\n+ * Return 0 iff ref has the correct format for a refname.  See\n+ * normalize_refname() for details.\n+ */\n+extern int check_ref_format(const char *ref, int flags);\n \n extern const char *prettify_refname(const char *refname);\n extern char *shorten_unambiguous_ref(const char *ref, int strict);\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 6ac5025..20a7782 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -39,6 +39,7 @@ invalid_ref 'heads/foo..bar'\n invalid_ref 'heads/foo?bar'\n valid_ref 'foo./bar'\n invalid_ref 'heads/foo.lock'\n+invalid_ref 'heads///foo.lock'\n valid_ref 'heads/foo@bar'\n invalid_ref 'heads/v@{ation'\n invalid_ref 'heads/foo\\bar'\n@@ -152,5 +153,7 @@ invalid_ref_normalized '/foo'\n invalid_ref_normalized 'heads/foo/../bar'\n invalid_ref_normalized 'heads/./foo'\n invalid_ref_normalized 'heads\\foo'\n+invalid_ref_normalized 'heads/foo.lock'\n+invalid_ref_normalized 'heads///foo.lock'\n \n test_done\n-- \n1.7.6.8.gd2879\n"},{"id":"175177","messageId":"1315568778-3592-6-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 5/6] Do not allow \".lock\" at the end of any refname component","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:17Z","receivedAt":"2011-09-09T11:46:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Allowing any refname component to end with \".lock\" is looking for\ntrouble; for example,\n\n    $ git br foo.lock/bar\n    $ git br foo\n    fatal: Unable to create '[...]/.git/refs/heads/foo.lock': File exists.\n\nTherefore, do not allow any refname component to end with \".lock\".\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n Documentation/git-check-ref-format.txt |    4 +---\n refs.c                                 |    6 +++---\n t/t1402-check-ref-format.sh            |    4 ++++\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex 3ab22b9..f2d21c7 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -28,7 +28,7 @@ git imposes the following rules on how references are named:\n \n . They can include slash `/` for hierarchical (directory)\n   grouping, but no slash-separated component can begin with a\n-  dot `.`.\n+  dot `.` or end with the sequence `.lock`.\n \n . They must contain at least one `/`. This enforces the presence of a\n   category like `heads/`, `tags/` etc. but the actual names are not\n@@ -47,8 +47,6 @@ git imposes the following rules on how references are named:\n \n . They cannot end with a slash `/` nor a dot `.`.\n \n-. They cannot end with the sequence `.lock`.\n-\n . They cannot contain a sequence `@{`.\n \n . They cannot contain a `\\`.\ndiff --git a/refs.c b/refs.c\nindex 372350e..6985a3f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -933,14 +933,14 @@ int normalize_refname(char *dst, int dstlen, const char *ref, int flags)\n \t\tif (component[0] == '.')\n \t\t\t/* Components must not start with '.'. */\n \t\t\treturn -1;\n+\t\tif (component_len >= 5 && !memcmp(&component[component_len - 5], \".lock\", 5))\n+\t\t\t/* Components must not end with \".lock\". */\n+\t\t\treturn -1;\n \t} while (ch != 0);\n \n \tif (last == '.')\n \t\t/* Refname must not end with '.'. */\n \t\treturn -1;\n-\tif (component_len >= 5 && !memcmp(&component[component_len - 5], \".lock\", 5))\n-\t\t/* Refname must not end with \".lock\". */\n-\t\treturn -1;\n \tif (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)\n \t\t/* Refname must have at least two components. */\n \t\treturn -1;\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 20a7782..6848bfb 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -40,6 +40,8 @@ invalid_ref 'heads/foo?bar'\n valid_ref 'foo./bar'\n invalid_ref 'heads/foo.lock'\n invalid_ref 'heads///foo.lock'\n+invalid_ref 'foo.lock/bar'\n+invalid_ref 'foo.lock///bar'\n valid_ref 'heads/foo@bar'\n invalid_ref 'heads/v@{ation'\n invalid_ref 'heads/foo\\bar'\n@@ -155,5 +157,7 @@ invalid_ref_normalized 'heads/./foo'\n invalid_ref_normalized 'heads\\foo'\n invalid_ref_normalized 'heads/foo.lock'\n invalid_ref_normalized 'heads///foo.lock'\n+invalid_ref_normalized 'foo.lock/bar'\n+invalid_ref_normalized 'foo.lock///bar'\n \n test_done\n-- \n1.7.6.8.gd2879\n"},{"id":"175178","messageId":"1315568778-3592-7-git-send-email-mhagger@alum.mit.edu","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 6/6] Add a REFNAME_ALLOW_UNNORMALIZED flag to check_ref_format()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T11:46:18Z","receivedAt":"2011-09-09T11:46:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Let the callers of check_ref_format() (and normalize_refname()) decide\nwhether to accept unnormalized refnames via a new\nREFNAME_ALLOW_UNNORMALIZED flag.  Change callers to set this flag,\nwhich preserves their current behavior.  (There are likely places\nwhere this flag can be removed.)\n\nAdd analogous options --allow-unnormalized and --no-allow-unnormalized\noptions to \"git check-ref-format\" and add tests.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n Documentation/git-check-ref-format.txt |    9 ++++++++-\n builtin/check-ref-format.c             |    6 +++++-\n builtin/checkout.c                     |    2 +-\n builtin/fetch-pack.c                   |    2 +-\n builtin/receive-pack.c                 |    3 ++-\n builtin/replace.c                      |    2 +-\n builtin/show-ref.c                     |    2 +-\n builtin/tag.c                          |    2 +-\n connect.c                              |    2 +-\n environment.c                          |    2 +-\n fast-import.c                          |    2 +-\n notes-merge.c                          |    4 ++--\n refs.c                                 |   11 +++++++----\n refs.h                                 |    5 ++++-\n remote.c                               |    8 +++++---\n sha1_name.c                            |    2 +-\n t/t1402-check-ref-format.sh            |    3 +++\n transport.c                            |    5 ++++-\n walker.c                               |    2 +-\n 19 files changed, 50 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex f2d21c7..fac44ec 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -71,7 +71,7 @@ reference name expressions (see linkgit:gitrevisions[7]):\n . at-open-brace `@{` is used as a notation to access a reflog entry.\n \n With the `--print` option, if 'refname' is acceptable, it prints the\n-canonicalized name of a hypothetical reference with that name.  That is,\n+normalized name of a hypothetical reference with that name.  That is,\n it prints 'refname' with any extra `/` characters removed.\n \n With the `--branch` option, it expands the ``previous branch syntax''\n@@ -95,6 +95,13 @@ OPTIONS\n \tof a one full pathname component (e.g., `foo/*/bar` but not\n \t`foo/bar*`).\n \n+--allow-unnormalized::\n+--no-allow-unnormalized::\n+\tControls whether <refname> is allowed to contain extra `/`\n+\tcharacters at the beginning and between name components.\n+\tThese extra slashes are stripped out of the value printed by\n+\tthe `--print` option.)  The default is `--allow-unnormalized`.\n+\n EXAMPLES\n --------\n \ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex 4c202af..ba0456c 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -38,7 +38,7 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n \tint print = 0;\n-\tint flags = 0;\n+\tint flags = REFNAME_ALLOW_UNNORMALIZED;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(builtin_check_ref_format_usage);\n@@ -55,6 +55,10 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)\n \t\t\tflags &= ~REFNAME_ALLOW_ONELEVEL;\n \t\telse if (!strcmp(argv[i], \"--refspec-pattern\"))\n \t\t\tflags |= REFNAME_REFSPEC_PATTERN;\n+\t\telse if (!strcmp(argv[i], \"--allow-unnormalized\"))\n+\t\t\tflags |= REFNAME_ALLOW_UNNORMALIZED;\n+\t\telse if (!strcmp(argv[i], \"--no-allow-unnormalized\"))\n+\t\t\tflags &= ~REFNAME_ALLOW_UNNORMALIZED;\n \t\telse\n \t\t\tusage(builtin_check_ref_format_usage);\n \t}\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2882116..18c3270 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \tnew->name = arg;\n \tsetup_branch_path(new);\n \n-\tif (!check_ref_format(new->path, 0) &&\n+\tif (!check_ref_format(new->path, REFNAME_ALLOW_UNNORMALIZED) &&\n \t    resolve_ref(new->path, branch_rev, 1, NULL))\n \t\thashcpy(rev, branch_rev);\n \telse\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 411aa7d..a4416e4 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)\n \tfor (ref = *refs; ref; ref = next) {\n \t\tnext = ref->next;\n \t\tif (!memcmp(ref->name, \"refs/\", 5) &&\n-\t\t    check_ref_format(ref->name + 5, 0))\n+\t\t    check_ref_format(ref->name + 5, REFNAME_ALLOW_UNNORMALIZED))\n \t\t\t; /* trash */\n \t\telse if (args.fetch_all &&\n \t\t\t (!args.depth || prefixcmp(ref->name, \"refs/tags/\") )) {\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 4e880ef..e3159c5 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -356,7 +356,8 @@ static const char *update(struct command *cmd)\n \tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n-\tif (prefixcmp(name, \"refs/\") || check_ref_format(name + 5, 0)) {\n+\tif (prefixcmp(name, \"refs/\") ||\n+\t\t\tcheck_ref_format(name + 5, REFNAME_ALLOW_UNNORMALIZED)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n \t\treturn \"funny refname\";\n \t}\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 15f0e5e..4490656 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,\n \t\t     \"refs/replace/%s\",\n \t\t     sha1_to_hex(object)) > sizeof(ref) - 1)\n \t\tdie(\"replace ref name too long: %.*s...\", 50, ref);\n-\tif (check_ref_format(ref, 0))\n+\tif (check_ref_format(ref, REFNAME_ALLOW_UNNORMALIZED))\n \t\tdie(\"'%s' is not a valid ref name.\", ref);\n \n \tif (!resolve_ref(ref, prev, 1, NULL))\ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 375a14b..dba92b2 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)\n \t\t\tif (strncmp(ref, match, matchlen))\n \t\t\t\tcontinue;\n \t\t}\n-\t\tif (check_ref_format(ref, 0)) {\n+\t\tif (check_ref_format(ref, REFNAME_ALLOW_UNNORMALIZED)) {\n \t\t\twarning(\"ref '%s' ignored\", ref);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 7aceaab..50d2018 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -412,7 +412,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n \tstrbuf_reset(sb);\n \tstrbuf_addf(sb, \"refs/tags/%s\", name);\n \n-\treturn check_ref_format(sb->buf, 0);\n+\treturn check_ref_format(sb->buf, REFNAME_ALLOW_UNNORMALIZED);\n }\n \n int cmd_tag(int argc, const char **argv, const char *prefix)\ndiff --git a/connect.c b/connect.c\nindex 292a9e2..d50f217 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)\n \tlen -= 5;\n \n \t/* REF_NORMAL means that we don't want the magic fake tag refs */\n-\tif ((flags & REF_NORMAL) && check_ref_format(name, 0))\n+\tif ((flags & REF_NORMAL) && check_ref_format(name, REFNAME_ALLOW_UNNORMALIZED))\n \t\treturn 0;\n \n \t/* REF_HEADS means that we want regular branch heads */\ndiff --git a/environment.c b/environment.c\nindex 8acbb87..2a41c03 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)\n \t\tif (strcmp((*c)->buf, \"/\") != 0)\n \t\t\tstrbuf_addf(&buf, \"refs/namespaces/%s\", (*c)->buf);\n \tstrbuf_list_free(components);\n-\tif (check_ref_format(buf.buf, 0))\n+\tif (check_ref_format(buf.buf, REFNAME_ALLOW_UNNORMALIZED))\n \t\tdie(\"bad git namespace path \\\"%s\\\"\", raw_namespace);\n \tstrbuf_addch(&buf, '/');\n \treturn strbuf_detach(&buf, NULL);\ndiff --git a/fast-import.c b/fast-import.c\nindex 4d55ee6..d8e9fe2 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -722,7 +722,7 @@ static struct branch *new_branch(const char *name)\n \n \tif (b)\n \t\tdie(\"Invalid attempt to create duplicate branch: %s\", name);\n-\tif (check_ref_format(name, REFNAME_ALLOW_ONELEVEL))\n+\tif (check_ref_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_ALLOW_UNNORMALIZED))\n \t\tdie(\"Branch name doesn't conform to GIT standards: %s\", name);\n \n \tb = pool_calloc(1, sizeof(struct branch));\ndiff --git a/notes-merge.c b/notes-merge.c\nindex bb8d7c8..c09ce7c 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -570,7 +570,7 @@ int notes_merge(struct notes_merge_options *o,\n \t/* Dereference o->local_ref into local_sha1 */\n \tif (!resolve_ref(o->local_ref, local_sha1, 0, NULL))\n \t\tdie(\"Failed to resolve local notes ref '%s'\", o->local_ref);\n-\telse if (!check_ref_format(o->local_ref, 0) &&\n+\telse if (!check_ref_format(o->local_ref, REFNAME_ALLOW_UNNORMALIZED) &&\n \t\tis_null_sha1(local_sha1))\n \t\tlocal = NULL; /* local_sha1 == null_sha1 indicates unborn ref */\n \telse if (!(local = lookup_commit_reference(local_sha1)))\n@@ -584,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,\n \t\t * Failed to get remote_sha1. If o->remote_ref looks like an\n \t\t * unborn ref, perform the merge using an empty notes tree.\n \t\t */\n-\t\tif (!check_ref_format(o->remote_ref, 0)) {\n+\t\tif (!check_ref_format(o->remote_ref, REFNAME_ALLOW_UNNORMALIZED)) {\n \t\t\thashclr(remote_sha1);\n \t\t\tremote = NULL;\n \t\t} else {\ndiff --git a/refs.c b/refs.c\nindex 6985a3f..c2a7c01 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -879,8 +879,11 @@ int normalize_refname(char *dst, int dstlen, const char *ref, int flags)\n \n \tch = *cp;\n \tdo {\n-\t\twhile (ch == '/')\n-\t\t\tch = *++cp; /* tolerate leading and repeated slashes */\n+\t\tif (flags && REFNAME_ALLOW_UNNORMALIZED) {\n+\t\t\t/* tolerate leading and repeated slashes */\n+\t\t\twhile (ch == '/')\n+\t\t\t\tch = *++cp;\n+\t\t}\n \n \t\t/*\n \t\t * We are at the start of a path component.  Record\n@@ -1132,7 +1135,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char\n struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)\n {\n \tchar refpath[PATH_MAX];\n-\tif (check_ref_format(ref, 0))\n+\tif (check_ref_format(ref, REFNAME_ALLOW_UNNORMALIZED))\n \t\treturn NULL;\n \tstrcpy(refpath, mkpath(\"refs/%s\", ref));\n \treturn lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);\n@@ -1140,7 +1143,7 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)\n \n struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)\n {\n-\tif (check_ref_format(ref, REFNAME_ALLOW_ONELEVEL))\n+\tif (check_ref_format(ref, REFNAME_ALLOW_ONELEVEL|REFNAME_ALLOW_UNNORMALIZED))\n \t\treturn NULL;\n \treturn lock_ref_sha1_basic(ref, old_sha1, flags, NULL);\n }\ndiff --git a/refs.h b/refs.h\nindex 8a15f83..f203726 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -99,6 +99,7 @@ extern int for_each_reflog(each_ref_fn, void *);\n \n #define REFNAME_ALLOW_ONELEVEL 1\n #define REFNAME_REFSPEC_PATTERN 2\n+#define REFNAME_ALLOW_UNNORMALIZED 4\n \n /*\n  * Check that ref is a valid refname according to the rules described\n@@ -110,7 +111,9 @@ extern int for_each_reflog(each_ref_fn, void *);\n  * OK and fit into dst.  If REFNAME_ALLOW_ONELEVEL is set in flags,\n  * then accept one-level reference names.  If REFNAME_REFSPEC_PATTERN\n  * is set in flags, then allow a \"*\" wildcard characters in place of\n- * one of the name components.\n+ * one of the name components.  If REFNAME_ALLOW_UNNORMALIZED is set\n+ * in flags, then allow extra \"/\" characters at the start of the\n+ * refname or between name components.\n  */\n extern int normalize_refname(char *dst, int dstlen, const char *ref, int flags);\n \ndiff --git a/remote.c b/remote.c\nindex 7059885..87da1e9 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -559,7 +559,9 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \n \t\trs[i].pattern = is_glob;\n \t\trs[i].src = xstrndup(lhs, llen);\n-\t\tflags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);\n+\t\tflags = REFNAME_ALLOW_ONELEVEL |\n+\t\t\t(is_glob ? REFNAME_REFSPEC_PATTERN : 0) |\n+\t\t\tREFNAME_ALLOW_UNNORMALIZED;\n \n \t\tif (fetch) {\n \t\t\t/*\n@@ -1403,7 +1405,7 @@ int get_fetch_map(const struct ref *remote_refs,\n \tfor (rmp = &ref_map; *rmp; ) {\n \t\tif ((*rmp)->peer_ref) {\n \t\t\tif (check_ref_format((*rmp)->peer_ref->name + 5,\n-\t\t\t\tREFNAME_ALLOW_ONELEVEL)) {\n+\t\t\t\tREFNAME_ALLOW_ONELEVEL|REFNAME_ALLOW_UNNORMALIZED)) {\n \t\t\t\tstruct ref *ignore = *rmp;\n \t\t\t\terror(\"* Ignoring funny ref '%s' locally\",\n \t\t\t\t      (*rmp)->peer_ref->name);\n@@ -1595,7 +1597,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla\n \tint len;\n \n \t/* we already know it starts with refs/ to get here */\n-\tif (check_ref_format(refname + 5, 0))\n+\tif (check_ref_format(refname + 5, REFNAME_ALLOW_UNNORMALIZED))\n \t\treturn 0;\n \n \tlen = strlen(refname) + 1;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 975ec3b..6a5d104 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -974,7 +974,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n \tif (name[0] == '-')\n \t\treturn -1;\n \tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n-\treturn check_ref_format(sb->buf, 0);\n+\treturn check_ref_format(sb->buf, REFNAME_ALLOW_UNNORMALIZED);\n }\n \n /*\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 6848bfb..a1d4c5b 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -27,9 +27,12 @@ invalid_ref() {\n \n valid_ref 'foo/bar/baz'\n valid_ref 'refs///heads/foo'\n+invalid_ref 'refs///heads/foo' --no-allow-unnormalized\n invalid_ref 'heads/foo/'\n valid_ref '/heads/foo'\n+invalid_ref '/heads/foo' --no-allow-unnormalized\n valid_ref '///heads/foo'\n+invalid_ref '///heads/foo' --no-allow-unnormalized\n invalid_ref './foo'\n invalid_ref './foo/bar'\n invalid_ref 'foo/./bar'\ndiff --git a/transport.c b/transport.c\nindex 225d9b8..65209af 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -754,7 +754,10 @@ void transport_verify_remote_names(int nr_heads, const char **heads)\n \t\t\tcontinue;\n \n \t\tremote = remote ? (remote + 1) : local;\n-\t\tif (check_ref_format(remote, REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))\n+\t\tif (check_ref_format(remote,\n+\t\t\t\t     REFNAME_ALLOW_ONELEVEL|\n+\t\t\t\t     REFNAME_REFSPEC_PATTERN|\n+\t\t\t\t     REFNAME_ALLOW_UNNORMALIZED))\n \t\t\tdie(\"remote part of refspec is not a valid name in %s\",\n \t\t\t\theads[i]);\n \t}\ndiff --git a/walker.c b/walker.c\nindex e5d8eb2..f0dbe58 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *\n {\n \tif (!get_sha1_hex(target, sha1))\n \t\treturn 0;\n-\tif (!check_ref_format(target, 0)) {\n+\tif (!check_ref_format(target, REFNAME_ALLOW_UNNORMALIZED)) {\n \t\tstruct ref *ref = alloc_ref(target);\n \t\tif (!walker->fetch_ref(walker, ref)) {\n \t\t\thashcpy(sha1, ref->old_sha1);\n-- \n1.7.6.8.gd2879\n"},{"id":"175190","messageId":"4E6A1D7D.6050602@gmail.com","threadId":"28345","inReplyTo":"1315568778-3592-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 0/6] Improved infrastructure for refname normalization","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2011-09-09T14:06:53Z","receivedAt":"2011-09-09T14:06:53Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"On 09/09/2011 07:46 AM, Michael Haggerty wrote:\n> As a prerequisite to storing references caches hierarchically (itself\n> needed for performance reasons), here is a patch series to help us get\n> refname normalization under control.\n>\n> The problem is that some UI accepts unnormalized reference names (like\n> \"/foo/bar\" or \"foo///bar\" instead of \"foo/bar\") and passes them on to\n> library routines without normalizing them.  The library, on the other\n> hand, assumes that the refnames are normalized.  Sometimes (mostly in\n> the case of loose references) unnormalized refnames happen to work,\n> but in other cases (like packed references or when looking up refnames\n> in the cache) they silently fail.  Given that refnames are sometimes\n> treated as path names, there is a chance that some security-relevant\n> bugs are lurking in this area, if not in git proper then in scripts\n> that interact with git.\n\nWhy can't the library do the normalization instead of expecting every \nother component that deals with reference names having to do it for the \nlibrary?\n\n[...]\n\n>\n> * Forbid \".lock\" at the end of any refname component, as directories\n>    with such names can conflict with attempts to create lock files for\n>    other refnames.\n\nI find this overly restrictive. If you need to create a lock based on a \nreference name or component, use a name for the lock object that starts \nwith one of the characters that reference names or components are \nalready forbidden from starting with.\n\n\nGitzilla\n"},{"id":"175199","messageId":"4E6A31D1.5020404@alum.mit.edu","threadId":"28345","inReplyTo":"4E6A1D7D.6050602@gmail.com","subject":"Re: [PATCH 0/6] Improved infrastructure for refname normalization","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-09T15:33:37Z","receivedAt":"2011-09-09T15:33:37Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/09/2011 04:06 PM, A Large Angry SCM wrote:\n> On 09/09/2011 07:46 AM, Michael Haggerty wrote:\n>> As a prerequisite to storing references caches hierarchically (itself\n>> needed for performance reasons), here is a patch series to help us get\n>> refname normalization under control.\n>>\n>> The problem is that some UI accepts unnormalized reference names (like\n>> \"/foo/bar\" or \"foo///bar\" instead of \"foo/bar\") and passes them on to\n>> library routines without normalizing them.  The library, on the other\n>> hand, assumes that the refnames are normalized.  Sometimes (mostly in\n>> the case of loose references) unnormalized refnames happen to work,\n>> but in other cases (like packed references or when looking up refnames\n>> in the cache) they silently fail.  Given that refnames are sometimes\n>> treated as path names, there is a chance that some security-relevant\n>> bugs are lurking in this area, if not in git proper then in scripts\n>> that interact with git.\n> \n> Why can't the library do the normalization instead of expecting every\n> other component that deals with reference names having to do it for the\n> library?\n\nThe library could do the normalization, but\n\n1. It would probably cost a lot of redundant checks as reference names\npass in and out of the library and back in again\n\n2. Normalization requires copying or overwriting the incoming string, so\neach time a refname crosses the library perimeter there might have to be\nan extra memory allocation with the associated headaches of dealing with\nthe ownership of the memory.\n\n3. The library doesn't encapsulate all uses of reference names; for\nexample, for_each_ref() invokes a callback function with the refname as\nan argument.  The callback function is free to do a strcmp() of the\nrefname (normalized by the library) with some arbitrary string that it\ngot from the command line.  Either the caller has to do the\nnormalization itself (i.e., outside of the library) or the library has\nto learn how to do every possible filtering operation with refnames.\n\n>> * Forbid \".lock\" at the end of any refname component, as directories\n>>    with such names can conflict with attempts to create lock files for\n>>    other refnames.\n> \n> I find this overly restrictive. If you need to create a lock based on a\n> reference name or component, use a name for the lock object that starts\n> with one of the characters that reference names or components are\n> already forbidden from starting with.\n\nI agree; this is unpleasantly restrictive.\n\nBut please remember that refnames already cannot end in \".lock\"\n(\"foo/bar.lock\" is already forbidden; this change also prohibits\n\"foo.lock/bar\").\n\nHowever, your suggested solution would cause problems if two versions of\ngit are running on the same machine.  An old version of git would not\nknow to respect the new version's lock files.  ISTM that this would be\ntoo dangerous.  Suggestions welcome.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"175221","messageId":"7vzkidtx81.fsf@alter.siamese.dyndns.org","threadId":"28345","inReplyTo":"4E6A31D1.5020404@alum.mit.edu","subject":"Re: [PATCH 0/6] Improved infrastructure for refname normalization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-09T17:57:34Z","receivedAt":"2011-09-09T17:57:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> The library could do the normalization, but\n>\n> 1. It would probably cost a lot of redundant checks as reference names\n> pass in and out of the library and back in again\n>\n> 2. Normalization requires copying or overwriting the incoming string, so\n> each time a refname crosses the library perimeter there might have to be\n> an extra memory allocation with the associated headaches of dealing with\n> the ownership of the memory.\n>\n> 3. The library doesn't encapsulate all uses of reference names; for\n> example, for_each_ref() invokes a callback function with the refname as\n> an argument.  The callback function is free to do a strcmp() of the\n> refname (normalized by the library) with some arbitrary string that it\n> got from the command line.  Either the caller has to do the\n> normalization itself (i.e., outside of the library) or the library has\n> to learn how to do every possible filtering operation with refnames.\n\n4. The caller needs to be corrected to pay attention to the normalization\nthe library did for it. Your code may use a string as a ref and then\ncreate something based on the refname; illustrating with a fictitious\nexample:\n\n\tref = make_branch_ref(\"refs/heads/%s\", branch_name);\n        update_ref(ref, sha1);\n        write_log(\"created branch '%s'\", branch_name);\n\nEven though make_branch_ref() may have removed duplicated slashes from the\nname in \"branch_name\" when it computed \"ref\", the log still will record\nunnormalized name.\n\nI think the callers need to be aware of the normalization in practice\nanyway for this reason, and a good way forward is to give the callers a\nlibrary interface to do so. It might even make sense to make the other\nparts of the API _reject_ unnormalized input to catch offending callers.\n\nBy the way, does this series introduce new infrastructure features that\ncan be reused in different areas, such as Hui's \"alt_odb path\nnormalization\" patch?\n"},{"id":"175252","messageId":"7vpqj9s385.fsf@alter.siamese.dyndns.org","threadId":"28345","inReplyTo":"1315568778-3592-7-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 6/6] Add a REFNAME_ALLOW_UNNORMALIZED flag to check_ref_format()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-09T23:30:50Z","receivedAt":"2011-09-09T23:30:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> Let the callers of check_ref_format() (and normalize_refname()) decide\n> whether to accept unnormalized refnames via a new\n> REFNAME_ALLOW_UNNORMALIZED flag.  Change callers to set this flag,\n> which preserves their current behavior.  (There are likely places\n> where this flag can be removed.)\n\nHmm, is it just me who finds --no-allow-unnormalized backwards?\n\nMore importantly, shouldn't every caller be required to normalize refnames\nby default, unless it can justify why it does not have to with a compelling\nreason?\n\nIn other words, I would be Ok if \"--no-require-normalized\" was the default\nfor \"git check-ref-format\" for scripts' use, and I also would be perfectly\nfine if callers feed un-normalized strings that came from the command line\nargument and other end user input to normalize_refname(), but once such a\nstring is normalized, shouldn't the rest of the callchain be passing the\nnormalized refname all the way?\n\nTo put it another way, my knee jerk reaction is that we shouldn't need\nsuch a \"flag\". Shouldn't it be sufficient for normalize_refname() and\nnothing else to allow unnormalized input, and everybody else should barf\nwhen they see an un-normalized input?\n"},{"id":"175257","messageId":"4E6ADA23.4010800@alum.mit.edu","threadId":"28345","inReplyTo":"7vzkidtx81.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/6] Improved infrastructure for refname normalization","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T03:31:47Z","receivedAt":"2011-09-10T03:31:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/09/2011 07:57 PM, Junio C Hamano wrote:\n> By the way, does this series introduce new infrastructure features that\n> can be reused in different areas, such as Hui's \"alt_odb path\n> normalization\" patch?\n\nThat code is for normalizing filesystem paths, right?\n\nThe rules for normalizing filesystem paths are similar to those for\nrefnames (except maybe for stripping the leading \"/\").  But the validity\nchecks are different, and should be kept separate in case some of the\nrules need to be tweaked.  Since I put the code for validity checks and\nnormalization of refnames in a single function, I don't think it makes\nsense to share code.\n\nIt would be possible to separate the validity checks from the\nnormalization, but that would require two scans of the refname.  And I\nthink it should be considered rather an accident that filesystem names\nand refnames have similar conventions (even though there is a strong\nhistorical reason for the similarity); they could some day diverge if,\nsay, we started adding support for Windows-native paths.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"175258","messageId":"4E6AE1D9.9010004@alum.mit.edu","threadId":"28345","inReplyTo":"7vpqj9s385.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/6] Add a REFNAME_ALLOW_UNNORMALIZED flag to check_ref_format()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T04:04:41Z","receivedAt":"2011-09-10T04:04:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/10/2011 01:30 AM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>> Let the callers of check_ref_format() (and normalize_refname()) decide\n>> whether to accept unnormalized refnames via a new\n>> REFNAME_ALLOW_UNNORMALIZED flag.  Change callers to set this flag,\n>> which preserves their current behavior.  (There are likely places\n>> where this flag can be removed.)\n> \n> [...]\n> To put it another way, my knee jerk reaction is that we shouldn't need\n> such a \"flag\". Shouldn't it be sufficient for normalize_refname() and\n> nothing else to allow unnormalized input, and everybody else should barf\n> when they see an un-normalized input?\n\nThat is a good idea.\n\nI will make the current normalize_refname() function static and hide the\nREFNAME_ALLOW_UNNORMALIZED option from the outside world.  Then I will\nwrite a new public normalize_refname() function that calls the static\nversion with REFNAME_ALLOW_UNNORMALIZED set, and change\ncheck_ref_format() to call normalize_refname() with\nREFNAME_ALLOW_UNNORMALIZED unset.\n\nWhat should I do with all of the current callers of check_ref_format(),\ngiven that I don't want to be personally responsible for analyzing and\nrewriting them all?  The hard-nosed approach would be to say that they\nare calling check_ref_format() without normalizing the refnames, so they\nare already broken (albeit perhaps sometimes accidentally functional),\nand it is OK that the new behavior of check_ref_format() causes them to\nfail explicitly.\n\nA more forgiving approach would be to implement another transition\nfunction like check_ref_format_deprecated_unsafe() that accepts\nunnormalized refnames, change the callers to use this function during\nthe transition, and remove it only after all callers have been fixed.\n\nSuggestions?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}