{"thread":{"id":"28350","subject":"[PATCH v2 2/7] Change bad_ref_char() to return a boolean value","startedAt":"2011-09-10T06:50:36Z","lastAt":"2011-09-13T17:43:57Z","messageCount":13,"participants":["Michael Haggerty","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"175264","messageId":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":null,"subject":"[PATCH v2 0/7] Improved infrastructure for refname normalization","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:36Z","receivedAt":"2011-09-10T06:50:36Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Patch series re-roll:\n\nThe main difference is that I incorporated feedback from Junio that it\nshould be made more difficult to use unnormalized references. The\nmechanism that I selected is a bit different than discussed in the\nmailing list; I changed normalize_refname() to accept unnormalized\nrefnames *only* if the dst argument is non-NULL. I changed\ncheck_ref_format() to reject unnormalized refnames but implemented a\ntransitional function check_ref_format_unsafe() that has the old\nbehavior and changed callers to use the _unsafe function to avoid\nchanging their behavior.\n\n\nAs 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 This function only accepts normalized refnames.\n\n* Add a function normalize_refname() that accepts unnormalized\n refnames, checks the format, and outputs a normalized version.\n\n* Add a function check_ref_format_unsafe() that has the old behavior.\n Change callers to use this function until they can be fixed.\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\nI suggest the following policy for handling unnormalized refnames more\nconsistently:\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 int len = strlen(arg);\n char *normalized_refname = xmalloc(len);\n if (normalize_refname(normalized_refname, len, arg, 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, 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 (7):\n t1402: add some more tests\n Change bad_ref_char() to return a boolean value\n git check-ref-format: add options --allow-onelevel and\n --refspec-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 tools to avoid the use of unnormalized refnames.\n\n Documentation/git-check-ref-format.txt | 44 ++++++++--\n builtin/check-ref-format.c | 76 +++++++++---------\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 | 139 +++++++++++++++++++------------\n refs.h | 46 +++++++++-\n remote.c | 54 ++++---------\n sha1_name.c | 4 +-\n t/t1402-check-ref-format.sh | 96 ++++++++++++++++++++--\n transport.c | 17 +---\n walker.c | 2 +-\n 20 files changed, 325 insertions(+), 185 deletions(-)\n\n-- \n1.7.6.8.gd2879\n"},{"id":"175263","messageId":"1315637443-14012-2-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 1/7] t1402: add some more tests","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:37Z","receivedAt":"2011-09-10T06:50:37Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty \n---\n t/t1402-check-ref-format.sh | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex ed4275a..eeb0b1a 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -13,6 +13,8 @@ invalid_ref() {\n \t\t\"test_must_fail git check-ref-format '$1'\"\n }\n \n+invalid_ref ''\n+invalid_ref '/'\n valid_ref 'heads/foo'\n invalid_ref 'foo'\n valid_ref 'foo/bar/baz'\n-- \n1.7.6.8.gd2879\n"},{"id":"175261","messageId":"1315637443-14012-3-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 2/7] Change bad_ref_char() to return a boolean value","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:38Z","receivedAt":"2011-09-10T06:50:38Z","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 \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":"175266","messageId":"1315637443-14012-4-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 3/7] git check-ref-format: add options --allow-onelevel and --refspec-pattern","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:39Z","receivedAt":"2011-09-10T06:50:39Z","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 \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 | 88 +++++++++++++++++++++++++++++---\n 3 files changed, 152 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' \n-'git check-ref-format' --print \n+'git check-ref-format' [--print]\n+ [--[no-]allow-onelevel] [--refspec-pattern] \n 'git check-ref-format' --branch \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 as a reference name pattern for a refspec\n+\t(as used with remote repositories). If this option is\n+\tenabled, 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] \\n\"\n+\"git check-ref-format [--print] [options] \\n\"\n \" or: git check-ref-format --branch \";\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 eeb0b1a..795301d 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -5,25 +5,38 @@ 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 invalid_ref ''\n invalid_ref '/'\n-valid_ref 'heads/foo'\n-invalid_ref 'foo'\n+invalid_ref '/' --allow-onelevel\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@@ -35,6 +48,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":"175268","messageId":"1315637443-14012-5-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 4/7] Change check_ref_format() to take a flags argument","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:40Z","receivedAt":"2011-09-10T06:50:40Z","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 \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 795301d..d7e8d90 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -84,11 +84,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":"175267","messageId":"1315637443-14012-6-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 5/7] Add a library function normalize_refname()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:41Z","receivedAt":"2011-09-10T06:50:41Z","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 \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] \\n\"\n \" or: git check-ref-format --branch \";\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 d7e8d90..b0b773b 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -42,6 +42,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@@ -155,5 +156,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":"175262","messageId":"1315637443-14012-7-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 6/7] Do not allow \".lock\" at the end of any refname component","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:42Z","receivedAt":"2011-09-10T06:50:42Z","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 \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 b0b773b..419788f 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -43,6 +43,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@@ -158,5 +160,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":"175265","messageId":"1315637443-14012-8-git-send-email-mhagger@alum.mit.edu","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 7/7] Add tools to avoid the use of unnormalized refnames.","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-10T06:50:43Z","receivedAt":"2011-09-10T06:50:43Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change normalize_refname() to only allow unnormalized refnames if its\ndst parameter is non-NULL. Callers who want to support unnormalized\nrefnames will need the unnormalized copy, and callers unprepared to\ndeal with a normalized copy shouldn't pretend to accept unnormalized\nrefnames.\n\nChange check_ref_format() to reject unnormalized refnames.\n\nAdd a new temporary function, check_ref_format_unsafe(), which accepts\nunnormalized refnames like the old check_ref_format(). Change callers\nto use this function so that their behavior is preserved, even though\nin many cases it is brokenness that is being preserved. Callers\nshould be fixed to use one of the new functions, then\ncheck_ref_format_unsafe() should be removed.\n\nAdd options --normalize and --no-normalize to \"git check-ref-format\"\nto access the new functionality and use the new options to add tests.\n\nSigned-off-by: Michael Haggerty \n---\n Documentation/git-check-ref-format.txt | 13 ++++++++-\n builtin/check-ref-format.c | 34 ++++++++++++++-----------\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 | 2 +-\n connect.c | 2 +-\n environment.c | 2 +-\n fast-import.c | 2 +-\n notes-merge.c | 4 +-\n refs.c | 17 ++++++++++---\n refs.h | 41 +++++++++++++++++++++++--------\n remote.c | 17 +++++++------\n sha1_name.c | 2 +-\n t/t1402-check-ref-format.sh | 3 ++\n transport.c | 4 ++-\n walker.c | 2 +-\n 19 files changed, 101 insertions(+), 54 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex f2d21c7..25a7c43 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -9,7 +9,8 @@ SYNOPSIS\n --------\n [verse]\n 'git check-ref-format' [--print]\n- [--[no-]allow-onelevel] [--refspec-pattern] \n+ [--[no-]allow-onelevel] [--refspec-pattern] [--[no-]normalize]\n+ \n 'git check-ref-format' --branch \n \n DESCRIPTION\n@@ -71,7 +72,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 +96,14 @@ OPTIONS\n \tof a one full pathname component (e.g., `foo/*/bar` but not\n \t`foo/bar*`).\n \n+--normalize::\n+--no-normalize::\n+\tControls whether is allowed to contain extra `/`\n+\tcharacters at the beginning and between name components. If\n+\tthis option is set, such extra slashes are stripped out of the\n+\tvalue printed by the `--print` option. The default is\n+\t`--normalize`.\n+\n EXAMPLES\n --------\n \ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex 4c202af..fc1ffd2 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -23,22 +23,12 @@ static int check_ref_format_branch(const char *arg)\n \treturn 0;\n }\n \n-static int check_ref_format_print(const char *arg, int flags)\n-{\n-\tint refnamelen = strlen(arg) + 1;\n-\tchar *refname = xmalloc(refnamelen);\n-\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 {\n \tint i;\n \tint print = 0;\n \tint flags = 0;\n+\tint normalize = 1;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(builtin_check_ref_format_usage);\n@@ -55,13 +45,27 @@ 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], \"--normalize\"))\n+\t\t\tnormalize = 1;\n+\t\telse if (!strcmp(argv[i], \"--no-normalize\"))\n+\t\t\tnormalize = 0;\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-\tif (print)\n-\t\treturn check_ref_format_print(argv[i], flags);\n-\telse\n-\t\treturn !!check_ref_format(argv[i], flags);\n+\tif (normalize) {\n+\t\tint refnamelen = strlen(argv[i]) + 1;\n+\t\tchar *refname = xmalloc(refnamelen);\n+\t\tif (normalize_refname(refname, refnamelen, argv[i], flags))\n+\t\t\treturn 1;\n+\t\tif (print)\n+\t\t\tprintf(\"%s\\n\", refname);\n+\t} else {\n+\t\tif (check_ref_format(argv[i], flags))\n+\t\t\treturn 1;\n+\t\tif (print)\n+\t\t\tprintf(\"%s\\n\", argv[i]);\n+\t}\n+\treturn 0;\n }\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2882116..cf16ac3 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_unsafe(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 411aa7d..640e1ac 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_unsafe(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 4e880ef..24d917b 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, 0)) {\n+\tif (prefixcmp(name, \"refs/\") || check_ref_format_unsafe(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 15f0e5e..ac6b154 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_unsafe(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 375a14b..ba5f66f 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_unsafe(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 7aceaab..10ee87c 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_unsafe(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 292a9e2..3fea6b1 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_unsafe(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 8acbb87..ec293e8 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_unsafe(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 4d55ee6..70271f0 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_unsafe(name, REFNAME_ALLOW_ONELEVEL))\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..7e7dd3a 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_unsafe(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@@ -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_unsafe(o->remote_ref, 0)) {\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..26720e8 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 (dst) {\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@@ -947,6 +950,12 @@ int normalize_refname(char *dst, int dstlen, const char *ref, int flags)\n \treturn 0;\n }\n \n+int check_ref_format_unsafe(const char *ref, int flags)\n+{\n+\tchar unused[PATH_MAX];\n+\treturn normalize_refname(unused, sizeof(unused), ref, flags);\n+}\n+\n int check_ref_format(const char *ref, int flags)\n {\n \treturn normalize_refname(NULL, 0, ref, flags);\n@@ -1132,7 +1141,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_unsafe(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@@ -1140,7 +1149,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_unsafe(ref, REFNAME_ALLOW_ONELEVEL))\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..bdfd230 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -102,24 +102,43 @@ extern int for_each_reflog(each_ref_fn, void *);\n \n /*\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+ * in Documentation/git-check-ref-format.txt. If\n+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept reference names\n+ * with only a single component. If REFNAME_REFSPEC_PATTERN is set in\n+ * flags, then allow a \"*\" wildcard character in place of one of the\n+ * name components.\n+ *\n+ * The handling of normalized/unnormalized refnames is determined by\n+ * dst:\n+ *\n+ * - If dst is non-NULL, then it must be an allocated character array\n+ * with length dstlen. Usually dstlen should be at least\n+ * strlen(ref)+1. Dst may point at the same memory as ref. In this\n+ * case, write a normalized copy of ref to dst, stripping out\n+ * superfluous \"/\" characters.\n+ *\n+ * - If dst is NULL, verify that the reference is already normalized\n+ * and do no copying.\n+ *\n+ * Return 0 iff the refname was OK and fit into dst.\n */\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+ * Return 0 iff ref has the correct format for a refname and is\n+ * correctly normalized. See normalize_refname() for details.\n */\n extern int check_ref_format(const char *ref, int flags);\n \n+/*\n+ * Return 0 iff ref has the correct format for a refname, *without*\n+ * requiring it to be normalized. This function is unsafe because\n+ * unnormalized refnames should not be used; please use\n+ * normalize_refname() if you want to accept unnormalized refnames or\n+ * check_ref_format() if you only want to accept normalized refnames.\n+ */\n+extern int check_ref_format_unsafe(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);\n \ndiff --git a/remote.c b/remote.c\nindex 7059885..00e7523 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -559,7 +559,8 @@ 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 \n \t\tif (fetch) {\n \t\t\t/*\n@@ -569,7 +570,7 @@ 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 if (check_ref_format(rs[i].src, flags))\n+\t\t\telse if (check_ref_format_unsafe(rs[i].src, flags))\n \t\t\t\tgoto invalid;\n \t\t\t/*\n \t\t\t * RHS\n@@ -581,7 +582,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp\n \t\t\t\t; /* ok */\n \t\t\telse if (!*rs[i].dst)\n \t\t\t\t; /* ok */\n-\t\t\telse if (check_ref_format(rs[i].dst, flags))\n+\t\t\telse if (check_ref_format_unsafe(rs[i].dst, flags))\n \t\t\t\tgoto invalid;\n \t\t} else {\n \t\t\t/*\n@@ -594,7 +595,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\tif (check_ref_format(rs[i].src, flags))\n+\t\t\t\tif (check_ref_format_unsafe(rs[i].src, flags))\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n \t\t\telse\n@@ -607,12 +608,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\tif (check_ref_format(rs[i].src, flags))\n+\t\t\t\tif (check_ref_format_unsafe(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\tif (check_ref_format(rs[i].dst, flags))\n+\t\t\t\tif (check_ref_format_unsafe(rs[i].dst, flags))\n \t\t\t\t\tgoto invalid;\n \t\t\t}\n \t\t}\n@@ -1402,7 +1403,7 @@ 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\tif (check_ref_format((*rmp)->peer_ref->name + 5,\n+\t\t\tif (check_ref_format_unsafe((*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@@ -1595,7 +1596,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_unsafe(refname + 5, 0))\n \t\treturn 0;\n \n \tlen = strlen(refname) + 1;\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 975ec3b..8458454 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_unsafe(sb->buf, 0);\n }\n \n /*\ndiff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh\nindex 419788f..411c271 100755\n--- a/t/t1402-check-ref-format.sh\n+++ b/t/t1402-check-ref-format.sh\n@@ -30,9 +30,12 @@ invalid_ref '/'\n invalid_ref '/' --allow-onelevel\n valid_ref 'foo/bar/baz'\n valid_ref 'refs///heads/foo'\n+invalid_ref 'refs///heads/foo' --no-normalize\n invalid_ref 'heads/foo/'\n valid_ref '/heads/foo'\n+invalid_ref '/heads/foo' --no-normalize\n valid_ref '///heads/foo'\n+invalid_ref '///heads/foo' --no-normalize\n invalid_ref './foo'\n invalid_ref './foo/bar'\n invalid_ref 'foo/./bar'\ndiff --git a/transport.c b/transport.c\nindex 225d9b8..e00610d 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -754,7 +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\tif (check_ref_format(remote, REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))\n+\t\tif (check_ref_format_unsafe(remote,\n+\t\t\t\t REFNAME_ALLOW_ONELEVEL|\n+\t\t\t\t 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}\ndiff --git a/walker.c b/walker.c\nindex e5d8eb2..a1c1ee2 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_unsafe(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":"175325","messageId":"7vehzmbd0o.fsf@alter.siamese.dyndns.org","threadId":"28350","inReplyTo":"1315637443-14012-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 0/7] Improved infrastructure for refname normalization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-12T04:28:07Z","receivedAt":"2011-09-12T04:28:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> Patch series re-roll:\n\nThanks for working on this. I very much like the general direction of the\nseries, the strategy to avoid wholesale audit of the callers and marking\nthe places that needs fixing with \"_unsafe()\".\n\nThere were a few minor things that looked worth mentioning while\nreviewing, though.\n\n - (style) You seem to be fond of pre-increment a lot, but in general our\n codebase prefers post-increment especially when the end result does not\n make any difference, e.g.\n\n\tfor (i = 1; ...; ++i) {\n \t...\n\n - (series structure) It might make the series progress easier to follow\n if you introduced check_ref_format_unsafe() in the same commit where\n you change check_ref_format() to take flags parameter.\n"},{"id":"175336","messageId":"4E6E2122.8000201@alum.mit.edu","threadId":"28350","inReplyTo":"7vehzmbd0o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/7] Improved infrastructure for refname normalization","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-12T15:11:30Z","receivedAt":"2011-09-12T15:11:30Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/12/2011 06:28 AM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n> There were a few minor things that looked worth mentioning while\n> reviewing, though.\n> \n> - (style) You seem to be fond of pre-increment a lot, but in general our\n> codebase prefers post-increment especially when the end result does not\n> make any difference, e.g.\n> \n> \tfor (i = 1; ...; ++i) {\n> \t...\n\nOK, changed.\n\n> - (series structure) It might make the series progress easier to follow\n> if you introduced check_ref_format_unsafe() in the same commit where\n> you change check_ref_format() to take flags parameter.\n\nOK. I'll take the opportunity to rename the functions to\ncheck_refname_format*(), to make it more obvious that they only concern\nthemselves with the refnames and not the references themselves.\n\nI discovered a bug in my code for handling refnames without\nnormalization; I will also fix that in v3.\n\nOTOH I am again having serious doubts that trying to support\nunnormalized refnames is a good idea. I will write more when I have\ntime to argue my case.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"175340","messageId":"7vwrdd90df.fsf@alter.siamese.dyndns.org","threadId":"28350","inReplyTo":"4E6E2122.8000201@alum.mit.edu","subject":"Re: [PATCH v2 0/7] Improved infrastructure for refname normalization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-12T16:44:12Z","receivedAt":"2011-09-12T16:44:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> OTOH I am again having serious doubts that trying to support\n> unnormalized refnames is a good idea.\n\nSorry, I am confused. I thought the way you are planning forward is to\nleave unnormalized ones unchecked as the current code does (and mark the\nplaces that do so with _unsafe()), with the eventual goal of fixing the\ncaller to pass only normalized ones to make call to the \"safe\" version?\n"},{"id":"175385","messageId":"4E6ED90D.1090704@alum.mit.edu","threadId":"28350","inReplyTo":"7vwrdd90df.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/7] Improved infrastructure for refname normalization","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-09-13T04:16:13Z","receivedAt":"2011-09-13T04:16:13Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/12/2011 06:44 PM, Junio C Hamano wrote:\n> Michael Haggerty writes:\n>> OTOH I am again having serious doubts that trying to support\n>> unnormalized refnames is a good idea.\n> \n> Sorry, I am confused. I thought the way you are planning forward is to\n> leave unnormalized ones unchecked as the current code does (and mark the\n> places that do so with _unsafe()), with the eventual goal of fixing the\n> caller to pass only normalized ones to make call to the \"safe\" version?\n\nThat was the plan, but after my experience trying to fix this problem I\nhave come to doubt that it is doable within a reasonable amount of work\nor even that support for unnormalized refnames is desirable. The\nproblem is that the API provided by refs.{c,h} is far from waterproof,\nand I keep finding more code elsewhere that manipulates and parses\nrefnames and makes implicit assumptions (sometimes spread over many\nfunctions) about their form.\n\nConsistency of the UI should be the goal. Supporting unnormalized\nrefnames some places, but not others, will just confuse and frustrate\nusers. The only two consistent alternatives are\n\n1. Unnormalized refnames are supported everywhere in the UI that\nrefnames are accepted, including clients like gitweb, gitk, egit, etc.\n\n2. Only normalized refnames are supported; unnormalized refnames are\nerrors that we report on a best-effort basis.\n\nOption (1) has a number of problems:\n\n* Claiming to support unnormalized refnames is far from the current\nsituation; therefore lots of current code would have to be considered\nbroken.\n\n* Fixing the code requires many new unit tests and fixes to many areas\nof the code, including clients outside of the main git project. I have\ntried fixing a couple of examples (\"git branch\", \"git rev-parse\", and\nthe first argument of \"git update-ref\") and it is pretty messy.\n\n* Some of the needed changes seem like they might conflict with other\nforms of DWIM; for example, the ambiguous_path() kludge.\n\n* The extra copying needed for normalization has a runtime cost and\ncomplicates memory management.\n\n* Unnormalized refnames are *themselves* a form of UI inconsistency and\ntherefore not a very noble goal. It is better that people learn that\neach reference has a single name, and unlearn that references were once\n1:1 with files under .git/refs.\n\nWhat is the benefit of (2) that justifies all of this work? To enable\nsloppy script writers to throw slashes around carelessly?\n\nOption (1) would be far easier. Then code only needs to treat\nunnormalized refnames like any other kind of invalid refname rather than\nmaking sure that unnormalized refnames work properly in combination with\nall other features.\n\nSo I propose the following:\n\n* Institute a policy that refnames in the UI must always be normalized\n\n* On a best-effort basis, emit errors whenever unnormalized refnames are\nencountered\n\n* Continue to support \"git check-ref-format --print\", which script\nwriters can use to normalize refnames if they need to.\n\nThe only disadvantage of a stricter policy is that some of today's\nsloppy scripts, which today might sometimes work (but not reliably),\nbreak in a pretty obvious way that can be fixed with a single call to\n\"git check-ref-format --print\".\n\nI'd rather get beyond this swamp and start working on the hierarchical\nreference cache, which will bring some real benefits. (The hierarchical\nreference cache requires some sanity in refname handling, which is why I\ngot into this mess in the first place.)\n\nWhat do people think?\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"175415","messageId":"7vlitswd5u.fsf@alter.siamese.dyndns.org","threadId":"28350","inReplyTo":"4E6ED90D.1090704@alum.mit.edu","subject":"Re: [PATCH v2 0/7] Improved infrastructure for refname normalization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-13T17:43:57Z","receivedAt":"2011-09-13T17:43:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> Consistency of the UI should be the goal. Supporting unnormalized\n> refnames some places, but not others, will just confuse and frustrate\n> users.\n> ...\n> So I propose the following:\n>\n> * Institute a policy that refnames in the UI must always be normalized\n>\n> * On a best-effort basis, emit errors whenever unnormalized refnames are\n> encountered\n>\n> * Continue to support \"git check-ref-format --print\", which script\n> writers can use to normalize refnames if they need to.\n>\n> The only disadvantage of a stricter policy is that some of today's\n> sloppy scripts, which today might sometimes work (but not reliably),\n> break in a pretty obvious way that can be fixed with a single call to\n> \"git check-ref-format --print\".\n>\n> I'd rather get beyond this swamp and start working on the hierarchical\n> reference cache, which will bring some real benefits. (The hierarchical\n> reference cache requires some sanity in refname handling, which is why I\n> got into this mess in the first place.)\n\nBoth the analysis and theproposal I find very sane and sensible. Thanks.\n"}]}